Skip to content

fix(risks): working, all-or-nothing CSV import of risks (#755) - #859

Merged
alex-dembele merged 11 commits into
masterfrom
755-fixrisks-csv-import-of-risks-and-assets-fails-silently
Oct 2, 2026
Merged

fix(risks): working, all-or-nothing CSV import of risks (#755)#859
alex-dembele merged 11 commits into
masterfrom
755-fixrisks-csv-import-of-risks-and-assets-fails-silently

Conversation

@alex-dembele

@alex-dembele alex-dembele commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Closes #755

Depends on #855 (#792). This branch merges #792's branch to use its CreateRiskUseCase.WithAssets and GormRiskAssetStore. Merge #855 first; after that, this PR's diff is only the import.

POST /risks/import was never mounted, and the use case behind it parsed zero rows and reported success. This PR mounts it and makes it real:

  • Validate every row first. Return every error with its line, column, a stable code and params. Write nothing if any row is invalid.
  • Write a valid file in one transaction through CreateRiskUseCase, behind risk:create + capRisks, with a whole-file plan-cap check.
  • New optional assets column: asset names (case-insensitive) or ids, separated by ;, resolved inside the caller's tenant only. An unknown or ambiguous name refuses the row. Links are written in the same transaction, so the stored score is the frozen formula with asset criticality.
  • Refuse files on the old 1–5 scale with an explicit conversion message. They are never converted silently.
  • The page renders every error code in French or English, falling back to the server's English text for an unknown code. No success toast unless created > 0. The template uses P 0–1, I 0–10.

Verification

go build ./...                       ok
go test ./... -count=1               79 packages ok, 0 FAIL
npx tsc -b --noEmit                  ok
npx eslint <changed files>           ok
npx vitest run                       102 files, 950 tests passed

New tests: TestImportRisks_ErrorsCarryCodesForTranslation, TestImportRisks_AssetsColumnLinksAndScores, TestImportRisks_UnknownOrAmbiguousAssetImportsNothing, TestImportRisks_AssetsColumnWithoutResolverIsRefused, TestRiskImportHTTP_AssetsResolveOnlyInCallersTenant (another tenant's asset name refuses the file).

Live (Postgres 18 + Vite, throwaway):

  • 0.5 × 6 linked to a CRITICAL asset → score 9.0 critical; LOW asset → 1.5 low; no asset → 3.0 medium. Links checked in risk_assets.

  • Web shared by two assets → ambiguous_asset; Ghost → unknown_asset; nothing persisted.

  • In French: « Valeur obligatoire. », « Doit être entre 0 et 1 (valeur : 1.4). », « Aucun actif « Ghost » dans l'inventaire. », and the legacy-scale message.

  • Earlier pass: bad file persists nothing, good file → 200 created: 2.

  • The page body is shared (frontend/src/shared/csvImport/) and every string comes from the FR/EN catalogue (csvImport.*). This keeps the i18n ratchet and the orphan audit green: the five components/shared/* modules that only the old page imported are deleted.

Honest remainders

@alex-dembele
fix(risk): derive the asset-criticality term in one place (#792)
5832653 
A risk with no linked asset was stored with a factor of 1.0 by the worker,
the handler event and the demo seed, but the breakdown and score-working
views computed it with 1.5. Every such risk therefore showed "stored score
does not match the working".

domain.RiskAssetCriticality is now the only derivation: the average
ScoreFactor of the linked assets, or NoAssetCriticalityFactor (1.0) when
none is linked. The worker's repository lookup and both read views use it.
1.0 keeps every score already stored valid; the choice is logged as D-062
for the owner.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
@alex-dembele
fix(risk): store the Score Engine's score on create and update (#792)
de9e3da 
Create and update wrote Impact x Probability and left asset criticality to
the Redis worker. Until the worker ran, or forever if the event was lost,
the register showed a score the formula cannot produce: P=0.5, I=2 on a
critical asset was stored as 1.0 instead of 3.0.

Both use cases now resolve the linked assets themselves, score through
pkg/scoring's engine with that criticality, and write the risk and its
risk_assets links in one transaction (GormRiskAssetStore). The handler no
longer links assets after the fact; it passes asset_ids through and still
publishes risk.updated for the worker's audit entry. A malformed asset id
is now a 400 instead of silently dropping every link, and the update path
looks assets up by tenant_id unconditionally.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
@alex-dembele
fix(risk): tenant-scope the reload after create and update (#792)
b317e79 
Both handlers re-read the risk by id alone to build the response. The use
case had already proved ownership, but the query still broke the rule that
every read filters by tenant_id. It now does.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
@alex-dembele
docs(decisions): record D-062 and D-063 (#792)
c172036 
D-062: a risk with no linked asset scores with the neutral factor 1.0,
as applied in this PR. D-063 (raised as a duplicate D-061): the owner
ships the restrained 3D tilt, tracked in #856.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
@alex-dembele
fix(risks): mount CSV import, all-or-nothing through CreateRiskUseCase (
9b3ea8c 
#755)

POST /risks/import was never mounted and its parser returned zero rows.
Every row is now validated first, with line and column for each error;
a valid file is written in one transaction through CreateRiskUseCase.
Legacy 1-5 scale files are refused with an explicit message, and the
plan cap is checked against the whole file.
@alex-dembele
fix(risks): report the server's import result exactly (#755)
f0ed0a4 
The page shows created, rejected and every row error as the server sent
them, validated with Zod. No success toast unless created > 0. The
template now uses the product scales (P 0-1, I 0-10).
@alex-dembele alex-dembele linked an issue Oct 1, 2026 that may be closed by this pull request
fix(risks): CSV import of risks and assets fails silently #755
Closed
@alex-dembele
fix(risks): translate import row errors from stable codes (#755)
f3a2556 
Each import error now carries a code and params; the page renders them
in French or English and falls back to the server's English message for
an unknown code.
@alex-dembele
Merge remote-tracking branch 'origin/792-fixrisk-create-and-update-st…
c9d0c6f 
…ore-p-×-i-without-asset-criticality' into 755-fixrisks-csv-import-of-risks-and-assets-fails-silently
@alex-dembele
feat(risks): link imported risks to assets so their criticality is sc…
83f2314 
…ored (#755)

An optional assets column names the tenant's assets by name or id.
Names resolve inside the caller's tenant only; an unknown or ambiguous
name refuses the row. Links are written in the import transaction via
the #792 asset store, so the stored score includes asset criticality.
This was referenced Oct 1, 2026
Closed
Closed
@alex-dembele
refactor(risks): shared CSV import page on catalogue keys (#755)
2e88aee 
The page body moves to shared/csvImport so the asset import can reuse
it, and every string, error codes included, now comes from the FR/EN
catalogue instead of inline ternaries, which the i18n ratchet refused.
The rewrite had left components/shared unreachable; those five unused
modules are deleted, as the orphan audit requires.
@alex-dembele alex-dembele mentioned this pull request Oct 1, 2026
Merged
@alex-dembele
chore(i18n): drop catalogue keys the old import page used (#755)
a83aa14 
The rewritten page reads csvImport.*; these ten keys had no caller left.
@alex-dembele
alex-dembele merged commit 1352739 into master Oct 2, 2026
15 of 30 checks passed
@alex-dembele
alex-dembele deleted the 755-fixrisks-csv-import-of-risks-and-assets-fails-silently branch October 2, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(risks): CSV import of risks and assets fails silently

1 participant

@alex-dembele