Skip to content

feat: Duplicate applied controls properly in mapping #2569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Duplicate applied controls properly in mapping #2569

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24f873c
feat: Duplicate applied controls properly in mapping
monsieurswag Sep 22, 2025
46c106f
Fix broken direct audit creation
monsieurswag Sep 22, 2025
169088e
Merge branch 'main' into feat/duplicate_controls_properly_in_mapping
monsieurswag Sep 24, 2025
ec31b10
Merge branch 'main' into feat/duplicate_controls_properly_in_mapping
monsieurswag Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 74 additions & 11 deletions backend/core/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5441,15 +5441,21 @@ def perform_create(self, serializer):
)

with transaction.atomic():
instance: ComplianceAssessment = serializer.save()
instance.create_requirement_assessments(baseline)
new_audit: ComplianceAssessment = serializer.save()
new_audit.create_requirement_assessments(baseline)

if baseline and baseline.framework == instance.framework:
instance.show_documentation_score = baseline.show_documentation_score
instance.save()
if baseline and baseline.framework == new_audit.framework:
new_audit.show_documentation_score = baseline.show_documentation_score
new_audit.save()

# Handle different framework case
elif baseline and baseline.framework != instance.framework:
elif baseline and baseline.framework != new_audit.framework:
duplicate_applied_controls = (
new_audit.folder != baseline.folder
and new_audit.folder not in baseline.folder.get_parent_folders()
)
duplicated_applied_controls: dict[str, AppliedControl] = {}

# Fetch mapping set and prefetch related data
mapping_set = RequirementMappingSet.objects.select_related(
"source_framework", "target_framework"
Expand All @@ -5459,8 +5465,10 @@ def perform_create(self, serializer):
)

# Compute results and get all affected requirement assessments
computed_assessments = instance.compute_requirement_assessments_results(
mapping_set, baseline
computed_assessments = (
new_audit.compute_requirement_assessments_results(
mapping_set, baseline
)
)

# Collect all source requirement assessment IDs
Expand Down Expand Up @@ -5505,14 +5513,69 @@ def perform_create(self, serializer):
RequirementAssessment.objects.bulk_update(updates, ["observation"])

# Handle M2M relationships in bulk
for assessment, evidences, controls in m2m_operations:
for assessment, evidences, applied_controls in m2m_operations:
assessment.evidences.add(*[ev.id for ev in evidences])
assessment.applied_controls.add(*[ac.id for ac in controls])

if duplicate_applied_controls:
new_applied_controls = []
for applied_control in applied_controls:
if (
duplicated_applied_control
:= duplicated_applied_controls.get(applied_control.id)
) is None:
duplicated_applied_control_data = {
field.name: getattr(applied_control, field.name)
for field in applied_control._meta.fields
if field.name
not in ["id", "pk", "created_at", "updated_at"]
}
duplicated_applied_control_data["folder"] = (
new_audit.folder
)

duplicated_applied_control, created = (
AppliedControl.objects.update_or_create(
defaults=duplicated_applied_control_data,
name=applied_control.name,
folder=new_audit.folder,
)
)
if created:
for field_name in [
"assets",
"evidences",
"filtering_labels",
"security_exceptions",
"objectives",
]:
duplicate_related_objects(
applied_control,
duplicated_applied_control,
new_audit.folder,
field_name,
)

duplicated_applied_control.owner.set(
applied_control.owner.all()
)
duplicated_applied_control.save()

duplicated_applied_controls[applied_control.id] = (
duplicated_applied_control
)
new_applied_controls.append(duplicated_applied_control)
assessment.applied_controls.add(
*(ac.id for ac in new_applied_controls)
)
else:
assessment.applied_controls.add(
*(ac.id for ac in applied_controls)
)
Comment on lines +5516 to +5573
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Avoid cross-folder evidence links; safer owner updates; simpler M2M adds.

  • Linking baseline evidences by ID can break folder-permission boundaries when duplicating across folders. Duplicate or link per helper policy instead.
  • Don’t overwrite owners on an already existing duplicate; union them to avoid clobbering.
  • Prefer passing model instances to add().
-                for assessment, evidences, applied_controls in m2m_operations:
-                    assessment.evidences.add(*[ev.id for ev in evidences])
+                for assessment, baseline_ra, evidences, applied_controls in m2m_operations:
+                    # Evidence: avoid cross-folder links when duplicating controls
+                    if duplicate_applied_controls:
+                        duplicate_related_objects(
+                            baseline_ra, assessment, new_audit.folder, "evidences"
+                        )
+                    else:
+                        assessment.evidences.add(*evidences)

@@
-                                duplicated_applied_control.owner.set(
-                                    applied_control.owner.all()
-                                )
+                                if created:
+                                    duplicated_applied_control.owner.set(
+                                        applied_control.owner.all()
+                                    )
+                                else:
+                                    duplicated_applied_control.owner.add(
+                                        *applied_control.owner.all()
+                                    )
@@
-                        assessment.applied_controls.add(
-                            *(ac.id for ac in new_applied_controls)
-                        )
+                        assessment.applied_controls.add(*new_applied_controls)
                     else:
-                        assessment.applied_controls.add(
-                            *(ac.id for ac in applied_controls)
-                        )
+                        assessment.applied_controls.add(*applied_controls)

Additionally, update_or_create keyed on (name, folder) may raise MultipleObjectsReturned if that pair isn’t unique. If uniqueness isn’t guaranteed, consider strengthening the lookup (e.g., include ref_id when present) or handling collisions.

🏁 Script executed:

#!/bin/bash
# Check for uniqueness constraints on AppliedControl (folder/name or similar)
rg -n -C3 'class\s+AppliedControl\b' backend/core/models.py
rg -n -C2 'unique_together|UniqueConstraint|unique\s*=' backend/core/models.py

Length of output: 1637

Avoid cross-folder evidence links; don't overwrite owners; handle update_or_create collisions.

  • Don’t link baseline evidences by ID when duplicating across folders — duplicate them instead (use duplicate_related_objects) rather than assessment.evidences.add(*[ev.id for ev in evidences]). (backend/core/views.py ~5405-5462)
  • When a duplicated AppliedControl already exists, union owners instead of overwriting: call .owner.set(...) only when created; otherwise .owner.add(*applied_control.owner.all()).
  • Pass model instances to .add() (assessment.applied_controls.add(*new_applied_controls)) instead of IDs.
  • Critical: update_or_create(..., name=applied_control.name, folder=new_audit.folder) can raise MultipleObjectsReturned — AppliedControl.name is non-unique and there’s no (name, folder) uniqueness constraint in backend/core/models.py (class AppliedControl ~line 2877). Strengthen the lookup (e.g., include ref_id), handle MultipleObjectsReturned, or add an explicit unique constraint + migration.
🤖 Prompt for AI Agents 
In backend/core/views.py around lines 5405-5462: when duplicating assessments
you currently link evidences by ID, overwrite owners, pass IDs into M2M.add, and
call update_or_create with a non-unique lookup which can raise
MultipleObjectsReturned. Fix by duplicating evidence objects via
duplicate_related_objects and add those duplicated instances to
assessment.evidences (not original IDs); when creating/looking up duplicated
AppliedControl, strengthen the lookup (e.g., include a stable unique field like
ref_id or another identifying field) or wrap update_or_create in a try/except to
catch MultipleObjectsReturned and disambiguate (or add a DB unique constraint +
migration beforehand); only call duplicated_applied_control.owner.set(...) when
a new AppliedControl was created, otherwise merge owners with
duplicated_applied_control.owner.add(*applied_control.owner.all()); and pass
model instances (not IDs) into
assessment.applied_controls.add(*new_applied_controls).


# Handle applied controls creation
if create_applied_controls:
# Prefetch all requirement assessments with their suggestions
assessments = instance.requirement_assessments.all().prefetch_related(
assessments = new_audit.requirement_assessments.all().prefetch_related(
"requirement__reference_controls"
)

Expand Down
Loading