Repository navigation
[BE] Skip foreign-key existence queries in CompetencyCriterion.save() #898
Description
User Story
As a course author bulk-editing competency criteria, I want the update to skip redundant database reads for each criterion, in order to keep large batches fast.
Description
CompetencyCriterion.save() calls full_clean() with no exclude. As a result, Django runs a separate SELECT on every save to check that each foreign key's row still exists. We need to exclude the foreign keys from that call and let the database's foreign-key constraints enforce them.
Context
- Follow-up from the review of feat: add bulk update competency criteria endpoint #874 (for [BE] Update/Edit Competency Criteria #759): feat: add bulk update competency criteria endpoint #874 (comment)
- Measured on feat: add bulk update competency criteria endpoint #874: each changed criterion costs 4 queries with override values and 5 with a named profile. Of those, 2 (3 with a profile) are
full_clean()looking up the foreign-key rows.
Acceptance Criteria
Scenario: Saving a criterion doesn't re-read the rows it points at
Given an existing criterion
When its rule is changed and it is saved
Then the save writes the criterion and its history record
And it does not read the criterion's group, tag association, or rule profile
Scenario: Bulk update cost no longer includes the foreign-key reads
Given a batch of criteria in one group whose rule the update changes
When the bulk update applies override values, and again when it names a rule profile
Then each changed criterion adds 2 queries to the batch, in both cases
Scenario: Override payload validation still runs on save
Given a criterion
When it is saved with override values whose payload doesn't fit the rule type
Then the save is refused with a validation error, as before
Scenario: A reference to a missing row is still refused
Given a criterion that points at a group, tag association, or rule profile that doesn't exist
When it is saved
Then the database refuses the writeTechnical Details
This is a suggested approach, not the source of truth. The User Story, Description, and Acceptance Criteria define what must be true when the work is done.
Approach
- In
CompetencyCriterion.save()insrc/openedx_learning/applets/cbe/models/criteria.py, passexclude=["group", "object_tag", "rule_profile"]tofull_clean()._validate_rule_source()inapi.pyalready uses the same exclude.clean()still runs, so override payload validation is unchanged. - Excluding a field also skips its not-null check. A dangling or missing foreign key now fails as an
IntegrityErrorfrom the database instead of aValidationErrorfromsave(). The API paths look up the group and tag before they build a criterion, so they shouldn't notice. Confirm that no caller catches theValidationErrorfor this case. - On SQLite and PostgreSQL, Django creates foreign-key constraints as
DEFERRABLE INITIALLY DEFERRED, so the database checks a dangling reference at commit, not atsave(). A test for the last scenario needs to force the check, for example withconnection.check_constraints(). MySQL checks immediately. - Pin the per-criterion cost with a
django_assert_num_queriestest onbulk_update_competency_criteria(): a fixed number of queries, plus 2 per changed criterion, for both override values and a named profile.
Additional context
CompetencyRuleProfile.save()in the same file also callsfull_clean()with noexclude(organization,course,competency_taxonomy). Profiles aren't saved in bulk, so it's out of scope here.- Django's
ForeignKey.validate(), which runs the lookup: https://github.com/django/django/blob/stable/5.2.x/django/db/models/fields/related.py
Activity
feat: add bulk update competency criteria endpoint (#874)