codegouvfr / codegouvfr/catalogi
Duplicate CNLL column in source-provenance modal: `importFromInnerIdentifiers` registers ghost rows for secondary sources
- Dominant language
- TypeScript
- Stars
- 43
- Forks
- 14
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 3
Description
## Symptom
On `/detail?id=`, the "Sources des données" modal shows **CNLL twice** for softwares that also have a Comptoir du Libre row pointing at a CNLL annuaire URL. Reproduced on NextCloud (`softwareId=93`):
- Column 1 — `CNLL / Jamais récupéré` — every field empty.
- Column 2 — `CNLL / Dernière récupération : ` — populated providers list.
## Root cause
Two independent code paths both write rows into `software_external_datas` with `sourceSlug='cnll'` for the same `softwareId`, using **different `externalId` values**, and neither is a bug in isolation — they just disagree on what a CNLL externalId *is*.
### Path A — `CNLL/index.ts` `discoverSoftwareLinks`
`api/src/core/adapters/CNLL/index.ts:13-20`
```ts
discoverSoftwareLinks: async () => {
const cnllProviders = await getCnllPrestatairesSill();
return cnllProviders.map(provider => ({
externalId: provider.sill_id.toString(), // ← SILL id
softwareId: provider.sill_id,
softwareName: provider.nom
}));
}
```
For NextCloud this inserts `externalId='93', sourceSlug='cnll', softwareId=93`. `getCNLLSoftwareExternalData` (`api/src/core/adapters/CNLL/getExternalData.ts:21`) then matches by `element.sill_id.toString() === externalId` and populates the row. **This is the correct, populated row.**
### Path B — `importFromInnerIdentifiers`
`api/src/core/usecases/importFromInnerIdentifiers.ts` (called from the update job)
It scans every external row's `identifiers` JSON and, for each identifier whose `subjectOf.url` matches a known source base URL, calls `saveMany([{ sourceSlug, externalId: identifier.value, softwareId }])`.
Comptoir du Libre's row for NextCloud contains (from the DB):
```json
{
"url": "https://annuaire.cnll.fr/solutions/466",
"value": "466",
"subjectOf": { "url": "https://cnll.fr/", "additionalType": "cnll" }
}
```
`resolveRegisterable` maps `https://cnll.fr/` → `cnll`, takes `identifier.value='466'` as the externalId, and at line 104 inserts a second row:
```
externalId='466', sourceSlug='cnll', softwareId=93, lastDataFetchAt=NULL
```
That row can never be populated: the refresh loop eventually calls `getCNLLSoftwareExternalData({ externalId: '466' })`, which does `cnllProviders.find(e => e.sill_id.toString() === '466')`, finds nothing (NextCloud's `sill_id` is 93), and returns `undefined`. The stub row stays forever.
### Why it surfaces as duplicate columns
`getDetails` in `api/src/core/adapters/dbApi/kysely/createPgSoftwareRepository.ts:428-435` does a plain `SELECT * FROM software_external_datas WHERE softwareId=?` and passes every row straight through to `dataBySource`, no grouping by `sourceSlug`. `SourceProvenanceView` renders one column per row → two `CNLL` columns.
## Why we can't "just use the real CNLL id" (investigated)
Explored the "CNLL's externalId should be the annuaire id (466), not `sill_id`" framing and it is **not implementable against CNLL's current API**:
- `https://annuaire.cnll.fr/api/prestataires-sill.json` (the one endpoint our code uses) contains only `{ nom, sill_id, prestataires[] }`. The annuaire solution id is not exposed.
- `https://annuaire.cnll.fr/solutions/466/` is an HTML page. Its back-link to SILL uses the software **name**, not `sill_id`:
```html
```
- Probed `api/{solutions,solutions.json,prestataires.json,societes.json,sill.json,prestataires-solutions.json,solutions-sill.json}` on annuaire.cnll.fr — all 404.
So the only `annuaire_id ↔ sill_id` mapping that exists is inside individual HTML pages, and the pivot there is the name, not the id. There is no clean way to obtain the annuaire id from CNLL today.
## Recommended fix
**Don't chase the annuaire id. Fix the importer's assumption.**
`importFromInnerIdentifiers` implicitly assumes that for every source, `identifier.value` (pulled out of some *other* source's identifiers array) is a valid externalId in the target source's own id space. That's true for primary sources whose externalId space matches the ids other sources cite (Wikidata Q-ids, HAL docids, CDL numeric ids). It is **not** true for secondary sources like CNLL whose only machine-readable endpoint is keyed by `sill_id`.
Proposed change:
1. In `api/src/core/usecases/importFromInnerIdentifiers.ts`, skip sources whose gateway is `SecondarySourceGateway` (or equivalently, skip by `source.kind` for now — currently just `CNLL`). Resolve the source once, check `sourceProfile`, drop it before `resolveRegisterable` ever runs. This prevents the ghost row from ever being created.
2. One-shot cleanup of existing orphans — either:
- a migration that deletes `software_external_datas` rows where `sourceSlug='cnll' AND lastDataFetchAt IS NULL`, or
- rely on a manual `DELETE` for deployed instances and add the filter in code so it stops reappearing.
3. (Optional, defensive) in `getDetails` group `dataBySource` by `sourceSlug` before returning, picking the row with `lastDataFetchAt IS NOT NULL` if there's a tie. Masks this class of bug for any future secondary source misconfiguration. Low cost, nice safety net — but the real fix is (1).
## Files touched by the fix
- `api/src/core/usecases/importFromInnerIdentifiers.ts` — skip secondary sources.
- `api/src/core/adapters/dbApi/kysely/migrations/_delete-orphan-cnll-external-data.ts` — new migration.
- (optional) `api/src/core/adapters/dbApi/kysely/createPgSoftwareRepository.ts` — dedupe `dataBySource` defensively.
## Out of scope / future work
If CNLL (or Abilian, who maintains the annuaire — the HTML carries `data-theme="abilian"`) ever publishes a `{ annuaire_id, sill_id, name }` endpoint, we could revisit and switch CNLL to a primary-style externalId. Until then, treating CNLL as a `sill_id`-indexed secondary source is the only thing the data supports.
## Reproduction
1. Run the update job (`cd api && pnpm update`) against a DB where a CDL row references `https://annuaire.cnll.fr/solutions/` in its `identifiers`.
2. `SELECT "externalId", "sourceSlug", "softwareId", "lastDataFetchAt" FROM software_external_datas WHERE "sourceSlug"='cnll' AND "softwareId"=93;` — two rows.
3. Open the detail page, click "Voir les sources" — two CNLL columns.
Contributor guide
Research direction
Start with api/src/core/usecases/importFromInnerIdentifiers.ts and trace how source profiles and secondary gateways are resolved before saveMany runs. Reproduce with cd api && pnpm update and the provided software_external_datas query, then verify that secondary-source imports no longer create orphan CNLL rows and the detail view shows one CNLL column.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100