feat: add all applied controls during mapping #2666
Conversation
Walkthroughcompute_requirement_assessments_results now returns a tuple: (list of RequirementAssessment, dict mapping each RequirementAssessment to source reference IDs). ComplianceAssessmentViewSet.perform_create unpacks the tuple, derives baseline assessments from the mapping, sets observations from selected baselines, and builds M2M relations from the mapped baseline items. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant View as ComplianceAssessmentViewSet
participant Model as RequirementAssessmentModel
participant DB as Database
Client->>View: POST create ComplianceAssessment
View->>Model: compute_requirement_assessments_results(mapping_set, source_assessment)
Model-->>View: (computed_assessments, assessment_source_dict)
Note over View: derive source IDs via assessment_source_dict\nquery baseline assessments (prefetch evidences, applied_controls)
View->>DB: Query baseline assessments by source IDs
DB-->>View: Baseline assessments
loop For each computed assessment
View->>View: select selected_source_id from assessment_source_dict
View->>View: selected_baseline_ra = baseline_assessments[selected_source_id]
View->>View: set observation = selected_baseline_ra.observation
View->>View: build evidences from selected_baseline_ra.evidences
View->>View: build applied_controls from chain of mapped baseline assessments
end
View->>DB: Bulk create/update assessments and M2M relations
DB-->>View: Persisted entities
View-->>Client: Created assessment payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (8)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/core/models.py(3 hunks)backend/core/views.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/core/views.py (1)
backend/core/models.py (2)
compute_requirement_assessments_results(5161-5249)applied_controls(5979-5980)
⏰ 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). (8)
- GitHub Check: build (3.12)
- GitHub Check: build_community_frontend
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: build_enterprise_frontend
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: test (3.12)
🔇 Additional comments (3)
backend/core/models.py (3)
5161-5165: LGTM! Method signature enhanced to return source mapping metadata.The updated signature correctly returns both the computed assessments and a dict tracking which source assessments contributed to each. This enables the caller to retrieve all related baseline objects (evidences, applied_controls) from the appropriate source assessments.
5230-5232: LGTM! Correctly captures all source assessment IDs.The code properly stores ALL source requirement assessment IDs that contributed to the mapping, not just the one selected for inference. This is the intended behavior to enable the caller to aggregate evidences and applied_controls from all related baseline assessments.
5249-5249: LGTM! Return statement matches updated signature.The method correctly returns the tuple as declared in the signature. Per the AI summary, the caller in
backend/core/views.pyhas been updated to unpack and use both values appropriately.
| ( | ||
| requirement_assessment, | ||
| baseline_ra.evidences.all(), | ||
| baseline_ra.applied_controls.all(), | ||
| selected_baseline_ra.evidences.all(), | ||
| itertools.chain.from_iterable( | ||
| requirement_assessment.applied_controls.all() | ||
| for requirement_assessment in baseline_requirement_assessments | ||
| ), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Fix generator capturing the wrong applied controls
itertools.chain.from_iterable(...) captures baseline_requirement_assessments by reference. Because you append that generator to m2m_operations and only iterate it after the outer loop finishes, every stored generator runs with baseline_requirement_assessments set to whatever the last iteration assigned. Earlier requirement assessments therefore end up copying the applied controls of the final baseline mapping, which is a regression. Materialize the union while you’re still inside the loop (e.g. build a list or set of control IDs) and store that concrete collection instead of the lazy generator.
Suggested fix:
- m2m_operations.append(
- (
- requirement_assessment,
- selected_baseline_ra.evidences.all(),
- itertools.chain.from_iterable(
- requirement_assessment.applied_controls.all()
- for requirement_assessment in baseline_requirement_assessments
- ),
- )
- )
+ baseline_control_ids = {
+ ac.id
+ for baseline_ra in baseline_requirement_assessments
+ for ac in baseline_ra.applied_controls.all()
+ }
+ m2m_operations.append(
+ (
+ requirement_assessment,
+ selected_baseline_ra.evidences.all(),
+ baseline_control_ids,
+ )
+ )
@@
- for assessment, evidences, controls in m2m_operations:
- assessment.evidences.add(*[ev.id for ev in evidences])
- assessment.applied_controls.add(*[ac.id for ac in controls])
+ for assessment, evidences, control_ids in m2m_operations:
+ assessment.evidences.add(*[ev.id for ev in evidences])
+ if control_ids:
+ assessment.applied_controls.add(*control_ids)📝 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.
| ( | |
| requirement_assessment, | |
| baseline_ra.evidences.all(), | |
| baseline_ra.applied_controls.all(), | |
| selected_baseline_ra.evidences.all(), | |
| itertools.chain.from_iterable( | |
| requirement_assessment.applied_controls.all() | |
| for requirement_assessment in baseline_requirement_assessments | |
| ), | |
| ) | |
| ) | |
| baseline_control_ids = { | |
| ac.id | |
| for baseline_ra in baseline_requirement_assessments | |
| for ac in baseline_ra.applied_controls.all() | |
| } | |
| m2m_operations.append( | |
| ( | |
| requirement_assessment, | |
| selected_baseline_ra.evidences.all(), | |
| baseline_control_ids, | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In backend/core/views.py around lines 5989 to 5997 the code appends an
itertools.chain.from_iterable(...) generator that closes over
baseline_requirement_assessments, causing all stored generators to reflect the
final loop value; instead, inside the loop materialize the applied-controls
collection (e.g., build a list or set of control IDs or control instances from
requirement_assessment.applied_controls.all() or from the baseline mapping) and
append that concrete collection to m2m_operations so later iteration uses the
correct, per-iteration controls rather than a late-bound generator.
Mohamed-Hacene
changed the title
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor