Repository navigation
Clone Biomedical Concepts and Dataset Specializations (closes #83) - #90
Conversation
The repo-wide ruleset requires a passing CodeQL code-scanning result before any PR can merge, but no CodeQL workflow existed anywhere in .github/workflows/ — every PR (e.g. #88) was permanently stuck at "Waiting for Code Scanning results" with no way to resolve it. Mirrors ci.yml's push/pull_request-on-all-branches trigger so results attach to PRs against any branch, not just main, plus a weekly schedule scan of the default branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add CodeQL workflow to satisfy required code-scanning check
As a BC/DSS curator, copy an existing BC or Specialization and tweak the group-level info while keeping its DECs/variables, per issue #83. Clone pre-fills the existing New BC / New Specialization form from a source record (group-level fields, DECs/variables) with the ID left blank, so nothing is saved until the curator edits and hits Save — reuses the existing create_bc()/save_decs() and specialization create() paths unchanged. Each cloned DEC's dec_id is blanked so save_decs() regenerates it under the new bc_id rather than keeping a stale "<old_bc_id>.DEC.N". BC clone is scoped to the BC + its DECs (not its linked specializations); specializations are cloned independently via their own action, matching the issue's treatment of BCs and specializations as parallel requests. - services/bc_service.py: add build_bc_clone(), returning a transient BC + DEC dicts copied from an existing bc_id. - routes/bc.py: /bc/new?clone_from=<bc_id> renders the New BC form pre-filled from the source. - routes/specializations.py: /specializations/?clone_from=<vlm_group_id> pre-fills the (already-collapsible) New Specialization panel from the source, without touching its vlm_group_id or the edit-in-place path. - templates/bc_detail.html: DEC table now renders from the `decs` route variable instead of `bc.decs`, so a transient (unsaved) clone object can be pre-filled; adds a Clone button and a "cloned from" banner. - templates/specializations.html: introduces a shared `fs` (edit_spec or clone_spec) template variable for prefill, keeping vlm_group_id and the published-lock checks scoped to true edit-in-place only; adds a Clone row action. - templates/bc_list.html: adds a Clone row action. - tests: services/bc_service, routes/bc, routes/specializations coverage for build_bc_clone() and both clone_from entry points. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review |
| # Fields copied when building a clone of an existing BC (group-level fields | ||
| # plus the same is_new-branch fields apply_bc_fields would set from a form). | ||
| _BC_CLONE_FIELDS = _BC_TEXT_FIELDS + ("ncit_code", "parent_bc_id", "loinc_code", "system", "system_name", "loinc_metadata", "ncit_metadata") |
There was a problem hiding this comment.
🟡 (optional) Cloning a BC copies ncit_metadata/loinc_metadata verbatim (build_bc_clone, services/bc_service.py:29) into hidden fields that a New BC form previously always left empty. If the curator types a different ncit_code or loinc_code by hand instead of re-running the lookup — JS only refreshes the hidden metadata via an explicit search-and-select (static/js/main.js:191,1078) — apply_bc_fields (services/bc_service.py:44-56) saves the new code paired with the old concept's stale metadata. Fix: blank ncit_metadata/loinc_metadata on clone like dec_id already is, or clear them server-side whenever the submitted code differs from the code that produced them.
Extended reasoning...
GET /bc/new?clone_from=C100 renders bc_detail.html with is_new=True: ncit_code text input prefilled 'C100' (line 161-162) and hidden #ncit_metadata prefilled with C100's saved NCIt JSON (line 145), because build_bc_clone copies loinc_metadata/ncit_metadata verbatim onto the transient clone. Before this diff, is_new=True BC() objects always had ncit_metadata=None, so this hidden field was always empty on New BC — the mismatch could not occur there; the clone path is what newly enables it. Curator edits the ncit_code input to 'C200' by hand — a natural way to 'tweak group-level info' — without clicking Look up NCIt. No listener clears #ncit_metadata on input to #ncit_code (static/js/main.js:84-126); only the 'Use this concept' handler (line 191/192) or the fetch-metadata block (gated on bc.bc_id, so it never runs for is_new) update it. On submit, apply_bc_fields' is_new branch sets bc.ncit_code='C200' but bc.ncit_metadata=the stale JSON describing C100, persisting a BC whose code and displayed NCIt metadata describe two different concepts. The identical mechanism applies to…
Verification: nit. The mechanism is real and reachable but low-impact. build_bc_clone copies ncit_metadata/loinc_metadata verbatim (services/bc_service.py:29, 105-115). The clone route renders bc_detail.html with is_new=True and this object (routes/bc.py:73-84), and the template prefills hidden #ncit_metadata (bc_detail.html:145) plus the visible ncit_code input (bc_detail.html:161-162). JS refreshes…
|
Clone functionality added to Biomedical Concepts and Data Set Specializations. |
Summary
Closes #83. As a BC/DSS curator, copy an existing BC or Specialization and tweak the group-level info while keeping its DECs/variables.
Design confirmed with the repo owner:
create_bc()/save_decs()and specializationcreate()write paths unchanged.package_dateandparent_bc_id) are copied verbatim, same as every other field, and remain editable before Save — confirmed no special-casing wanted.Changes:
services/bc_service.py—build_bc_clone(bc_id): returns a transient (unsaved)BiomedicalConcept+ DEC dicts copied from an existing BC. Each DEC'sdec_idis blanked sosave_decs()regenerates it under the newbc_idon save, instead of keeping a stale<old_bc_id>.DEC.N.routes/bc.py—GET /bc/new?clone_from=<bc_id>renders the New BC form pre-filled from the source (or flashes + redirects to the list if the source doesn't exist).routes/specializations.py—GET /specializations/?clone_from=<vlm_group_id>pre-fills the New Specialization panel (auto-expanded) from the source, without touchingvlm_group_idor the existing edit-in-place path.templates/bc_detail.html— the DEC table now renders from thedecsroute variable instead ofbc.decs(needed because a transient clone object isn't session-bound, so itsdecsrelationship can't be queried); adds a Clone button + a "cloned from" banner.templates/specializations.html— introduces a sharedfs(edit_spec or clone_spec) variable for prefill, while keepingvlm_group_idand the published-lock checks scoped to true edit-in-place only; adds a Clone row action.templates/bc_list.html— adds a Clone row action.Test plan
TestBuildBcClone(bc_service),TestCloneBc(bc routes),TestCloneSpecialization(specializations routes)pytest --tb=short— 492 passed (independently re-verified)isort/black/flake8clean (independently re-verified viapre-commit run --all-files)