feat: Sync applied controls to their reference control #2897
Conversation
WalkthroughAdds an AppliedControl sync flag and makes ReferenceControl propagate category, csf_function and description updates to synced AppliedControl records; introduces a migration, frontend form checkbox and schema changes, and adds translation keys for the new UI text across locale files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as AppliedControlForm
participant API as Backend API
participant DB as Database
User->>UI: Edit AppliedControl (set reference_control)
UI->>API: Submit AppliedControl (includes reference_control)
API->>DB: Save ReferenceControl (create/update)
note right of DB `#f0f4c3`: ReferenceControl persisted
DB-->>API: Saved ReferenceControl
API->>DB: Query AppliedControls where reference_control=ID and is_synced_from_reference_control=True
DB-->>API: List of synced AppliedControls
API->>DB: bulk_update(category, csf_function) on matched AppliedControls
DB-->>API: bulk_update result
API->>DB: bulk_update(description) on matched AppliedControls
DB-->>API: bulk_update result
API-->>UI: Save response
UI-->>User: Show saved / synced state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/core/models.py (1)
1455-1494:ReferenceControl.is_deletablestill uses old reverse nameappliedcontrol_setYou changed
AppliedControl.reference_controlto userelated_name="applied_controls"(Line 3685), butReferenceControl.is_deletable()still checksself.appliedcontrol_set.exists()(Line 1492). With the new related_name,appliedcontrol_setno longer exists, so this will raiseAttributeErrorwheneveris_deletable()is called.Update the method to use the new reverse relation:
- def is_deletable(self) -> bool: + def is_deletable(self) -> bool: """ Returns True if the framework can be deleted """ - if self.requirements.exists() or self.appliedcontrol_set.exists(): + if self.requirements.exists() or self.applied_controls.exists(): return False return True
🧹 Nitpick comments (5)
frontend/messages/en.json (1)
2671-2674: New sync strings look good; optional wording polish.
Consider: “Updates fields when the reference control changes.”- "syncWithReferenceControlHelpText": "Updates fields on reference control update." + "syncWithReferenceControlHelpText": "Updates fields when the reference control changes."frontend/src/lib/components/Forms/ModelForm/AppliedControlPolicyForm.svelte (1)
72-75: Prefer rune-based derivation over manual store subscriptionThis works, but subscribing directly to
form.formand mutatinghasReferenceControlis less idiomatic than using a derived rune fromformStore, and it also avoids having to think about unsubscribe behavior.You can simplify and align with existing
$formStoreusage by replacing the manual subscription with a derived value:- let hasReferenceControl = $state(false); - form.form.subscribe((newFormData) => { - hasReferenceControl = Boolean(newFormData.reference_control); - }); + const hasReferenceControl = $derived(Boolean($formStore.reference_control));This keeps the flag in sync with the form store without extra wiring.
frontend/messages/da.json (1)
1262-1264: Danish phrasing nitConsider a more idiomatic help text: add “når” to improve flow.
- "syncWithReferenceControlHelpText": "Opdaterer felter ved opdatering af referencekontrol." + "syncWithReferenceControlHelpText": "Opdaterer felter, når referencekontrollen opdateres."frontend/messages/de.json (1)
989-991: German style tweak (optional)Slightly more natural help-text phrasing:
- "syncWithReferenceControlHelpText": "Aktualisiert Felder bei Aktualisierung der Referenzkontrolle." + "syncWithReferenceControlHelpText": "Aktualisiert Felder, wenn die Referenzkontrolle aktualisiert wird."backend/core/models.py (1)
1455-1485: Guard sync to AppliedControls so it only runs when fields actually changeThe
save()override always recomputes and bulk-updates all syncedAppliedControlrows, even whencategory,csf_function, anddescriptionare unchanged. On busy instances or heavily reused reference controls, this can generate a lot of unnecessary writes.Consider checking whether any of the synced fields actually changed before doing the propagation:
def save(self, *args, **kwargs): - old_description = "" - if self.pk: - old_reference_control = ReferenceControl.objects.filter(pk=self.pk).first() - if old_reference_control is not None: - old_description = old_reference_control.description - - return_value = super().save(*args, **kwargs) - - synced_applied_controls = self.applied_controls.filter(is_synced_from_reference_control=True) + old_description = "" + old_category = None + old_csf_function = None + if self.pk: + old_reference_control = ReferenceControl.objects.filter(pk=self.pk).first() + if old_reference_control is not None: + old_description = old_reference_control.description + old_category = old_reference_control.category + old_csf_function = old_reference_control.csf_function + + return_value = super().save(*args, **kwargs) + + # Short‑circuit if nothing relevant changed + if ( + not self.pk + or ( + old_category == self.category + and old_csf_function == self.csf_function + and old_description == self.description + ) + ): + return return_value + + synced_applied_controls = self.applied_controls.filter( + is_synced_from_reference_control=True + )This keeps behavior the same but avoids unnecessary bulk updates when saving unrelated fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
backend/core/migrations/0115_appliedcontrol_is_synced_from_reference_control_and_more.py(1 hunks)backend/core/models.py(2 hunks)frontend/messages/ar.json(1 hunks)frontend/messages/cs.json(1 hunks)frontend/messages/da.json(1 hunks)frontend/messages/de.json(1 hunks)frontend/messages/el.json(1 hunks)frontend/messages/en.json(2 hunks)frontend/messages/es.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/messages/hi.json(1 hunks)frontend/messages/hr.json(1 hunks)frontend/messages/hu.json(1 hunks)frontend/messages/id.json(1 hunks)frontend/messages/it.json(1 hunks)frontend/messages/nl.json(1 hunks)frontend/messages/pl.json(1 hunks)frontend/messages/pt.json(1 hunks)frontend/messages/ro.json(1 hunks)frontend/messages/sv.json(1 hunks)frontend/messages/tr.json(1 hunks)frontend/messages/uk.json(1 hunks)frontend/messages/ur.json(1 hunks)frontend/src/lib/components/Forms/ModelForm/AppliedControlPolicyForm.svelte(2 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Backend Linters
backend/core/migrations/0115_appliedcontrol_is_synced_from_reference_control_and_more.py
[error] 1-1: ruff format --check detected formatting changes needed. 2 files would be reformatted.
backend/core/models.py
[error] 1-1: ruff format --check detected formatting changes needed. 2 files would be reformatted.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (3.12)
- GitHub Check: test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: build_enterprise_frontend
- GitHub Check: build_community_frontend
- GitHub Check: Analyze (python)
🔇 Additional comments (20)
frontend/messages/nl.json (1)
985-987: Approve the Dutch translation additions — all language files updated consistently.The new translation keys follow established naming conventions and provide accurate Dutch translations for the reference control synchronization feature. JSON syntax is valid. Verification confirms all 21 language files contain the corresponding
syncWithReferenceControlentries, ensuring complete internationalization coverage.frontend/messages/it.json (1)
1359-1361: Translation additions are properly formatted and consistent across locales.The new
syncWithReferenceControlandsyncWithReferenceControlHelpTextkeys are correctly added to the Italian translation map with appropriate context-aware help text. The JSON structure remains valid with proper comma placement.frontend/messages/hr.json (1)
1948-1950: Translation additions are properly formatted and consistent across locales.The new
syncWithReferenceControlandsyncWithReferenceControlHelpTextkeys are correctly added to the Croatian translation map with context-aware help text that aligns with the feature intent. The JSON structure remains valid.frontend/messages/uk.json (1)
1496-1498: Translation additions are properly formatted and consistent across locales.The new
syncWithReferenceControlandsyncWithReferenceControlHelpTextkeys are correctly added to the Ukrainian translation map with context-aware help text. The JSON structure remains valid with proper comma placement, and the translations are semantically aligned with the reference control synchronization feature.frontend/messages/tr.json (1)
1715-1717: Turkish sync-with-reference-control translations look correct and consistentKey names and JSON syntax are valid, and the label/help text align with the new synchronization behavior.
frontend/messages/pt.json (1)
982-984: Portuguese sync-with-reference-control entries are syntactically and semantically alignedThe new keys follow existing naming conventions, JSON formatting is correct, and the text clearly describes syncing fields from the reference control.
frontend/messages/ro.json (1)
979-981: Romanian translations for sync-with-reference-control are consistent and correctKey names, punctuation, and wording all match the intended synchronization behavior and other locales.
frontend/messages/es.json (1)
989-991: Spanish sync-with-reference-control strings are well-formed and aligned with the featureJSON syntax is valid, the keys match the UI usage, and the label/help text correctly describe updating fields when the reference control changes.
frontend/messages/sv.json (1)
973-975: Swedish sync-with-reference-control translations are consistent and syntactically correctThe new keys integrate cleanly with existing entries, and the text accurately conveys syncing fields on reference control updates.
frontend/messages/ar.json (1)
951-952: LGTM (Arabic additions are clear and idiomatic).
No issues spotted.frontend/messages/en.json (1)
2671-2671: Minor: consistent tone with existing hints.
“Hint: more options can be defined through terminologies” reads fine. No change required.frontend/messages/el.json (1)
1680-1682: LGTM (Greek additions are accurate and consistent).
No issues.frontend/messages/ur.json (1)
950-952: New Urdu translations and JSON formatting look correctKeys and values are consistent with other locales and the JSON remains valid (proper commas, no structural issues).
frontend/src/lib/components/Forms/ModelForm/AppliedControlPolicyForm.svelte (1)
134-141: Checkbox wiring for reference-control sync is consistent and correctThe conditional rendering based on
hasReferenceControland the binding tois_synced_from_reference_controllook correct, and the label/helpText keys match the new i18n entries.frontend/messages/fr.json (1)
2673-2675: French translations and key additions are consistent
syncWithReferenceControland its help text are clearly worded, aligned with the feature, and the JSON structure (including the precedingwebhookUrl) remains valid.frontend/messages/pl.json (1)
1722-1724: Polish translations and new keys look good
viewLess,syncWithReferenceControl, and the help text are correctly added, semantically match the feature, and keep the JSON valid.frontend/messages/cs.json (1)
970-972: LGTMCzech translations are accurate and consistent.
frontend/messages/id.json (1)
1167-1169: All locales properly contain both keysVerification confirms all message files in
frontend/messages/define bothsyncWithReferenceControlandsyncWithReferenceControlHelpTextkeys. The changes are complete and consistent across all locales.frontend/src/lib/utils/schemas.ts (1)
157-209: AppliedControlSchema updates align with backend model
ref_idbeingoptional().nullable()and the newis_synced_from_reference_controlboolean with defaulttrueboth match the backend model (nullableCharField,BooleanField(default=True)) and existing schema patterns. No issues from a validation/typing perspective.backend/core/models.py (1)
3685-3693: Code changes approved—formatting resolvedThe new
related_name="applied_controls"linkage andis_synced_from_reference_controlBooleanField are consistent with the codebase. Ruff formatting has been successfully applied tobackend/core/models.py. All checks are now clear.
backend/core/migrations/0115_appliedcontrol_is_synced_from_reference_control_and_more.py
Show resolved
Hide resolved
| "dueDate": "Due date", | ||
| "startDate": "Start date", | ||
| "endDate": "End date", | ||
| "endDate": "End Date", |
There was a problem hiding this comment.
Keep sentence case for labels to match “Start date”.
Use “End date”.
- "endDate": "End Date",
+ "endDate": "End date",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "endDate": "End Date", | |
| "endDate": "End date", |
🤖 Prompt for AI Agents
In frontend/messages/en.json around line 212, the label "endDate": "End Date"
uses title case while "Start date" uses sentence case; update the value to "End
date" to match sentence case and maintain consistency across localization
labels.
| "syncWithReferenceControl": "संदर्भ नियंत्रणों के साथ सिंक करें", | ||
| "syncWithReferenceControlHelpText": "संदर्भ नियंत्रण अपडेट होने पर फ़ील्ड अपडेट करता है।" | ||
| } |
There was a problem hiding this comment.
Pluralization nit: use “फ़ील्ड्स” for “fields”.
Current text is singular.
- "syncWithReferenceControlHelpText": "संदर्भ नियंत्रण अपडेट होने पर फ़ील्ड अपडेट करता है।"
+ "syncWithReferenceControlHelpText": "संदर्भ नियंत्रण अपडेट होने पर फ़ील्ड्स अपडेट करता है।"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "syncWithReferenceControl": "संदर्भ नियंत्रणों के साथ सिंक करें", | |
| "syncWithReferenceControlHelpText": "संदर्भ नियंत्रण अपडेट होने पर फ़ील्ड अपडेट करता है।" | |
| } | |
| "syncWithReferenceControl": "संदर्भ नियंत्रणों के साथ सिंक करें", | |
| "syncWithReferenceControlHelpText": "संदर्भ नियंत्रण अपडेट होने पर फ़ील्ड्स अपडेट करता है।" | |
| } |
🤖 Prompt for AI Agents
In frontend/messages/hi.json around lines 952 to 954, the help text uses the
singular "फ़ील्ड" but should be plural; change "संदर्भ नियंत्रण अपडेट होने पर
फ़ील्ड अपडेट करता है।" to use the plural form "फ़ील्ड्स" (e.g., "संदर्भ नियंत्रण
अपडेट होने पर फ़ील्ड्स अपडेट करते/अपडेट होते हैं।" — pick the correct verb
agreement for consistency in the file) so the translation reads as plural
"fields" and matches the key intent.
| "viewLess": "Kevesebb megjelenítése", | ||
| "syncWithReferenceControl": "Szinkronizálás a referenciaellenőrzésekkel", | ||
| "syncWithReferenceControlHelpText": "Frissíti a mezőket a referenciaellenőrzés frissítésekor." |
There was a problem hiding this comment.
Terminology consistency: use “kontroll” (not “ellenőrzés”)
Elsewhere in this file you use “Referencia kontrollok”. Align these new strings to the same term.
- "syncWithReferenceControl": "Szinkronizálás a referenciaellenőrzésekkel",
- "syncWithReferenceControlHelpText": "Frissíti a mezőket a referenciaellenőrzés frissítésekor."
+ "syncWithReferenceControl": "Szinkronizálás a referencia kontrollokkal",
+ "syncWithReferenceControlHelpText": "Frissíti a mezőket a referencia kontroll frissítésekor."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "viewLess": "Kevesebb megjelenítése", | |
| "syncWithReferenceControl": "Szinkronizálás a referenciaellenőrzésekkel", | |
| "syncWithReferenceControlHelpText": "Frissíti a mezőket a referenciaellenőrzés frissítésekor." | |
| "viewLess": "Kevesebb megjelenítése", | |
| "syncWithReferenceControl": "Szinkronizálás a referencia kontrollokkal", | |
| "syncWithReferenceControlHelpText": "Frissíti a mezőket a referencia kontroll frissítésekor." |
🤖 Prompt for AI Agents
In frontend/messages/hu.json around lines 982 to 984, the Hungarian strings use
“referenciaellenőrzésekkel”/“ellenőrzés” but the project standard uses
“kontroll”; update the keys syncWithReferenceControl and
syncWithReferenceControlHelpText to replace “ellenőrzés” with “kontroll” (e.g.,
use “Referencia kontrollok” style and adjust surrounding wording/word-joining to
be grammatically correct).
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/core/models.py (1)
1488-1494: Fix reverse relation accessor inis_deletable()methodThe
AppliedControl.reference_controlForeignKey declaresrelated_name="applied_controls", makingself.appliedcontrol_setinvalid and causing anAttributeErrorat runtime.Update line 1491 in
backend/core/models.py:- if self.requirements.exists() or self.appliedcontrol_set.exists(): + if self.requirements.exists() or self.applied_controls.exists():
🧹 Nitpick comments (2)
backend/core/models.py (2)
1455-1487: ReferenceControl.save sync logic looks good but consider change-detection and downstream sync behaviorThe propagation of
category,csf_function, and (conditionally)descriptionto syncedAppliedControlinstances is correct and the use ofbulk_updateavoids per-row saves, which is good.Two follow-ups to consider:
- You already fetch
old_reference_control; you could also skip the bulk updates when neithercategory,csf_function, nordescriptionactually changed to avoid unnecessary writes on every save.- These
bulk_updatecalls bypassAppliedControl.save()(and thus_trigger_syncto external integrations). If external ITSM tickets are expected to reflect reference-control changes, you may want to explicitly trigger integration syncs for the affected applied controls, or clearly document that only directAppliedControledits propagate remotely.
3684-3692: related_name change and new sync flag: verify impact on existing data and callers
- Changing
reference_controltorelated_name="applied_controls"is a nice improvement, but it’s a breaking change for any existingreference_control.appliedcontrol_setusages (we saw one inReferenceControl.is_deletable). Please ensure all such call sites are updated or add a compatibility shim (e.g., a property delegatingappliedcontrol_settoapplied_controls) if needed.- The new
is_synced_from_reference_control = models.BooleanField(default=True, ...)means all existingAppliedControlrows will be treated as “synced” after the migration. Unless your migration explicitly flips this toFalsefor controls that already diverged from theirReferenceControl(custom category/CSF/description), the nextReferenceControl.save()may unexpectedly overwrite user customizations. It’s worth double-checking the migration or adding a follow-up data migration to set this flag based on current field differences.- Ruff is failing on this file; some of the new lines (e.g., the long field declaration) likely need wrapping. Please run
ruff format backend/core/models.py(or the project’s standard formatter) to satisfy the backend linters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/core/models.py(2 hunks)frontend/messages/en.json(2 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/lib/utils/schemas.ts
- frontend/messages/fr.json
- frontend/messages/en.json
🧰 Additional context used
🧬 Code graph analysis (1)
backend/core/models.py (1)
backend/core/tests/fixtures.py (1)
applied_controls(58-69)
🪛 GitHub Actions: Backend Linters
backend/core/models.py
[error] 1-1: Ruff format check failed: file would be reformatted by 'ruff format --check'. Run 'ruff format' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: build_community_frontend
- GitHub Check: build_enterprise_frontend
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
|
Don't think we need a migration or a sophisticated solution for this. We can consider multiple lightweight implementations:
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.