Skip to content

Comments

Fix exercise extra_fields migration for non-m_of_n mastery models#5714

Merged
rtibbles merged 6 commits intolearningequality:hotfixesfrom
rtibbles:exercise_your_rights
Feb 20, 2026
Merged

Fix exercise extra_fields migration for non-m_of_n mastery models#5714
rtibbles merged 6 commits intolearningequality:hotfixesfrom
rtibbles:exercise_your_rights

Conversation

@rtibbles
Copy link
Member

Summary

migrate_extra_fields was carrying over non-null m/n values for non-m_of_n mastery models (e.g. do_all, num_correct_in_a_row). The mastery criteria JSON schema requires m and n to be null for these models, causing frontend AJV validation to fail and exercises to appear incomplete — even though backend mark_complete passed them (it skips schema validation for old-style data).

Fixing just the m/n nulling in migrate_extra_fields would have been sufficient to resolve the immediate bug. However, to avoid having to worry about old-style exercise data ever again, this PR also:

  • Fixes the ricecooker import path (create_node) to run migrate_extra_fields on incoming exercise data before writing to DB
  • Adds a management command fix_exercise_extra_fields to repair existing data in production — both already-migrated exercises with invalid m/n and any remaining old-style exercises that haven't been migrated yet

Verification: all new and existing tests pass (test_contentnodes.py, test_views_internal.py).

References

Fixes #5667

Reviewer guidance

  • utils/nodes.py:528-530 — the core fix: 3 lines that null out m/n for non-m_of_n models. This runs on every exercise API read/write via consolidate_extra_fields and ExtraFieldsSerializer.update, so correctness here is critical. Only m_of_n should preserve m/n values; all other mastery models should have them set to null.
  • views/internal.py:839-842 — adding migrate_extra_fields to the ricecooker import path. This runs before the existing completion_criteria.validate() block, so newly-migrated data gets validated too. Test coverage confirms new-style extra_fields pass through unchanged.
  • Management command — iterates all exercises using iterator() with Python-side filtering to avoid expensive nested JSON queries. Individual save(update_fields=...) per affected node. Safe for large datasets but will be slow on production (millions of exercises). Run with --dry-run first to verify counts.

rtibbles and others added 2 commits February 19, 2026 16:34
Old-style exercise extra_fields were being migrated with non-null m/n
values for non-m_of_n mastery models (e.g. do_all, num_correct_in_a_row).
The mastery criteria JSON schema requires m and n to be null for these
models, causing frontend AJV validation to fail and exercises to appear
incomplete.

- Fix migrate_extra_fields to set m/n to null for non-m_of_n models
- Add management command fix_exercise_extra_fields to repair existing
  data in production

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ricecooker import path (create_node) was writing old-style
extra_fields directly to the DB without running migration. This
ensures incoming exercise data is migrated to the new-style
options.completion_criteria format before being persisted.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
not extra_fields.get("options", {}).get("completion_criteria", {})
and mastery_model is not None
):
if mastery_model != exercises.M_OF_N:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the core of the fix, really - m and n should be None if we don't have an exercises.M_OF_N mastery model.

@rtibbles
Copy link
Member Author

Updated!

Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

LGTM! Tests have good coverage over critical logic. Dry run logic and command logging makes sense

@rtibbles rtibbles merged commit c7120d5 into learningequality:hotfixes Feb 20, 2026
13 checks passed
@rtibbles rtibbles deleted the exercise_your_rights branch February 20, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants