fix: correct applied_control 'impact' attribute for the csv export by martinzerty · Pull Request #3784 · intuitem/ciso-assistant-community

martinzerty · 2026-03-26T09:11:54Z

Replace 'impact' attribute of the applied controls by the one in the db called 'control_impact' in the csv export.
Before, we looked for the attribute named 'impact'. However, as it does not exist, the result for this attribute in the csv export was always empty

Summary by CodeRabbit

Bug Fixes
- Exported action-plan files (CSV/XLSX/Excel) now show the corrected "Control impact" column with accurate values.
- Removed an extra blank row in XLSX exports for cleaner output.
- Fixed spacing in the review action label so its display text appears correctly.

… the db called 'control_impact' in the csv export

coderabbitai · 2026-03-26T09:12:25Z

📝 Walkthrough

Walkthrough

Exported action-plan outputs now source the "Control impact" value from control_impact (previously impact) across Excel/CSV/XLSX; the export header label was changed to "Control impact". A DRF action name string was altered (extra spacing introduced) and a stray blank line was removed from the XLSX action output generation.

Changes

Cohort / File(s)	Summary
Action-plan export & DRF action `backend/core/views.py`	Replaced uses of `item.get("impact")` with `item.get("control_impact")`; changed Excel/CSV/XLSX header label from `"Impact"` to `"Control impact"`; export field mapping now defines `control_impact` sourced from `get_control_impact_display`; adjusted `to_review` DRF action `name` string (extra spacing introduced); removed one extra blank line in XLSX output generation.
Tooling cleanup `tools/convert_v1_to_v2.py`	Removed a commented debug `print` line inside `build_converted_workbook` (no functional change).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nibbled lines where headers slept,
Swapped impact for control_impact — softly crept,
A tidy name got a quirky space,
One blank line hopped from its place,
Exports now hum with gentler pace.

🚥 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 directly addresses the main change: correcting the 'impact' attribute in CSV export to use the proper database field name.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch CA-1630-export-plan-daction-champ-impact-vide

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.}

coderabbitai

🧹 Nitpick comments (1)

backend/core/views.py (1)
9714-9714: Correct CSV key fix; consider matching XLSX key usage too.

Good fix—control_impact matches the serializer output and resolves the empty CSV “Impact” column.
For consistency, please also update Line 9757 in action_plan_xlsx, which still reads item.get("impact").
Proposed parity change
-                "impact": item.get("impact"),
+                "impact": item.get("control_impact"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/core/views.py` at line 9714, The CSV fix uses
item.get("control_impact") to match the serializer; make the XLSX output
consistent by changing the usage in the action_plan_xlsx logic from
item.get("impact") to item.get("control_impact") so the "Impact" column in the
Excel export shows the same value as the CSV/serializer; locate the
action_plan_xlsx function in backend/core/views.py and replace the
item.get("impact") occurrence with item.get("control_impact").

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/core/views.py`:
- Line 9714: The CSV fix uses item.get("control_impact") to match the
serializer; make the XLSX output consistent by changing the usage in the
action_plan_xlsx logic from item.get("impact") to item.get("control_impact") so
the "Impact" column in the Excel export shows the same value as the
CSV/serializer; locate the action_plan_xlsx function in backend/core/views.py
and replace the item.get("impact") occurrence with item.get("control_impact").

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e17b160c-d9f5-4ce6-bdcc-b2243ca69afb

📥 Commits

Reviewing files that changed from the base of the PR and between f5ee641 and c98ef7b.

📒 Files selected for processing (1)

backend/core/views.py

Mohamed-Hacene · 2026-03-26T15:30:56Z

backend/core/views.py

We have the same issue here

and remove debug code

coderabbitai

♻️ Duplicate comments (1)

backend/core/views.py (1)

6305-6305: ⚠️ Potential issue | 🟡 Minor

Fix typo in action display name.

The name string has an accidental split word ("ac ceptances"), which leaks into API docs/UI metadata.

✏️ Suggested fix

-    `@action`(detail=False, name="Get ac  ceptances to review")
+    `@action`(detail=False, name="Get acceptances to review")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/core/views.py` at line 6305, Fix the typo in the action decorator's
display name: update the `@action`(...) annotation that currently reads name="Get
ac  ceptances to review" so the name string is corrected to "Get acceptances to
review" (locate the decorator on the view method where `@action`(detail=False,
name=...) is declared) to remove the extra space and ensure API docs/UI show the
proper label.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@backend/core/views.py`:
- Line 6305: Fix the typo in the action decorator's display name: update the
`@action`(...) annotation that currently reads name="Get ac  ceptances to review"
so the name string is corrected to "Get acceptances to review" (locate the
decorator on the view method where `@action`(detail=False, name=...) is declared)
to remove the extra space and ensure API docs/UI show the proper label.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b6a95016-f479-418d-83a1-f839cf0ac41e

📥 Commits

Reviewing files that changed from the base of the PR and between c2bc721 and ead08ee.

📒 Files selected for processing (2)

backend/core/views.py
tools/convert_v1_to_v2.py

💤 Files with no reviewable changes (1)

tools/convert_v1_to_v2.py

eric-intuitem

LGTM

fix: replace 'impact' attribute of the applied controls by the one in…

c98ef7b

… the db called 'control_impact' in the csv export

martinzerty self-assigned this Mar 26, 2026

martinzerty requested review from Mohamed-Hacene, ab-smith and nas-tabchiche March 26, 2026 09:12

coderabbitai bot reviewed Mar 26, 2026

View reviewed changes

Mohamed-Hacene requested changes Mar 26, 2026

View reviewed changes

backend/core/views.py Outdated

Copy link
Copy Markdown

Collaborator

Mohamed-Hacene Mar 26, 2026

Choose a reason for hiding this comment

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

We have the same issue here

martinzerty and others added 3 commits March 26, 2026 17:17

fix: forgot to patch all the occurences

c2bc721

Merge branch 'main' into CA-1630-export-plan-daction-champ-impact-vide

0394ad1

fix column name

ead08ee

and remove debug code

coderabbitai bot reviewed Mar 28, 2026

View reviewed changes

eric-intuitem approved these changes Mar 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct applied_control 'impact' attribute for the csv export#3784

fix: correct applied_control 'impact' attribute for the csv export#3784
martinzerty wants to merge 4 commits intomainfrom
CA-1630-export-plan-daction-champ-impact-vide

martinzerty commented Mar 26, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 26, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Mohamed-Hacene Mar 26, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

eric-intuitem left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

martinzerty commented Mar 26, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mohamed-Hacene Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

eric-intuitem left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martinzerty commented Mar 26, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 26, 2026 •

edited

Loading