Remove treatment, scores, and owner from risks - #1798
Open
SachaProbo wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
5 issues found across 37 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/risk.go">
<violation number="1" location="pkg/coredata/risk.go:647">
P2: When the risk disappears or falls outside the scope between `LoadByID` and `Update`, this method now returns success. Check `RowsAffected()` and return `ErrResourceNotFound` when it is zero.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_lib/usePrefillFromRisk.ts">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_lib/usePrefillFromRisk.ts:69">
P2: When a risk is selected, `next()` ignores the query result, but the create dialog still disables every field until this network request completes. Remove this fetch-based gate and apply `emptyPrefill(riskId)` locally.</violation>
</file>
<file name="pkg/coredata/migrations/20260828T143500Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260828T143500Z.sql:28">
P2: This migration permanently drops treatment/owner/score columns, but the backfill (20260825T113401Z) only copies data for risks attached to 5x5 scenario analyses with complete treatment/owner/inherent values. Risks carrying this data outside that narrow set lose it irrecoverably with no safety check in this destructive migration.</violation>
<violation number="2" location="pkg/coredata/migrations/20260828T143500Z.sql:30">
P1: When a risk is not linked to a 5x5 scenario, or has incomplete treatment data, the backfill creates no treatment plan, but this statement still deletes its existing values. Preserve those values in a treatment plan or validate and stop the migration before dropping the columns.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/overview/RiskOverviewPage.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/overview/RiskOverviewPage.tsx:64">
P2: When a risk has no note, the empty state now shows the `emptyScores` copy ("Scores are set on a treatment plan in a risk analysis."), which no longer matches what the page displays. The scores section was removed, so this fallback is now rendered for a missing note and misleads users. Use a note-specific empty message (or a new key such as `riskOverviewPage.emptyNote`) instead.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
SachaProbo
force-pushed
the
SachaProbo/drop-catalog-risk-fields
branch
from
August 28, 2026 15:24
783c4b5 to
af01847
Compare
Contributor
There was a problem hiding this comment.
5 existing issues remain and 2 new issues found across 37 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/cmd/risk/create/create.go">
<violation number="1" location="pkg/cmd/risk/create/create.go:136">
P2: Removing the --treatment, --inherent-likelihood, and --inherent-impact flags from prb risk create breaks an existing caller: contrib/seed.sh (lines 466-472) still passes all three to the command. Cobra returns an error for unknown flags and the script runs under set -euo pipefail, so contrib/seed.sh aborts during risk seeding. Update create_risk in contrib/seed.sh to stop passing those args (the values now belong to treatment plans).</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/overview/RiskOverviewPage.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/overview/RiskOverviewPage.tsx:53">
P2: When a risk has a note, this branch never renders `emptyScores`, so the overview omits the message explaining that scores moved to treatment plans. Render the note and `emptyScores` independently; note presence does not determine whether treatment-plan scores exist.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 5 unresolved issues already reported by Cubic.
Fix all with cubic | Re-trigger cubic
SachaProbo
force-pushed
the
SachaProbo/drop-catalog-risk-fields
branch
from
August 28, 2026 16:02
af01847 to
bfd7d20
Compare
Those fields now live on treatment plans. Catalog risks keep identity only: name, category, description, and note. Signed-off-by: Sacha Al Himdani <sacha@probo.com>
SachaProbo
force-pushed
the
SachaProbo/drop-catalog-risk-fields
branch
from
August 28, 2026 16:06
bfd7d20 to
9910c15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Those fields now live on treatment plans. Catalog
risks keep identity only: name, category,
description, and note.
Summary by cubic
Catalog risks now keep only their identity fields—name, category, description, and note—while treatment, scores, and owner remain on treatment plans. This removes those fields from risk storage and every client-facing contract, so callers and integrations must stop sending or requesting them.
Tests
+54-572Updates console and MCP coverage plus test factories for the reduced risk contract, removing obsolete scoring, treatment, and owner checks.
Coredata
+65-155Adds a migration and simplifies risk persistence, projections, and supported ordering fields.
GraphQL API
+13-101Updates console queries, mutations, resolvers, and response types to match the reduced risk model.
MCP
+18-124Updates MCP tool inputs, outputs, generated types, and ordering options for the new risk contract.
prb (CLI)
+37-234Updates
prb apiexamples and risk commands to stop sending and displaying removed values.Service
+29-401Aligns risk service requests, document generation, and exported risk-list templates with the catalog-only model.
App: console
+24-376Removes obsolete controls and columns from risk forms, tables, overviews, and linked-risk cards; treatment-plan prefill no longer reads these values from a risk.
Package: n8n-node
+0-235Removes the fields from n8n risk create and update inputs, returned fields, and sort options.
Other
+36-42Updates
contrib/seed.shto stop passing treatment and score arguments when seeding catalog risks.Written for commit 9910c15. Summary will update on new commits.