feat(UX): Display available update icon for frameworks in audit #3334
Conversation
📝 WalkthroughWalkthroughAdded a read-only Framework.has_update property and exposed it via serializers; frontend mappings, a new FrameworkName field component, and page templates were updated to surface an "update available" indicator; tests were updated to expect the new field. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Frontend as Svelte_App
participant Backend as API
participant DB as Database_or_Library
Browser->>Frontend: Navigate to framework or assessment page
Frontend->>Backend: GET /api/... (request framework/assessment)
Backend->>DB: Load Framework and related Library
DB-->>Backend: Framework data (includes library.has_update)
Backend-->>Frontend: JSON response (includes framework.has_update)
Frontend->>Frontend: ModelTable reads meta.has_update
alt has_update == true
Frontend-->>Browser: Render name + update icon/label (FrameworkName)
else
Frontend-->>Browser: Render name only
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@backend/core/models.py`:
- Around line 2065-2068: The has_update property currently dereferences
self.library which can be None; update the has_update getter (the property
method named has_update) to guard against a null FK by first checking if
self.library is not None (or using getattr/self.library and a default) and
return a safe fallback (e.g., False) when library is missing, otherwise return
the existing self.library.has_update value; ensure you reference the has_update
property and the self.library attribute in the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@frontend/src/routes/`(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte:
- Around line 489-502: The code uses value.has_update when computing
isUpdatableFramework; change that to use optional chaining so it reads key ===
'framework' && value?.has_update (update the const isUpdatableFramework), and
keep the rest of the JSX that checks isUpdatableFramework (the icon and the
{m.updateAvailable()} text) intact; this ensures safe access to has_update while
leaving safeTranslate and m.updateAvailable() usage unchanged.
| {@const isUpdatableFramework = key === 'framework' && value.has_update} | ||
| <div class="flex flex-col"> | ||
| <div | ||
| class="text-sm font-medium text-gray-800 capitalize-first" | ||
| data-testid={key.replaceAll('_', '-') + '-field-title'} | ||
| > | ||
| {#if isUpdatableFramework} | ||
| <i title={m.updateAvailable()} class="fa-solid fa-circle-up text-success-600-400" | ||
| ></i> | ||
| {/if} | ||
| {safeTranslate(key)} | ||
| {#if isUpdatableFramework} | ||
| {`(${m.updateAvailable()})`} | ||
| {/if} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd frontend/src/routes/\(app\)/\(third-party\)/compliance-assessments/\[id=uuid\] && ls -laRepository: intuitem/ciso-assistant-community
Length of output: 718
🏁 Script executed:
cat -n "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte" | head -550 | tail -100Repository: intuitem/ciso-assistant-community
Length of output: 4745
🏁 Script executed:
# Search for PageData and type definitions in this route
rg -n "PageData|export.*type" "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]" -A 5Repository: intuitem/ciso-assistant-community
Length of output: 3259
🏁 Script executed:
# Search for the load function and +page.server.ts or +page.ts
fd -e ts -e js "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]"Repository: intuitem/ciso-assistant-community
Length of output: 670
🏁 Script executed:
# Look for any .server.ts or .ts files in this route directory
git ls-files "frontend/src/routes/*compliance-assessments*" | grep -E "\+page\.(server\.)?ts"Repository: intuitem/ciso-assistant-community
Length of output: 645
🏁 Script executed:
cat -n "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/+page.server.ts"Repository: intuitem/ciso-assistant-community
Length of output: 6755
🏁 Script executed:
cat -n "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/types.ts"Repository: intuitem/ciso-assistant-community
Length of output: 862
🏁 Script executed:
# Check what PageData looks like in generated types
cat -n "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/.svelte-kit/types/routes/(app)/(third-party)/compliance-assessments/%5Bid=uuid%5D/$types.d.ts" 2>/dev/null || echo "Generated types not available in sandbox"Repository: intuitem/ciso-assistant-community
Length of output: 117
🏁 Script executed:
# Search for PageData definitions in the codebase
rg -n "export.*type PageData|interface PageData" frontend/src -A 10 | head -50Repository: intuitem/ciso-assistant-community
Length of output: 548
🏁 Script executed:
# Search for ComplianceAssessmentSchema to understand the type shape
rg -n "ComplianceAssessmentSchema" frontend/src -A 20 | head -100Repository: intuitem/ciso-assistant-community
Length of output: 10802
🏁 Script executed:
# Search for compliance assessment serializer or API definition
rg -n "class ComplianceAssessment|def.*compliance.*assessment" --type=py | head -20Repository: intuitem/ciso-assistant-community
Length of output: 2140
🏁 Script executed:
# Look for schema definitions in frontend lib
cat -n "frontend/src/lib/utils/schemas.ts" | grep -A 30 "ComplianceAssessment"Repository: intuitem/ciso-assistant-community
Length of output: 3179
🏁 Script executed:
# Check if framework field is marked as optional/required anywhere
rg -n "framework.*required|framework.*optional|framework:" frontend/src/lib -A 2 | head -50Repository: intuitem/ciso-assistant-community
Length of output: 2602
Use optional chaining to safely access has_update on the framework object.
While the backend appears to guarantee framework is always present (evidenced by direct access at line 486 in the same component), defensive programming practices suggest using optional chaining at line 489:
🔧 Suggested change
- {`@const` isUpdatableFramework = key === 'framework' && value.has_update}
+ {`@const` isUpdatableFramework = key === 'framework' && value?.has_update}This protects against potential API changes or edge cases where framework might be absent or incomplete.
🤖 Prompt for AI Agents
In
`@frontend/src/routes/`(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte
around lines 489 - 502, The code uses value.has_update when computing
isUpdatableFramework; change that to use optional chaining so it reads key ===
'framework' && value?.has_update (update the const isUpdatableFramework), and
keep the rest of the JSX that checks isUpdatableFramework (the icon and the
{m.updateAvailable()} text) intact; this ensures safe access to has_update while
leaving safeTranslate and m.updateAvailable() usage unchanged.
There was a problem hiding this comment.
Defensive programming can make bad backend behavior going unoticed for a longer time because of the absence of bug it should cause to the frontend.
Letting undefined has_update creating bug on the frontend would lead to earlier detect of such bad behavior.
The has_update property should always be defined as enforced by the serializer anyway.
There was a problem hiding this comment.
@monsieurswag, you make an excellent point about the tradeoffs between defensive programming and fail-fast approaches. You're absolutely right that masking undefined values with optional chaining could delay detection of serializer bugs or API contract violations. If the backend should always provide has_update per the serializer contract, letting the frontend error immediately when it's missing is a valuable signal for catching regressions early.
I appreciate the clarification about your architectural preference here—fail-fast makes sense for tightly coupled frontend-backend codebases where catching contract violations quickly is more valuable than resilience to unexpected states.
✏️ Learnings added
Learnt from: monsieurswag
Repo: intuitem/ciso-assistant-community PR: 3334
File: frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/+page.svelte:489-502
Timestamp: 2026-02-02T14:37:17.618Z
Learning: In the ciso-assistant-community frontend codebase, prefer fail-fast approach over defensive programming with optional chaining when backend serializers enforce property contracts. This helps detect backend bugs earlier rather than masking them.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: melinoix
Repo: intuitem/ciso-assistant-community PR: 3107
File: frontend/src/routes/(app)/(internal)/recap/+page.svelte:49-51
Timestamp: 2025-12-29T15:52:14.696Z
Learning: In the recap page (frontend/src/routes/(app)/(internal)/recap/+page.svelte), perimeter.folder is guaranteed to be present when perimeter.compliance_assessments exist, so null safety checks are not necessary for perimeter.folder.str in that context.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/lib/components/ModelTable/FrameworkName.svelte`:
- Around line 8-10: The component currently assumes rest.meta exists when
creating hasUpdate (let hasUpdate = $derived(rest.meta.has_update)), which can
throw if meta is missing; change the hasUpdate initialization to guard against
missing meta by using optional chaining and a safe default (e.g., use
rest.meta?.has_update or rest.meta?.has_update ?? false) when calling $derived,
and keep the existing destructuring (cell, ...rest) and symbols ($props,
$derived, hasUpdate, meta.has_update) intact so the component remains resilient
to callers that omit meta.
In `@frontend/src/routes/`(app)/(internal)/frameworks/[id=uuid]/+page.svelte:
- Around line 52-60: The details loop is rendering the boolean field has_update
as a noisy row; update the Object.entries filtering used in the each block (the
expression starting with Object.entries(data.framework).filter...) to also
exclude the 'has_update' key alongside 'id', 'created_at', and
'reference_controls' so that data.framework.has_update is not iterated and
displayed in the detail list.
| let { cell, ...rest }: Props = $props(); | ||
|
|
||
| let hasUpdate = $derived(rest.meta.has_update); |
There was a problem hiding this comment.
Guard against missing meta to prevent runtime errors.
If meta isn’t provided by the table caller, rest.meta.has_update will throw. A defensive optional chain keeps the component resilient.
✅ Suggested fix
- let hasUpdate = $derived(rest.meta.has_update);
+ let hasUpdate = $derived(Boolean(rest?.meta?.has_update));📝 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.
| let { cell, ...rest }: Props = $props(); | |
| let hasUpdate = $derived(rest.meta.has_update); | |
| let { cell, ...rest }: Props = $props(); | |
| let hasUpdate = $derived(Boolean(rest?.meta?.has_update)); |
🤖 Prompt for AI Agents
In `@frontend/src/lib/components/ModelTable/FrameworkName.svelte` around lines 8 -
10, The component currently assumes rest.meta exists when creating hasUpdate
(let hasUpdate = $derived(rest.meta.has_update)), which can throw if meta is
missing; change the hasUpdate initialization to guard against missing meta by
using optional chaining and a safe default (e.g., use rest.meta?.has_update or
rest.meta?.has_update ?? false) when calling $derived, and keep the existing
destructuring (cell, ...rest) and symbols ($props, $derived, hasUpdate,
meta.has_update) intact so the component remains resilient to callers that omit
meta.
There was a problem hiding this comment.
If the meta field is undefined for whatever reason it's a good thing for it to create a bug as we would detect this bug earlier than if we handle the undefined scenario silently.
There was a problem hiding this comment.
@monsieurswag, that's a fair point! The fail-fast approach makes sense here—if meta is unexpectedly undefined, it signals a contract violation by the caller, and catching it immediately during development is more valuable than masking the issue. I'll note your preference.
✏️ Learnings added
Learnt from: monsieurswag
Repo: intuitem/ciso-assistant-community PR: 3334
File: frontend/src/lib/components/ModelTable/field/FrameworkName.svelte:8-10
Timestamp: 2026-02-02T14:38:02.984Z
Learning: In the CISO Assistant Community codebase, prefer fail-fast error handling over defensive optional chaining when a component expects required data (e.g., `rest.meta` in FrameworkName.svelte). Let missing required fields throw errors to catch integration bugs early rather than handling them silently.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app_tests/api/test_api_compliance_assessments.py (2)
1-6:⚠️ Potential issue | 🟠 MajorCI formatting failure (ruff format) must be fixed.
Pipeline reports
ruff format --check .would reformat a file. Please runruff formatand commit the result so CI passes.
221-229:⚠️ Potential issue | 🟠 MajorAdd
has_updatefield to UPDATE response expectation for consistency.The Framework serializer emits
has_updateas a read-only field in all responses. GET and CREATE response expectations include"has_update": False(lines 135-136, 176-177), but the UPDATE expectation (lines 221-229) omits it. This creates an inconsistency that makes the test fragile and incomplete.🔧 Suggested adjustment
"max_score": Framework.objects.all()[0].max_score, "ref_id": str(Framework.objects.all()[0].ref_id), + "has_update": False,
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app_tests/api/test_api_compliance_assessments.py (1)
128-137:⚠️ Potential issue | 🟡 MinorAvoid hard‑coding
has_updatein expectations to prevent brittle tests.
If fixtures or seed data ever setframework.library.has_update = True, these assertions will fail even though the API is correct. Derive the expected value from the actual framework instance used in each test.🔧 Suggested change (apply similarly in all three tests)
- "framework": { - "id": str(Framework.objects.all()[0].id), - "str": str(Framework.objects.all()[0]), + "framework": { + "id": str(Framework.objects.all()[0].id), + "str": str(Framework.objects.all()[0]), "implementation_groups_definition": None, "reference_controls": [], "min_score": 1, "max_score": 4, "ref_id": str(Framework.objects.all()[0].ref_id), - "has_update": False, + "has_update": ( + Framework.objects.all()[0].library.has_update + if Framework.objects.all()[0].library + else False + ), },Also applies to: 169-178, 222-231
Glossary:
A framework is considered as "updatable" if the library to which the frameworks belongs is itself updatable (
framework.library.has_update == True).(Even though it might not update the framework itself in some edge cases like an update which only update the description of the library).
List of features:
[1]Display the "update available" message and icon in the framework name display of the/compliance-assessments/{uuid}detail view.[2]Display the "update available" message and icon in the "name" column of the<ModelTable/>of the/frameworkslist view for updatable frameworks.[3]Display the "update available" message and icon at the top of the/frameworks/{uuid}detail view for updatable frameworks.The reviewer shall decide if we keep features
[2]and[3](as only feature[1]was originally meant to be implemented).Summary by CodeRabbit
New Features
Tests