Skip to content

Commit c7120d5

Browse files
rtibblesclaude
andauthored
Fix exercise extra_fields migration for non-m_of_n mastery models (#5714)
* Fix migrate_extra_fields to null m/n for non-m_of_n mastery models 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 <[email protected]> * Migrate old-style exercise extra_fields on ricecooker import 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 <[email protected]> * Cleanup management command and add more logging. * Target dry_run more focally for better info, cleanup redundant save. * Lazy loading, lazy coding. * Mark_complete returns an empty list if complete. --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent 7e8048f commit c7120d5

5 files changed

Lines changed: 516 additions & 1 deletion

File tree

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import json
2+
import logging as logmodule
3+
import time
4+
5+
from django.core.management.base import BaseCommand
6+
from le_utils.constants import content_kinds
7+
from le_utils.constants import exercises
8+
9+
from contentcuration.models import ContentNode
10+
from contentcuration.utils.nodes import migrate_extra_fields
11+
12+
logging = logmodule.getLogger("command")
13+
14+
CHUNKSIZE = 5000
15+
16+
17+
def _needs_m_n_fix(extra_fields):
18+
"""
19+
Check if already-migrated extra_fields have non-null m/n
20+
on a non-m_of_n mastery model.
21+
"""
22+
try:
23+
threshold = extra_fields["options"]["completion_criteria"]["threshold"]
24+
except (KeyError, TypeError):
25+
return False
26+
mastery_model = threshold.get("mastery_model")
27+
if mastery_model is None or mastery_model == exercises.M_OF_N:
28+
return False
29+
return threshold.get("m") is not None or threshold.get("n") is not None
30+
31+
32+
def _needs_old_style_migration(extra_fields):
33+
"""
34+
Check if extra_fields still has old-style top-level mastery_model.
35+
"""
36+
return isinstance(extra_fields, dict) and "mastery_model" in extra_fields
37+
38+
39+
class Command(BaseCommand):
40+
help = (
41+
"Fix exercise extra_fields that were migrated with invalid m/n values "
42+
"in their completion criteria threshold. Non-m_of_n mastery models "
43+
"require m and n to be null, but old data may have had non-null values "
44+
"that were carried over during migration. Also migrates any remaining "
45+
"old-style extra_fields to the new format."
46+
)
47+
48+
def add_arguments(self, parser):
49+
parser.add_argument(
50+
"--dry-run",
51+
action="store_true",
52+
help="Report what would be changed without modifying the database.",
53+
)
54+
55+
def handle(self, *args, **options):
56+
dry_run = options.get("dry_run", False)
57+
start = time.time()
58+
59+
# Single pass over all exercises, filtering in Python to avoid
60+
# expensive nested JSON field queries in the database.
61+
queryset = ContentNode.objects.filter(kind_id=content_kinds.EXERCISE)
62+
63+
total = ContentNode.objects.filter(kind_id="exercise").count()
64+
migrated_fixed = 0
65+
migrated_complete = 0
66+
old_style_fixed = 0
67+
old_style_complete = 0
68+
exercises_checked = 0
69+
70+
for node in queryset.iterator(chunk_size=CHUNKSIZE):
71+
fix_type, complete = self._process_node(node, dry_run)
72+
if fix_type == "old_style":
73+
old_style_fixed += 1
74+
if complete:
75+
old_style_complete += 1
76+
elif fix_type == "m_n_fix":
77+
migrated_fixed += 1
78+
if complete:
79+
migrated_complete += 1
80+
exercises_checked += 1
81+
if exercises_checked % CHUNKSIZE == 0:
82+
logging.info(
83+
"{} / {} exercises checked".format(exercises_checked, total)
84+
)
85+
logging.info(
86+
"{} marked complete out of {} old style fixed".format(
87+
old_style_complete, old_style_fixed
88+
)
89+
)
90+
logging.info(
91+
"{} marked complete out of {} migrated fixed".format(
92+
migrated_complete, migrated_fixed
93+
)
94+
)
95+
96+
logging.info("{} / {} exercises checked".format(exercises_checked, total))
97+
logging.info(
98+
"{} marked complete out of {} old style fixed".format(
99+
old_style_complete, old_style_fixed
100+
)
101+
)
102+
logging.info(
103+
"{} marked complete out of {} migrated fixed".format(
104+
migrated_complete, migrated_fixed
105+
)
106+
)
107+
logging.info(
108+
"Done in {:.1f}s. Fixed {} migrated exercises, "
109+
"migrated {} old-style exercises.{}".format(
110+
time.time() - start,
111+
migrated_fixed,
112+
old_style_fixed,
113+
" (dry run)" if dry_run else "",
114+
)
115+
)
116+
117+
def _process_node(self, node, dry_run):
118+
ef = node.extra_fields
119+
if isinstance(ef, str):
120+
try:
121+
ef = json.loads(ef)
122+
except (json.JSONDecodeError, ValueError):
123+
return None, None
124+
if not isinstance(ef, dict):
125+
return None, None
126+
127+
if _needs_old_style_migration(ef):
128+
ef = migrate_extra_fields(ef)
129+
fix_type = "old_style"
130+
elif _needs_m_n_fix(ef):
131+
ef["options"]["completion_criteria"]["threshold"]["m"] = None
132+
ef["options"]["completion_criteria"]["threshold"]["n"] = None
133+
fix_type = "m_n_fix"
134+
else:
135+
return None, None
136+
node.extra_fields = ef
137+
complete = not node.mark_complete()
138+
if not dry_run:
139+
node.save(update_fields=["extra_fields", "complete"])
140+
return fix_type, complete

0 commit comments

Comments
 (0)