Skip to content

feat: round-trip QTI AssessmentItem raw_data through save and sync#6028

Open
rtibblesbot wants to merge 2 commits into
learningequality:unstablefrom
rtibblesbot:issue-5999-a58df5
Open

feat: round-trip QTI AssessmentItem raw_data through save and sync#6028
rtibblesbot wants to merge 2 commits into
learningequality:unstablefrom
rtibblesbot:issue-5999-a58df5

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Extends AssessmentItem to store and round-trip an editor-authored QTI item's full XML in raw_data (type='QTI'), rather than the structured question/answers/hints fields legacy types use.

  • Adds QTI as a valid AssessmentItem.type
  • Persists raw_data verbatim (no whitespace trimming) and skips the legacy answers/hints JSON-shape coercion for QTI items
  • Validates raw_data against the QTI 3.0 schema during sync save, rejecting invalid XML per-item through the existing sync error channel
  • Extracts src/srcset/href/data checksum references from the QTI XML and wires them into the existing set_files claim/create/delete flow (mirrors the markdown-scan path legacy types already use)

Scoped to the sync API save path only, per the issue — ricecooker/internal API and QTI export/publish are unaffected.

References

Fixes #5999
Related: #5970 (QTI editor), #4877 (existing type validation)

Reviewer guidance

  • contentcuration/contentcuration/viewsets/assessmentitem.py: validate() raises from within is_valid() (not create()/update()) specifically so a QTI schema error attributes to one item in the sync error channel instead of tripping the batch's broad except Exception — worth confirming that reasoning holds
  • get_filenames_from_qti_item parses raw_data with the QTI validator's existing secure_parser() (XXE/entity-expansion hardened) — reused rather than re-implemented
  • test_qti_assessmentitem_full_round_trip covers byte-for-byte round-trip on create/update plus file extraction/removal
  • Full backend suite passes (no regressions)
  • 3 pre-existing FileThumbnailTestCase failures on unstable are unrelated to this change (reproduced against unmodified unstable)

AI usage

Implemented with Claude Code from a maintainer-approved plan, using TDD per task. Wiring up schema validation broke an earlier whitespace round-trip test — the test padded raw_data with leading whitespace, which placed content before the XML declaration and is itself invalid XML. Fixed the test to pad trailing whitespace only, which still exercises trim_whitespace=False without producing invalid input.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?
  • Ran pre-flight CI checks (lint, format, tests) and verified all pass
  • Rebased onto the target branch and resolved any conflicts
  • Reorganized commit history into clean, logical commits
  • Audited the diff to ensure only issue-relevant files are changed
  • Built PR body from the repository's PR template with evidence blocks

Status: 🟡 Waiting for feedback · updated 2026-07-03 08:11 UTC

@rtibblesbot rtibblesbot marked this pull request as ready for review July 3, 2026 06:28
rtibbles
rtibbles requested changes Jul 3, 2026
View reviewed changes

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to do the QTI constant properly - upgrade to le-utils 0.2.18.

Comment thread contentcuration/contentcuration/models.py Outdated
max_length=50,
choices=exercises.question_choices + (("true_false", "True/False"),),
choices=exercises.question_choices
+ (("true_false", "True/False"), ("QTI", "QTI")),

@rtibbles rtibbles Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be added by upgrading le-utils, not adding it manually.

@rtibblesbot rtibblesbot Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — bumped le-utils to 0.2.18 (ships exercises.QTI) and removed the manual "QTI" tuple entry.

Comment thread contentcuration/contentcuration/viewsets/assessmentitem.py Outdated
filenames = get_filenames_from_assessment(aitem)
filenames = (
get_filenames_from_qti_item(aitem)
if aitem.type == "QTI"

@rtibbles rtibbles Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be referencing the le utils constant, not hard coded.

@rtibblesbot rtibblesbot Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — now references exercises.QTI.

Comment thread contentcuration/contentcuration/viewsets/assessmentitem.py Outdated
@@ -135,14 +198,17 @@ def set_files(self, all_objects, all_validated_data=None): # noqa C901
md_fields_modified = {

@rtibbles rtibbles Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable name is now misleading "md_fields_modified" - should be changed to "editable_text_fields_modified"

@rtibblesbot rtibblesbot Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — renamed to editable_text_fields_modified.

Comment thread contentcuration/contentcuration/viewsets/assessmentitem.py Outdated
# except Exception in create_from_changes/update_from_changes and
# reported as "Internal server error" for the whole batch.
data = super(AssessmentItemSerializer, self).validate(data)
if self._item_type == "QTI":

@rtibbles rtibbles Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use constant.

@rtibblesbot rtibblesbot Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — now references exercises.QTI.

@rtibblesbot
chore(deps): bump le-utils from 0.2.17 to 0.2.18
65e898c 
Ships the QTI question type constant so AssessmentItem code can
reference exercises.QTI instead of a hardcoded string.
@rtibblesbot
feat: round-trip QTI AssessmentItem raw_data through save and sync
397f1f7 
Store the editor's authored QTI XML on AssessmentItem (type='QTI',
full item XML in raw_data) and round-trip it byte-for-byte through
the save and sync API paths.

- Persist raw_data verbatim, bypassing legacy answers/hints JSON
  coercion for QTI items
- Validate raw_data against the QTI 3.0 schema on save, rejecting
  invalid items through the sync error channel
- Extract src/srcset/href/data file references from QTI XML into the
  existing File claim/create/delete flow

Fixes learningequality#5999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rtibbles rtibbles rtibbles requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[QTI] Allow API endpoint to handle AssessmentItem.raw_data to store QTI data

2 participants

@rtibblesbot @rtibbles