-
Notifications
You must be signed in to change notification settings - Fork 307
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.
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
fix: mapping documentation score transfer when cloning #1512
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes add a new optional integer field, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ViewSet
participant ComplianceAssessment
participant RequirementAssessment
participant Baseline
Client->>ViewSet: POST new assessment
ViewSet->>ComplianceAssessment: perform_create(serializer)
alt Baseline provided & matching framework
ComplianceAssessment->>Baseline: Retrieve show_documentation_score
ComplianceAssessment->>ComplianceAssessment: Set show_documentation_score
else Baseline provided but different framework
ComplianceAssessment->>ComplianceAssessment: Handle alternate condition (elif case)
end
ComplianceAssessment->>RequirementAssessment: create_requirement_assessments(baseline)
alt Baseline exists
RequirementAssessment->>Baseline: Copy documentation_score
end
RequirementAssessment->>ComplianceAssessment: Return updated assessment data
ComplianceAssessment->>Client: Respond with new assessment
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (4)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/core/models.py (1)
2988-2990
: Consider explicit null handling for documentation scores.While the code correctly includes the documentation score in the global score calculation when enabled, it could benefit from more explicit null handling to avoid potential issues.
Consider this refinement:
if self.show_documentation_score: - score += ras.documentation_score or 0 + if ras.documentation_score is not None: + score += ras.documentation_score + n += 1 - n += 1This change:
- Only increments the counter when a valid documentation score exists
- Makes the null handling more explicit and intentional
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/core/models.py
(2 hunks)backend/core/views.py
(1 hunks)
🔇 Additional comments (4)
backend/core/models.py (2)
3448-3449
: LGTM! Well-defined field declaration.The
documentation_score
field is correctly defined as an optional integer field with appropriate verbose name.
2930-2934
: LGTM! Proper handling of documentation score in baseline assessment copying.The code correctly:
- Copies the documentation score from the baseline assessment with null safety
- Includes the field in bulk update operations
Also applies to: 2956-2956
backend/core/views.py (2)
3537-3540
: LGTM! Proper handling of documentation score inheritance.The code correctly copies the documentation score setting from the baseline when frameworks match, ensuring consistent documentation score behavior.
3542-3542
: LGTM! Improved control flow with elif.The change to
elif
correctly ensures mutual exclusivity with the previous condition, making the control flow clearer and more maintainable.
Summary by CodeRabbit