feat: export audit as powerpoint deck #3136
Conversation
📝 WalkthroughWalkthroughAdds PPTX export end-to-end: backend generator produces PPTX-ready context and charts; viewset renders a PPTX template and streams the file; frontend adds a proxy route, UI export link, and translations; python-pptx added as a dependency. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Frontend UI
participant Route as SvelteKit Route
participant API as Backend ViewSet
participant Gen as Generator
participant PPTX as python-pptx
User->>UI: Click "Export as PPTX"
UI->>Route: GET /compliance-assessments/{id}/export/pptx
Route->>API: Fetch pptx_report(pk)
API->>Gen: gen_audit_pptx_context(pk, tree, lang)
Gen->>Gen: Compute metrics, category scores, controls
Gen-->>API: Return context + chart buffers
API->>PPTX: Load template & replace placeholders
API->>PPTX: Insert chart images and tables from buffers
PPTX-->>API: Render PPTX bytes
API-->>Route: Stream PPTX response
Route-->>User: Download exec_report.pptx
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 2
Fix all issues with AI Agents 🤖
In @backend/core/generators.py:
- Around line 794-797: safe_translate can raise KeyError for unsupported lang
codes; update it to first guard against None/"--" keys, then fetch the
translations mapping with i18n_dict.get(lang, i18n_dict.get("en", {})) before
calling .get(key, key) so unsupported languages fall back to English (or an
empty dict) instead of crashing. Apply the same defensive lookup pattern to
gen_audit_context to avoid KeyError when accessing i18n_dict[lang].
In
@frontend/src/routes/(app)/(internal)/compliance-assessments/[id=uuid]/export/pptx/+server.ts:
- Line 14: The generated filename uses new Date().toISOString() which contains
colons (and dots) invalid on Windows; update the fileName construction in
+server.ts to sanitize the timestamp (e.g., replace colons and dots with safe
characters or use Date.now()). For example, change the fileName variable
assignment (const fileName = ...) to use new
Date().toISOString().replace(/[:.]/g, '-') or use Date.now() so the resulting
string has no illegal characters before appending .pptx.
🧹 Nitpick comments (6)
frontend/src/routes/(app)/(internal)/compliance-assessments/[id=uuid]/export/pptx/+server.ts (2)
10-12: Propagate the original backend error status instead of hardcoding 400.The current implementation always returns a 400 error regardless of the actual backend response (could be 404, 403, 500, etc.). This makes debugging harder and provides inaccurate error information to the client.
🔎 Proposed fix
- if (!res.ok) { - error(400, 'Error fetching the PPTX file'); - } + if (!res.ok) { + error(res.status, 'Error fetching the PPTX file'); + }
16-22: TheTransfer-Encoding: chunkedheader is unnecessary.When passing a
ReadableStream(fromres.body) to theResponseconstructor, the runtime automatically handles chunked transfer encoding. Manually setting this header is redundant and could potentially cause issues with some HTTP/2 implementations where chunked encoding is not used.🔎 Proposed fix
return new Response(res.body, { headers: { 'Content-Type': 'application/vnd.openxmlformats-officedocument.presentationml.presentation', - 'Content-Disposition': `attachment; filename="${fileName}"`, - 'Transfer-Encoding': 'chunked' + 'Content-Disposition': `attachment; filename="${fileName}"` } });backend/core/generators.py (3)
646-712: Significant code duplication withgen_audit_context.The helper functions
count_category_results,aggregate_category_scores,i18n_dict, andsafe_translateare duplicated fromgen_audit_context(lines 333-519). This violates DRY and increases maintenance burden.Consider extracting these to module-level functions that both
gen_audit_contextandgen_audit_pptx_contextcan share.🔎 Suggested approach
# Move to module level (before gen_audit_context) I18N_DICT = { "en": { "compliant": "Compliant", # ... rest of translations }, "fr": { "compliant": "Conformes", # ... rest of translations }, } def safe_translate(lang: str, key: str) -> str: if key is None or key == "--": return "-" return I18N_DICT.get(lang, I18N_DICT["en"]).get(key, key) def count_category_results(data): # ... existing implementation pass def aggregate_category_scores(data): # ... existing implementation passThen both
gen_audit_contextandgen_audit_pptx_contextcan call these shared functions.
847-879: N+1 query performance issue when counting requirements per control.Each iteration queries the database separately to count
RequirementAssessmentobjects for each applied control. With many controls, this creates significant database overhead.🔎 Proposed optimization using annotation
from django.db.models import Count, Q # Replace the individual queries with a single annotated query applied_controls = AppliedControl.objects.filter( requirement_assessments__in=requirement_assessments_objects ).distinct().annotate( requirements_count=Count( 'requirement_assessments', filter=Q(requirement_assessments__compliance_assessment=audit) ) ) p1_controls = [] for ac in applied_controls.filter(priority=1): p1_controls.append({ "name": ac.name, "description": safe_translate(lang, ac.description), "status": safe_translate(lang, ac.status), "category": safe_translate(lang, ac.category), "coverage": ac.requirements_count, # Use annotated value }) full_controls = [] for ac in applied_controls: full_controls.append({ "name": ac.name, "description": safe_translate(lang, ac.description), "prio": f"P{ac.priority}" if ac.priority else "-", "status": safe_translate(lang, ac.status), "eta": safe_translate(lang, ac.eta), "category": safe_translate(lang, ac.category), "coverage": ac.requirements_count, # Use annotated value })Note: The same N+1 issue exists in
gen_audit_context(lines 572-605). Consider fixing both.
754-755: Missing error handling for empty requirements case.Unlike
gen_audit_context(line 474-475) which logs an error when no requirements are found (total == 0), this function silently continues. Consider adding similar handling for consistency and debugging.🔎 Proposed fix
total = sum([v for v in aggregated.values()]) + if total == 0: + print("Error:: No requirements found, something is wrong. aborting ..") aggregated["total"] = totalAlternatively, consider raising an exception or returning an error response instead of just logging.
backend/core/views.py (1)
7843-8115: Solid PPTX export flow; consider a couple of defensive tweaks around context handlingThe overall structure (language handling, template fallback, filtered tree, context generation, text/chart/table injection, streamed response) looks good and aligns with the existing
word_reportbehavior.Two areas worth tightening up:
- Robustness of
category_scoreshandling
table_category_scorescurrently assumescontext["category_scores"]is dict-like and calls.values(). The generator docstring describescategory_scoresas a “data list”, which may instead be a list of dicts; a mismatch would raise at runtime.To make this resilient to either a list or a dict (and to a missing key), you can normalize before iterating:
Suggested change for
table_category_scoresblock- "table_category_scores": { - "headers": ["Category", "Average Score", "Items", "Scored"], - "data": [ - [ - v["name"], - str(v["average_score"]), - str(v["item_count"]), - str(v["scored_count"]), - ] - for v in context["category_scores"].values() - ], - }, + "table_category_scores": { + "headers": ["Category", "Average Score", "Items", "Scored"], + "data": [ + [ + v.get("name", ""), + str(v.get("average_score", "")), + str(v.get("item_count", "")), + str(v.get("scored_count", "")), + ] + for v in ( + (context.get("category_scores") or {}).values() + if isinstance(context.get("category_scores"), dict) + else (context.get("category_scores") or []) + ) + ], + },
- Guarding against missing/
Nonechart buffersIf any of the chart buffers in
chart_imagescan beNonein some edge cases,item["chart_buffer"].seek(0)will raise. A small guard in the detection loop avoids hard failures:Example guard around chart placeholders
- for chart_name, chart_buffer in chart_images.items(): + for chart_name, chart_buffer in chart_images.items(): + if chart_buffer is None: + continue placeholder = "{{" + chart_name + "}}"Both are small, localized changes that make the export action more tolerant of context shape changes and partial data.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/core/templates/core/audit_report_template_en.pptxis excluded by!**/*.pptxbackend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
backend/core/generators.pybackend/core/views.pybackend/pyproject.tomlfrontend/messages/de.jsonfrontend/messages/en.jsonfrontend/messages/es.jsonfrontend/messages/fr.jsonfrontend/messages/it.jsonfrontend/messages/nl.jsonfrontend/src/routes/(app)/(internal)/compliance-assessments/[id=uuid]/export/pptx/+server.tsfrontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/routes/(app)/(internal)/compliance-assessments/[id=uuid]/export/pptx/+server.ts (1)
frontend/src/lib/utils/constants.ts (1)
BASE_API_URL(4-8)
backend/core/views.py (1)
backend/core/generators.py (2)
gen_audit_context(332-643)gen_audit_pptx_context(646-921)
backend/core/generators.py (1)
backend/core/models.py (3)
ComplianceAssessment(5711-6620)AppliedControl(4058-4498)RequirementAssessment(6623-6932)
⏰ 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). (9)
- GitHub Check: build (3.12)
- GitHub Check: build_community_frontend
- 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: enterprise-startup-docker-compose-test
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
🔇 Additional comments (9)
frontend/messages/nl.json (1)
269-272: asPPTX translation is consistent with existing export labelsThe new
"asPPTX": "als PowerPoint"string matches the pattern of"asPDF","asCSV", and"asWord"and is syntactically valid JSON. No further changes needed.frontend/messages/en.json (1)
354-359: English asPPTX label fits existing export options
"asPPTX": "as PowerPoint"is clear, consistent with nearby"asWord"/"asXLSX"labels, and JSON structure is correct. No changes required.backend/pyproject.toml (1)
9-53: python-pptx dependency aligns with new PPTX export featureAdding
python-pptx = "^1.0.2"under runtime dependencies is appropriate for the new PPTX report generation and is consistent with the existing document/report tooling in this project. Just ensure the corresponding lockfile and deployment images/environments are updated so this dependency (and its transitive ones, like Pillow) are available at runtime.frontend/messages/es.json (1)
269-272: PPTX label is consistent with existing export options
"asPPTX": "como PowerPoint"matches the pattern ofasPDF/asCSV/asWordand uses correct Spanish phrasing and capitalization. Looks good.frontend/messages/it.json (1)
269-272: Italian PPTX translation aligned with other formats
"asPPTX": "come PowerPoint"is consistent withasPDF/asCSV/asWordand is idiomatic Italian for the export option. No changes needed.frontend/messages/fr.json (1)
350-350: LGTM! Translation is correct and well-placed.The French translation "en PowerPoint" follows the same pattern as other export format translations (e.g., "en Word" on line 349) and is appropriately positioned among related export options.
frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte (1)
630-634: LGTM! PPTX export link is correctly implemented.The new PPTX export option:
- Follows the same URL pattern and styling as existing export options (CSV, XLSX, Word)
- Is correctly placed within the permission gate for non-third-party users
- Uses the proper translation function
m.asPPTX()- Integrates seamlessly with the existing export menu structure
frontend/messages/de.json (1)
272-272: LGTM!The German translation "als PowerPoint" follows the established naming convention (
asXXXpattern) and is consistent with the other export format translations (asPDF, asCSV, asWord).backend/core/views.py (1)
63-70: PPTX imports and generator wiring look appropriateThe new
pptximports andgen_audit_pptx_contextintegration are consistent with the existing Word report flow and stay nicely scoped to reporting. No issues spotted here.
| def safe_translate(lang: str, key: str) -> str: | ||
| if key is None or key == "--": | ||
| return "-" | ||
| return i18n_dict[lang].get(key, key) |
There was a problem hiding this comment.
safe_translate may fail for unsupported languages.
The function only supports 'en' and 'fr' in i18n_dict. If a different language code is passed, accessing i18n_dict[lang] will raise a KeyError.
🔎 Proposed fix with fallback
def safe_translate(lang: str, key: str) -> str:
if key is None or key == "--":
return "-"
- return i18n_dict[lang].get(key, key)
+ return i18n_dict.get(lang, i18n_dict["en"]).get(key, key)Note: The same issue exists in gen_audit_context at line 519.
🤖 Prompt for AI Agents
In @backend/core/generators.py around lines 794-797, safe_translate can raise
KeyError for unsupported lang codes; update it to first guard against None/"--"
keys, then fetch the translations mapping with i18n_dict.get(lang,
i18n_dict.get("en", {})) before calling .get(key, key) so unsupported languages
fall back to English (or an empty dict) instead of crashing. Apply the same
defensive lookup pattern to gen_audit_context to avoid KeyError when accessing
i18n_dict[lang].
| error(400, 'Error fetching the PPTX file'); | ||
| } | ||
|
|
||
| const fileName = `audit-exec-summary-${new Date().toISOString()}.pptx`; |
There was a problem hiding this comment.
Filename contains colons from ISO timestamp which are invalid on Windows.
The ISO timestamp format includes colons (e.g., 2024-01-05T20:13:30.000Z) which are invalid characters in Windows filenames. This could cause issues when users download the file on Windows systems.
🔎 Proposed fix to sanitize the filename
- const fileName = `audit-exec-summary-${new Date().toISOString()}.pptx`;
+ const fileName = `audit-exec-summary-${new Date().toISOString().replace(/:/g, '-')}.pptx`;📝 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.
| const fileName = `audit-exec-summary-${new Date().toISOString()}.pptx`; | |
| const fileName = `audit-exec-summary-${new Date().toISOString().replace(/:/g, '-')}.pptx`; |
🤖 Prompt for AI Agents
In
@frontend/src/routes/(app)/(internal)/compliance-assessments/[id=uuid]/export/pptx/+server.ts
around line 14, The generated filename uses new Date().toISOString() which
contains colons (and dots) invalid on Windows; update the fileName construction
in +server.ts to sanitize the timestamp (e.g., replace colons and dots with safe
characters or use Date.now()). For example, change the fileName variable
assignment (const fileName = ...) to use new
Date().toISOString().replace(/[:.]/g, '-') or use Date.now() so the resulting
string has no illegal characters before appending .pptx.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/core/generators.py (2)
683-735: Extract duplicated helper functions to module level.
count_category_results,aggregate_category_scores,i18n_dict, andsafe_translateare duplicated betweengen_audit_contextandgen_audit_pptx_context. Consider extracting these to module-level definitions to reduce maintenance burden and potential for drift.🔎 Suggested approach
# At module level (before gen_audit_context) I18N_DICT = { "en": { ... }, "fr": { ... }, } def _safe_translate(lang: str, key: str) -> str: if key is None or key == "--": return "-" return I18N_DICT.get(lang, I18N_DICT["en"]).get(key, key) def _count_category_results(data): # ... existing implementation def _aggregate_category_scores(data): # ... existing implementationThen both
gen_audit_contextandgen_audit_pptx_contextcan call these shared helpers.
870-902: N+1 query pattern for counting requirement assessments.Each iteration executes a separate
RequirementAssessment.objects.filter(...).count()query. For audits with many applied controls, this can degrade performance significantly.🔎 Consider using annotation
from django.db.models import Count # Annotate the count directly on the queryset applied_controls_annotated = applied_controls.annotate( requirements_count=Count( 'requirementassessment', filter=Q(requirementassessment__compliance_assessment=audit) ) ) for ac in applied_controls_annotated.filter(priority=1): p1_controls.append({ "name": ac.name, # ... "coverage": ac.requirements_count, })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/core/templates/core/audit_report_template_en.pptxis excluded by!**/*.pptx
📒 Files selected for processing (2)
backend/core/generators.pybackend/core/views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/core/views.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-10T17:04:11.726Z
Learnt from: nas-tabchiche
Repo: intuitem/ciso-assistant-community PR: 1332
File: frontend/src/lib/components/ModelTable/server/ModelTable.svelte:173-211
Timestamp: 2025-02-10T17:04:11.726Z
Learning: In the CISO Assistant codebase, the `unsafeTranslate` function is named "unsafe" because it does not fall back to the input string when the translation is undefined, unlike `safeTranslate`. The name does not indicate security concerns.
Applied to files:
backend/core/generators.py
⏰ 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). (9)
- GitHub Check: build (3.12)
- GitHub Check: test (3.12)
- GitHub Check: build_enterprise_frontend
- GitHub Check: build_community_frontend
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
backend/core/generators.py (2)
100-131: LGTM!The donut chart improvements are well-implemented: conditional percentage display for readability, external legend placement, and proper tight layout with
bbox_inches="tight"for clean output.
909-952: LGTM!The context assembly is well-structured with clear sections for text data, statistics, chart buffers, and data lists. Perimeter handling properly guards against None values.
| # Filter out zero values to avoid cluttering | ||
| filtered_data = [(v, l, c) for v, l, c in zip(values, labels, plot_colors) if v > 0] | ||
| if filtered_data: | ||
| values, labels, plot_colors = zip(*filtered_data) | ||
| values, labels, plot_colors = list(values), list(labels), list(plot_colors) |
There was a problem hiding this comment.
Empty data after filtering may cause unexpected behavior.
If all values are zero, filtered_data will be empty, and zip(*filtered_data) produces nothing. The subsequent ax.pie() call with empty values may render incorrectly or raise warnings.
🔎 Proposed fix to handle all-zero case
# Filter out zero values to avoid cluttering
filtered_data = [(v, l, c) for v, l, c in zip(values, labels, plot_colors) if v > 0]
if filtered_data:
values, labels, plot_colors = zip(*filtered_data)
values, labels, plot_colors = list(values), list(labels), list(plot_colors)
+ else:
+ # Keep original data if all values are zero to show empty state
+ pass
supported tags for now:
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.