Skip to content

OpenConceptLab/ocl_online#655 | Show the versioning mode in version details - #97

Open
snyaggarwal wants to merge 3 commits into
mainfrom
ocl_online#655
Open

snyaggarwal wants to merge 3 commits into
mainfrom
ocl_online#655

Conversation

@snyaggarwal

@snyaggarwal snyaggarwal commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

OpenConceptLab/ocl_online#655, OpenConceptLab/ocl_online#673

What this does

This shows Seeded From in a version's details on the Versions tab: HEAD, the version it was seeded from (for example 2.82), or Import. The value comes from the new seeded_from field in oclapi2. The dialog never shows the field as raw JSON; a seed it doesn't recognise shows nothing.

  • src/components/repos/seededFrom.js: turns seeded_from into that label.
  • src/components/repos/versionsTab.styles.js: adds seeded_from to the version attribute labels and order.
  • Translations: en, es, pt-BR and zh.

Testing

  • npm test: 125 tests pass, including 3 new ones for the label.
  • eslint is clean on the changed files.

The backend part is in OpenConceptLab/oclapi2 on the branch of the same name.

🤖 Generated with Claude Code

@snyaggarwal snyaggarwal left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable findings in this frontend change.

@snyaggarwal snyaggarwal left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second review: the head SHA is unchanged and I found no actionable findings.

@snyaggarwal snyaggarwal left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third-round review

The head is unchanged from the previous review (6442269a). I rechecked the PR in the context of the updated backend contract and found no new actionable findings, performance issues, or nits.

@snyaggarwal
snyaggarwal marked this pull request as ready for review November 9, 2026 14:52

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review, with a Codex pass (commit 01a2fc4)

Verdict: approved with this fix. Apply the suggestion below (or an equivalent) and add the test, and it's good to merge after oclapi2#985. "Seeded From" works as D7 in the Repo Versions Model proposes. I ran this branch against production LOINC with seeded_from added to the API responses in the browser. An import-seeded version shows "Import", a version made from HEAD shows "HEAD", and HEAD shows the version the import seeded. The labels are in all four languages, and the row never shows raw JSON for the shapes oclapi2#985 writes today.

Fix before merge

  1. [P2] src/components/repos/versionsTab.styles.js:134: a seed with no label crashes the page. getSeededFromLabel returns undefined for {"type": "repo_version", "url": null} and for any type it doesn't know. EntityAttributesDialog then falls back to get(entity, field), gets the object back and, with no type, renders it as a React child. React throws "Objects are not valid as a React child", and the whole page shows "Something went wrong." I reproduced this by giving one LOINC version {"type": "repo_version", "url": null}. The migration in oclapi2#985 writes that shape for a version whose source has no HEAD, and later seed types (ocl_online#678) would hit it too, since TBv3 and the API deploy separately. The new tests check the helper's return value but not the dialog, so they pass. @snyaggarwal: there's a one-click suggestion inline that leaves the row out when there's no label. Please add a test through getVersionAttributeFields with a null URL and an unknown type. Codex found this on its own and rates it P1. I'd call it P2 because nothing on production has that shape today, but it's a one-line fix for a whole-page crash.

Nit

  • [P3] The PR title still says "versioning mode".

How it was checked

  • Read OpenConceptLab/ocl_online#673, the diff against main (it merges cleanly) and EntityAttributesDialog. CI is green.
  • Ran this branch's dev build against the production API, read-only, with seeded_from added to version responses in the browser: HEAD, an import-seeded version, a version made from HEAD, and one with no URL.
  • Codex (codex-cli 0.169.0, xhigh), one pass on the whole diff: 1 finding, the one above.
findings.json
[
 {
  "id": "W1",
  "severity": "P2",
  "file": "src/components/repos/versionsTab.styles.js",
  "line": 134,
  "claim": "Seeds without a display label crash the attributes dialog.",
  "scenario": "For the migration shape {\"type\":\"repo_version\",\"url\":null}, or an unknown seed type returned by a newer API, getSeededFromLabel returns undefined. EntityAttributesDialog then uses info?.value || get(entity, field), recovering the original object. With the JSON formatter disabled, React throws 'Objects are not valid as a React child'. This was reproduced through the dialog; the helper-only tests pass without exercising this fallback.",
  "action": "Prevent fallback to the seed object when no label exists, and add a dialog-path test covering null-URL and unknown-type seeds.",
  "confidence": "high",
  "source": "codex (F1.1, rated P1) + claude, reproduced in a dev build"
 }
]

Comment on lines +133 to 135
if(field === 'seeded_from')
info.value = getSeededFromLabel(value, t);
fields[field] = info;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] With no label, EntityAttributesDialog falls back to get(entity, field), gets the object back and React throws, so the whole page shows "Something went wrong." This leaves the row out instead. I checked it in the dev build with {"type": "repo_version", "url": null}: the dialog opens without the row, and ESLint is clean.

Suggested change
if(field === 'seeded_from')
info.value = getSeededFromLabel(value, t);
fields[field] = info;
if(field === 'seeded_from') {
info.value = getSeededFromLabel(value, t);
// with no label the dialog falls back to the raw object, which React can't render
if(!info.value)
return;
}
fields[field] = info;

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants