Skip to content

Conversation

@martinzerty
Copy link
Contributor

@martinzerty martinzerty commented Jan 27, 2026

base progress calculation depending on the implementation groups of the audit

Summary by CodeRabbit

  • Bug Fixes
    • Improved compliance assessment progress display: progress now chooses the most accurate source—uses group-based progress when present, otherwise falls back to stored progress or computes progress as needed.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2026

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

📝 Walkthrough

Walkthrough

The ComplianceAssessmentReadSerializer's progress field was changed from a ReadOnlyField to a SerializerMethodField, and a get_progress(self, obj) method was added to compute progress using selected_implementation_groups, the object's progress attribute, or obj.get_progress() as a fallback.

Changes

Cohort / File(s) Summary
Serializer Progress Computation
backend/core/serializers.py
Replaced progress ReadOnlyField with a SerializerMethodField and added get_progress(self, obj) implementing conditional logic: if obj.selected_implementation_groups is truthy return obj.get_progress(), else return obj.progress if present, otherwise fallback to obj.get_progress().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through lines of serializer light,
🥕 Swapped a field to make progress bright,
🌱 A gentle method checks groups and state,
🌙 Falls back softly — tidy and straight,
✨ Hooray for small hops that make code right.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adjusting progress calculation to depend on implementation groups, which aligns with the code changes modifying the progress field logic in ComplianceAssessmentReadSerializer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@backend/core/serializers.py`:
- Line 1846: The progress calculation is reading the raw
obj.selected_implementation_groups while the serialized
selected_implementation_groups uses its getter, causing inconsistencies; update
get_progress(self, obj) to use the serializer's getter (e.g., call
self.get_selected_implementation_groups(obj) or the same method used to populate
the selected_implementation_groups SerializerMethodField) instead of
obj.selected_implementation_groups so both the serialized groups and progress
respect the framework's implementation_groups_definition.

source="get_selected_implementation_groups"
)
progress = serializers.ReadOnlyField()
progress = serializers.SerializerMethodField()
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the specific lines in the serializer
head -n 1900 backend/core/serializers.py | tail -n 60

Repository: intuitem/ciso-assistant-community

Length of output: 2179


🏁 Script executed:

# Find and inspect the ComplianceAssessment model methods
rg -n "def get_selected_implementation_groups|def get_progress|def get_requirement_assessments" backend/core/models.py -A 5

Repository: intuitem/ciso-assistant-community

Length of output: 968


🏁 Script executed:

# Check how selected_implementation_groups is used in the model
rg -n "selected_implementation_groups" backend/core/models.py -B 2 -A 2

Repository: intuitem/ciso-assistant-community

Length of output: 4822


Selected implementation groups may not align with progress calculation.

The serialized selected_implementation_groups field uses the getter, which returns [] when the framework lacks implementation_groups_definition. However, get_progress() checks the raw obj.selected_implementation_groups field directly, bypassing that same framework-level check.

This creates a divergence: if the raw field is populated but the framework definition is missing, the progress calculation filters requirements using the raw field while the API exposes an empty groups list. The progress value becomes inconsistent with the selected groups shown to the client.

Use the getter consistently in get_progress() to ensure both the serialized field and progress calculation respect the framework's implementation groups definition:

🔧 Proposed adjustment
 def get_progress(self, obj):
-    if obj.selected_implementation_groups:
-        return obj.get_progress()
-    if hasattr(obj, "progress"):
-        return obj.progress
-    return obj.get_progress()
+    selected_groups = obj.get_selected_implementation_groups()
+    if selected_groups:
+        return obj.get_progress()
+    return obj.get_progress()
🤖 Prompt for AI Agents
In `@backend/core/serializers.py` at line 1846, The progress calculation is
reading the raw obj.selected_implementation_groups while the serialized
selected_implementation_groups uses its getter, causing inconsistencies; update
get_progress(self, obj) to use the serializer's getter (e.g., call
self.get_selected_implementation_groups(obj) or the same method used to populate
the selected_implementation_groups SerializerMethodField) instead of
obj.selected_implementation_groups so both the serialized groups and progress
respect the framework's implementation_groups_definition.

@martinzerty
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants