feat: add priority to findings #2670
Conversation
tchoumi313
added
the
db-migration
WalkthroughAdds a priority field to Finding via migration and model update, exposes it in the API (filtering and a new choices endpoint), and integrates priority into frontend schemas, CRUD config, filters, tables, and the Finding form with initial option fetching and value normalization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Frontend (Finding Form)
participant API as Backend API (FindingViewSet)
participant C as Cache
participant DB as Database
rect rgba(220,240,255,0.4)
note over U: onMount
U->>API: GET /api/findings/status
U->>API: GET /api/findings/severity
U->>API: GET /api/findings/priority
API->>C: Check cache (priority choices)
alt cache hit
C-->>API: choices
else cache miss
API->>DB: Read Finding.PRIORITY choices
DB-->>API: choices
API->>C: Store choices
end
API-->>U: JSON choices
note over U: Normalize priority to numbers
end
rect rgba(235,255,235,0.4)
note over U: List Findings with filters
U->>API: GET /api/findings?priority=P1..P4
API->>DB: Filter by priority
DB-->>API: Results
API-->>U: Paginated findings
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 0
🧹 Nitpick comments (4)
frontend/src/lib/utils/table.ts (1)
1806-1807: Filter hook is correct; consider adding colored tags for findings priorityOptional: mirror the flag styling used for policies/applied-controls so findings show priority flags in lists.
Example addition in FIELD_COLORED_TAG_MAP (frontend/src/lib/utils/crud.ts):
findings: { name: { keys: { priority: { P1: { text: '', cssClasses: 'fa-solid fa-flag text-red-500' }, P2: { text: '', cssClasses: 'fa-solid fa-flag text-orange-500' }, P3: { text: '', cssClasses: 'fa-solid fa-flag text-blue-500' }, P4: { text: '', cssClasses: 'fa-solid fa-flag text-gray-500' }, '--': { text: '', cssClasses: '' } } } } }frontend/src/lib/components/Forms/ModelForm/FindingForm.svelte (3)
51-61: Fetch options in parallel and normalize priority immediatelyReduce latency and avoid extra reactivity by using Promise.all and converting priority values right after fetch.
Apply:
-onMount(async () => { - if (!model.selectOptions) { - const selectOptions = { - status: await fetch('/findings/status').then((r) => r.json()), - priority: await fetch('/findings/priority').then((r) => r.json()), - severity: await fetch('/findings/severity').then((r) => r.json()) - }; - model.selectOptions = selectOptions; - } -}); +onMount(async () => { + if (!model.selectOptions) { + const [status, priorityRaw, severity] = await Promise.all([ + fetch('/findings/status').then((r) => r.json()), + fetch('/findings/priority').then((r) => r.json()), + fetch('/findings/severity').then((r) => r.json()) + ]); + const priority = Array.isArray(priorityRaw) + ? priorityRaw.map((o) => ({ ...o, value: Number(o.value) })) + : priorityRaw; + model.selectOptions = { status, priority, severity }; + } +});
62-70: Avoid legacyrun; convert values where fetched (or use $effect)This legacy reactive wrapper isn’t needed after in-place conversion above. Remove to simplify.
Apply:
-// Convert priority values from strings to integers for proper schema validation -run(() => { - if (model?.selectOptions?.priority) { - model.selectOptions.priority.forEach((element) => { - element.value = parseInt(element.value); - }); - } -});
116-123: Priority Select integration looks goodBindings and labels are consistent with existing fields.
Optionally honor parent lock:
<Select ... disabled={isParentLocked} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/core/migrations/0105_finding_priority.py(1 hunks)backend/core/models.py(2 hunks)backend/core/views.py(2 hunks)frontend/src/lib/components/Forms/ModelForm/FindingForm.svelte(3 hunks)frontend/src/lib/utils/crud.ts(1 hunks)frontend/src/lib/utils/schemas.ts(1 hunks)frontend/src/lib/utils/table.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/core/views.py (2)
backend/crq/views.py (1)
priority(1087-1088)backend/core/models.py (1)
Finding(5586-5670)
backend/core/models.py (2)
backend/core/views.py (2)
priority(2008-2009)priority(7414-7415)backend/crq/views.py (1)
priority(1087-1088)
⏰ 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: enterprise-startup-functional-test (3.12)
- GitHub Check: build_community_frontend
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: build_enterprise_frontend
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
frontend/src/lib/utils/table.ts (2)
1780-1801: Adding 'priority' to findings table head/body is consistentPlacement next to severity is good; no issues.
359-369: FINDINGS_PRIORITY_FILTER wiring looks correct; backend ‘findings/priority’ endpoint confirmed
Endpoint/props align with existing patterns. Nice addition.frontend/src/lib/utils/crud.ts (1)
1366-1381: LGTM: findings selectFields/filters extended with priorityNumeric valueType for severity/priority is correct; filter list is comprehensive.
1 participant
Summary by CodeRabbit