Return QTI for still-legacy assessment items on read - #6073
Conversation
| continue | ||
| try: | ||
| # A new dict, so the language does not leak into the response. | ||
| result = convert_legacy_question_to_qti(dict(item, language=language)) |
There was a problem hiding this comment.
(Ported from rtibblesbot on #6095) convert_legacy_question_to_qti raises on math directly inside an <li>, which FlowContentElement does not accept.
There was a problem hiding this comment.
Current assessments can include links (a elements) in their markdown format, but when transformed into a QTI XML, the conversion fails.
There was a problem hiding this comment.
Both are the same defect: the pydantic HTML models are narrower than the QTI XSD they encode.
Swept it by running every construct render_markdown can emit (the gfm-like preset, plus raw HTML passthrough) through the converter in all four slots it renders into — question, choice answer, hint, text-entry prompt. Eight places matched, all now fixed:
a@hrefrejected every absolute URL — external links, autolinks,mailto:. The schema types itxs:anyURI; only schemes that execute rather than navigate stay rejected.li,td,th,caption,dt,ddandqti-simple-choicetook nomath, though every one of those content models admitsm3:math.
Publish and the ricecooker upload reach the same converter, so both stop failing on these too.
One case is left, and it is not a model that can be widened: ~~text~~ renders <s>, which the QTI 3.0 HTML profile has no element for (no del/ins either) — same for any raw HTML tag an author typed. Either the mark is dropped during rendering or the item stays unconvertible. With log-and-skip it now degrades to an unsupported card instead of a 500. Tell me which you want and I will do it.
| # serialize_object() turns these into a 404 (base.py), reporting | ||
| # a corrupt row as a missing one; re-raise as a type it does not | ||
| # catch. pydantic and json errors both subclass ValueError. | ||
| raise LegacyConversionError( |
There was a problem hiding this comment.
Uncaught: one bad row 500s the whole contentnode__in list. Log and skip per item; isUnsupported renders it.
There was a problem hiding this comment.
Done — consolidate logs the failure with the assessment_id and leaves the row as the legacy row it is, so the rest of the contentnode__in list still returns and the editor renders it via isUnsupported. LegacyConversionError is gone; the two tests that asserted a raise now assert the sibling item still converts and that the detail route returns 200 rather than a 404.
Searched the branch for the same shape — a per-item failure that aborts a batch. consolidate was the only one; the hint loop in _create_catalog_info already logs and skips.
Outside the branch: publish's create_assessment_item has no per-item catch either, so one unconvertible item still fails the whole exercise archive. Pre-existing, not touched here — say if you want it in this PR.
|
|
||
| def consolidate(self, items, queryset): | ||
| for item in items: | ||
| language = item.pop("contentnode__language__lang_code") |
There was a problem hiding this comment.
(Ported from rtibblesbot on #6095) Publish uses the channel lang_code; convert_legacy_question_to_qti hardcodes "en" — frozen on first write.
There was a problem hiding this comment.
Fixed: the read path now tags each item with its content node's lang_code, falling back to the channel's — the same chain as _node_language() and default_language in archive.py, so the API hands out what publish writes. Both come off the queryset: the node join in values, the channel as a subquery on main_tree__tree_id. Tests cover a node language, the channel fallback, per-node languages across a contentnode__in read, and the en default when neither has one.
Searched for other hardcoded languages on this path: convert_legacy_question_to_qti's or "en" is the only one, and it is now reached only when node and channel are both unset.
| # qti-item-body does not accept; empty P so an untyped question still | ||
| # renders as an editable paragraph. | ||
| body = _create_html_content_from_text(item.question) or [P()] | ||
| return Div(children=body), None |
There was a problem hiding this comment.
(Ported from rtibblesbot on #6095) No interaction in the Div body: the card reports NO_INTERACTION forever, and feeds the wipe at QTIItemEditor/index.vue:140.
There was a problem hiding this comment.
We should have a basic empty choice interaction if no questions or answers exist, similar to what we create for a new empty interaction on #6095
There was a problem hiding this comment.
Done. An answerless choice item now converts to a qti-choice-interaction carrying the question as its prompt and one empty qti-simple-choice, plus its response declaration and response processing — what _defaultState() opens a new choice interaction with in #6095. The Div body with no interaction is gone, so nothing reports NO_INTERACTION and the wipe at QTIItemEditor/index.vue:140 has no trigger. Golden fixture updated.
Checked the other converter paths that could produce a body without an interaction: text entry always emits one, and so does the Perseus custom interaction. This branch was the only one.
4131050 to
e14cecb
Compare
There was a problem hiding this comment.
@rtibblesbot, I'm still getting errors for the s element. Yes, it should be supported. Also, a change for link elements: we won't provide support for links anymore. These items are intended to be rendered on devices without access to the internet, so please, instead of transforming them into links, strip them in the conversion to QTI (keep the word, but remove the link, i.e., [a](b.com) -> a), and revert support for links on the pydantic models.
|
Swept the class rather than the tag: rendered a corpus of every construct the One consequence needs your call: Links — stripped in the conversion, text kept. Applied to the rendered markup rather than the markdown tokens, so a raw-HTML
|
|
@rtibblesbot, ah, I didn't know it was not supported by the QTI XSD. Let's not provide support for this either, and let's strip them out just like links, then ( |
6ad8254 to
5c34bdb
Compare
|
Strikethrough — stripped like links now: Swept the class rather than the tag: rendered every construct the
|
Inline maths: li, td, th, caption, dt, dd and qti-simple-choice took no math, though the XSD admits m3:math in all seven. Links and inline marks (a, s, del, ins, u, mark, strike): unwrapped in the rendered markup, keeping their text. An anchor has nothing to navigate to on a device with no internet access, and the QTI 3.0 HTML profile has no element for the marks. Publish and ricecooker upload reach the same converter, so both stop failing on these items too.
…ction Every newly added question is one: the editor writes type single_selection with no answers, and the XSD requires at least one qti-simple-choice, so conversion raised. Stand in the single empty choice the QTI editor opens a new choice interaction with.
consolidate() replaces each legacy row's type and raw_data with the converter's output, so the client only ever sees QTI. QTI and perseus_question rows pass through. Items are tagged with the content node's language, falling back to the channel's, matching what publish writes. An item that cannot be converted is logged and left as the legacy row it is, so one bad row does not cost the whole contentnode__in list; the editor renders it as unsupported. Goes away with the global backfill (learningequality#6007).
5c34bdb to
255bd5f
Compare
2 participants
Summary
The assessment item endpoint returned still-legacy rows as stored, so every client had to understand the five legacy question shapes alongside QTI. The read path now converts each still-legacy row to
type='QTI'with the item XML inraw_data. QTI and Perseus rows pass through as stored, and a converted item is tagged with the barelang_codeof its content node's language, falling back to the channel's, matching publish. A row that cannot be converted is logged and left legacy, so one bad item does not cost the rest of the list.Two converter fixes were needed to get there, and both also apply to publish and ricecooker upload:
qti-simple-choice, the same shape the QTI editor opens a new interaction with.li,td,th,caption,dt,ddandqti-simple-choiceaccepted nomath, though the XSD admits it in all seven. Links and the inline marks (s,del,ins,u,mark,strike) are unwrapped in the rendered markup instead of modelled — an anchor has nothing to navigate to offline, and the QTI 3.0 HTML profile has no element for a mark. Their text survives.References
Closes #6030. All four acceptance criteria are met; nothing is deferred. The AC-to-test mapping in this comment still holds except for its failure-handling row, which review superseded — see Deviations. The boxes on the issue are unticked only because
rtibblesbotlacksUpdateIssue— please tick them on merge. Consumes the converter from #6003; removed again by the backfill in #6007.Deviations from the issue spec
logger.exceptionplus the row staying legacy, which the editor renders as unsupported — not by raising. Raising 500s a wholecontentnode__inlist on one bad row; log-and-skip per item was requested in review and matches the publish precedent on #6029.Reviewer guidance
AssessmentTab.vue:21still mounts the legacyAssessmentEditor, and a converted row now arrives withquestion="",answers="[]",hints="[]". Can this merge before the QTI editor replaces it, given that editor writes those fields back throughupdateAssessmentItems?viewsets/assessmentitem.pyadds two language keys to the viewset'svalues— one a join, one aChannelsubquery — and pops both off inconsolidate. Does anything else consumingself.valuessee them?fixtures/single_selection_no_answers.xmlis a golden fixture, not a test —test_single_selection_no_answersasserts converter output against it, verified failing on a one-character mutation.AI usage
Used Claude Code to implement this from a plan agreed on the issue beforehand, writing the failing tests first. Verified with the assessment item viewset suite, the QTI utils suite, the exercise archive suite, the publish and channel creation suites, and pre-commit.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-08-19 22:29 UTC