diff --git a/.github/ISSUE_TEMPLATE/1-bug.yml b/.github/ISSUE_TEMPLATE/1-bug.yml new file mode 100644 index 00000000..44e0d877 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/1-bug.yml @@ -0,0 +1,42 @@ +name: Report a Bug +description: An unexpected error or behavior +projects: + - pendingintent/soa-workbench +type: Bug +body: + - type: dropdown + id: topic + validations: + required: true + attributes: + label: Topic + multiple: true + options: + - User Interface + - Class Entity + - Biomedical Concepts + - SDTM Dataset Specializations + - Custom + - type: markdown + attributes: + value: | + In the fields below, please provide a + [Minimal Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example). + - type: textarea + id: problem + validations: + required: true + attributes: + label: Describe the issue you are having in detail. Provide steps to reproduce, if applicable. + - type: textarea + id: screenshots + validations: + required: false + attributes: + label: Past any screenshots that help demonstrate your issue. + - type: textarea + id: files + validations: + required: false + attributes: + label: Attach any files that help reproducing or diagnosing your issue. diff --git a/.github/ISSUE_TEMPLATE/2-feature.yml b/.github/ISSUE_TEMPLATE/2-feature.yml new file mode 100644 index 00000000..21cd93c2 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/2-feature.yml @@ -0,0 +1,37 @@ +name: Feature/Enhancement Request +description: A request for a new Feature or Enhancement +projects: + - pendingintent/soa-workbench +type: Feature +body: + - type: dropdown + id: topic + validations: + required: true + attributes: + label: Topic + multiple: true + options: + - User Interface + - Class Entity + - Biomedical Concepts + - SDTM Dataset Specializations + - Custom + - type: textarea + id: feature-request + validations: + required: true + attributes: + label: Describe the feature you would like in detail. + - type: textarea + id: screenshots + validations: + required: false + attributes: + label: Paste any screenshots or diagrams that help describe the new feature. + - type: textarea + id: files + validations: + required: false + attributes: + label: Attach any files that help demonstrate or explain your new feature. \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..9b03e3e8 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: false +contact_links: + - name: Question and Answers + url: https://github.com/pendingintent/soa-workbench/discussions/categories/q-a + about: Please ask (or answer) any general user questions here. diff --git a/README.md b/README.md index 764b9a84..8fe80b38 100644 --- a/README.md +++ b/README.md @@ -26,10 +26,16 @@ This will ensure the submodule is always up-to-date. ## Installation +**Prerequisite: Python 3.11 or newer.** On macOS the built-in +`python3` is 3.9, which is too old. Install a newer one with +Homebrew (`brew install python@3.13`), from python.org, or use +`uv venv --python 3.13` in place of the `venv` step below. + Recommended: editable install for development. ```bash > python3 -m venv .venv > source .venv/bin/activate +> pip install --upgrade pip > pip install -r requirements.txt > pre-commit install > pre-commit run --all-files diff --git a/pyproject.toml b/pyproject.toml index 1ce942f2..e5b37afd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,9 +7,9 @@ name = "soa-builder" version = "0.1.0" description = "Schedule of Activities normalization, expansion, and validation utilities" readme = "README.md" -requires-python = ">=3.9" +requires-python = ">=3.11" authors = [ { name = "Pending Intent" } ] -license = { text = "Proprietary" } +license = { text = "MIT" } dependencies = [ "pandas>=2.0.0", "click>=8.1.0", diff --git a/src/soa_builder/web/app.py b/src/soa_builder/web/app.py index b6252ee8..a588d7f7 100644 --- a/src/soa_builder/web/app.py +++ b/src/soa_builder/web/app.py @@ -57,11 +57,13 @@ _migrate_element_table, _migrate_rename_cell_table, _migrate_rollback_add_elements_restored, + _migrate_rollback_add_cells_skipped, _migrate_add_epoch_type, _migrate_visit_columns, _migrate_timing_add_member_of_timeline, _migrate_instances_add_member_of_timeline, _migrate_matrix_cells_add_instance_id, + _migrate_matrix_cells_backfill_instance_id, _migrate_activity_concept_add_href, _migrate_activity_concept_add_dss, _migrate_study_cell_add_order_index, @@ -158,6 +160,7 @@ _diff_freezes_limited, _get_freeze, _list_freezes, + ROLLBACK_AUDIT_COLUMNS, _list_rollback_audit, ) from .routers import rollback as rollback_router @@ -227,6 +230,7 @@ get_next_concept_uid as _get_next_concept_uid, load_epoch_type_options, soa_exists, + resolve_instance_id_for_visit, load_epoch_type_map, table_has_columns as _table_has_columns, iso_duration_to_days, @@ -319,6 +323,7 @@ def _configure_logging(): _migrate_activity_concept_add_href() _migrate_activity_concept_add_dss() _migrate_matrix_cells_add_instance_id() +_migrate_matrix_cells_backfill_instance_id() _migrate_instances_add_member_of_timeline() _migrate_timing_add_member_of_timeline() _migrate_visit_columns() @@ -337,6 +342,7 @@ def _configure_logging(): _migrate_copy_cell_data() _migrate_element_id() _migrate_rollback_add_elements_restored() +_migrate_rollback_add_cells_skipped() _migrate_activity_add_uid() _migrate_arm_add_type_fields() _migrate_element_audit_columns() @@ -3058,7 +3064,8 @@ def set_cell(soa_id: int, payload: CellCreate): cur = conn.cursor() # Upsert semantics: find existing cur.execute( - "SELECT id FROM matrix_cells WHERE soa_id=? AND visit_id=? AND activity_id=?", + "SELECT id, instance_id FROM matrix_cells " + "WHERE soa_id=? AND visit_id=? AND activity_id=?", (soa_id, payload.visit_id, payload.activity_id), ) row = cur.fetchone() @@ -3079,15 +3086,37 @@ def set_cell(soa_id: int, payload: CellCreate): (payload.status, row[0]), ) cid = row[0] + inst_id = row[1] else: + inst_id = resolve_instance_id_for_visit(cur, soa_id, payload.visit_id) + if inst_id is None: + logger.warning( + "visit-only matrix cell written (soa=%s visit=%s): visit " + "does not map to exactly one instance; use the " + "instance-based endpoints", + soa_id, + payload.visit_id, + ) cur.execute( - "INSERT INTO matrix_cells (soa_id, visit_id, activity_id, status) VALUES (?,?,?,?)", - (soa_id, payload.visit_id, payload.activity_id, payload.status), + "INSERT INTO matrix_cells (soa_id, visit_id, instance_id, " + "activity_id, status) VALUES (?,?,?,?,?)", + ( + soa_id, + payload.visit_id, + inst_id, + payload.activity_id, + payload.status, + ), ) cid = cur.lastrowid conn.commit() conn.close() - return {"cell_id": cid, "status": payload.status} + # Cells without an instance never render in the matrix or export + return { + "cell_id": cid, + "status": payload.status, + "instance_resolved": inst_id is not None, + } # API endpoint fr returning a matrix for a Study/SOA @@ -3460,17 +3489,7 @@ def export_xlsx(soa_id: int, left: Optional[int] = None, right: Optional[int] = ) audit_df = pd.DataFrame(audit_rows) if audit_df.empty: - audit_df = pd.DataFrame( - columns=[ - "id", - "freeze_id", - "performed_at", - "visits_restored", - "activities_restored", - "cells_restored", - "concepts_restored", - ] - ) + audit_df = pd.DataFrame(columns=list(ROLLBACK_AUDIT_COLUMNS)) bio = io.BytesIO() # Prepare cover sheet metadata # Fetch study core metadata (name, study fields, created_at) @@ -6177,9 +6196,18 @@ def ui_toggle_cell( conn.close() current = "" else: + inst_id = resolve_instance_id_for_visit(cur, soa_id, int(visit_id)) + if inst_id is None: + conn.close() + raise HTTPException( + 409, + "visit does not map to exactly one instance; " + "toggle the cell with instance_id instead", + ) cur.execute( - "INSERT INTO matrix_cells (soa_id, visit_id, activity_id, status) VALUES (?,?,?,?)", - (soa_id, int(visit_id), activity_id, "X"), + "INSERT INTO matrix_cells (soa_id, visit_id, instance_id, " + "activity_id, status) VALUES (?,?,?,?,?)", + (soa_id, int(visit_id), inst_id, activity_id, "X"), ) conn.commit() conn.close() diff --git a/src/soa_builder/web/initialize_database.py b/src/soa_builder/web/initialize_database.py index d886d40c..27049268 100644 --- a/src/soa_builder/web/initialize_database.py +++ b/src/soa_builder/web/initialize_database.py @@ -465,7 +465,8 @@ def _init_db(): activities_restored INTEGER, cells_restored INTEGER, concepts_restored INTEGER, - elements_restored INTEGER + elements_restored INTEGER, + cells_skipped INTEGER )""" ) # Reorder audit (tracks manual drag reorder operations for visits & activities) diff --git a/src/soa_builder/web/migrate_database.py b/src/soa_builder/web/migrate_database.py index 056e6c0d..636cde23 100644 --- a/src/soa_builder/web/migrate_database.py +++ b/src/soa_builder/web/migrate_database.py @@ -656,6 +656,22 @@ def _migrate_rollback_add_elements_restored(): logger.warning("rollback_audit migration failed: %s", e) +# Migration: Add cells_skipped to rollback_audit +def _migrate_rollback_add_cells_skipped(): + try: + conn = _connect() + cur = conn.cursor() + cur.execute("PRAGMA table_info(rollback_audit)") + cols = {r[1] for r in cur.fetchall()} + if "cells_skipped" not in cols: + cur.execute("ALTER TABLE rollback_audit ADD COLUMN cells_skipped INTEGER") + conn.commit() + logger.info("Added cells_skipped column to rollback_audit") + conn.close() + except Exception as e: # pragma: no cover + logger.warning("rollback_audit cells_skipped migration failed: %s", e) + + # Migration: Add activity_uid to activity def _migrate_activity_add_uid(): """Add activity_uid column if missing; backfill as Activity_.""" @@ -935,6 +951,78 @@ def _migrate_matrix_cells_add_instance_id(): logger.warning("matrix_cells instance_id migration failed: %s", e) +def _migrate_matrix_cells_backfill_instance_id() -> int: + """Set instance_id on legacy visit-only matrix cells (idempotent). + + A cell is resolved through visit.encounter_uid, but only when that + encounter maps to exactly one instance. Ambiguous cells, cells with + no matching instance, and cells that would duplicate an existing + instance-keyed cell are left untouched. visit_id is kept. Returns + the number of cells updated. + """ + try: + conn = _connect() + cur = conn.cursor() + cur.execute("PRAGMA table_info(matrix_cells)") + if "instance_id" not in {r[1] for r in cur.fetchall()}: + conn.close() + return 0 + # Back the visit -> encounter -> instance joins used below, by + # the resolver, and by the activityIds lookup + cur.execute( + "CREATE INDEX IF NOT EXISTS idx_instances_soa_encounter " + "ON instances(soa_id, encounter_uid)" + ) + cur.execute( + "CREATE INDEX IF NOT EXISTS idx_visit_soa_encounter " + "ON visit(soa_id, encounter_uid)" + ) + cur.execute( + "UPDATE matrix_cells SET instance_id = (" + "SELECT i.id FROM visit v " + "INNER JOIN instances i " + "ON i.encounter_uid = v.encounter_uid " + "AND i.soa_id = v.soa_id " + "WHERE v.id = matrix_cells.visit_id " + "AND v.soa_id = matrix_cells.soa_id) " + "WHERE instance_id IS NULL AND visit_id IS NOT NULL " + "AND (SELECT COUNT(*) FROM visit v " + "INNER JOIN instances i " + "ON i.encounter_uid = v.encounter_uid " + "AND i.soa_id = v.soa_id " + "WHERE v.id = matrix_cells.visit_id " + "AND v.soa_id = matrix_cells.soa_id " + "AND v.encounter_uid IS NOT NULL) = 1 " + "AND NOT EXISTS (SELECT 1 FROM matrix_cells x " + "INNER JOIN visit v ON v.id = matrix_cells.visit_id " + "INNER JOIN instances i " + "ON i.encounter_uid = v.encounter_uid " + "AND i.soa_id = v.soa_id " + "WHERE x.soa_id = matrix_cells.soa_id " + "AND x.instance_id = i.id " + "AND x.activity_id = matrix_cells.activity_id)" + ) + updated = cur.rowcount + conn.commit() + cur.execute( + "SELECT COUNT(*) FROM matrix_cells " + "WHERE instance_id IS NULL AND visit_id IS NOT NULL" + ) + remaining = cur.fetchone()[0] + conn.close() + if updated or remaining: + logger.info( + "matrix_cells instance_id backfill: %d updated, " + "%d legacy cells left unresolved", + updated, + remaining, + ) + return updated + except Exception as e: + logger.warning("matrix_cells instance_id backfill failed: %s", e) + return 0 + + def _migrate_activity_concept_add_href(): """Add href column to store the API URI from which codeSystem and codeSystemVersion USDM properties can be derived""" try: diff --git a/src/soa_builder/web/routers/_freeze_helpers.py b/src/soa_builder/web/routers/_freeze_helpers.py index 5bd38533..53ddce5d 100644 --- a/src/soa_builder/web/routers/_freeze_helpers.py +++ b/src/soa_builder/web/routers/_freeze_helpers.py @@ -1146,30 +1146,83 @@ def _rollback_freeze(soa_id: int, freeze_id: int) -> dict: ) new_id = cur.lastrowid visit_id_map[v.get("id")] = new_id + cur.execute("PRAGMA table_info(activity)") + activity_cols = {r[1] for r in cur.fetchall()} + optional_activity_cols = [ + c + for c in ("activity_uid", "label", "description", "superscript") + if c in activity_cols + ] activity_id_map = {} for a in sorted(activities, key=lambda x: x.get("order_index", 0)): + cols = ["soa_id", "name", "order_index"] + vals = [soa_id, a.get("name"), a.get("order_index")] + for col in optional_activity_cols: + if a.get(col) is not None: + cols.append(col) + vals.append(a.get(col)) cur.execute( - "INSERT INTO activity (soa_id,name,order_index) VALUES (?,?,?)", - (soa_id, a.get("name"), a.get("order_index")), + f"INSERT INTO activity ({','.join(cols)}) " + f"VALUES ({','.join('?' * len(cols))})", + vals, ) new_id = cur.lastrowid activity_id_map[a.get("id")] = new_id + # Snapshots store the matrix axis under "visits" (instance rows + # carrying instance_uid) and cells keyed on instance_id. Instances + # are not rebuilt by rollback, so map each snapshot instance id to + # the live row through its immutable instance_uid. + snap_instance_uid = { + v.get("id"): v.get("instance_uid") for v in visits if v.get("instance_uid") + } + cur.execute( + "SELECT instance_uid, id FROM instances WHERE soa_id=?", + (soa_id,), + ) + live_instance_id = {r[0]: r[1] for r in cur.fetchall()} + has_cell_superscript = _table_has_columns(cur, "matrix_cells", ("superscript",)) inserted_cells = 0 + skipped_cells = 0 + skipped_missing_instance = 0 for c in cells: - old_vid = c.get("visit_id") old_aid = c.get("activity_id") - status = c.get("status", "").strip() + status = (c.get("status") or "").strip() if status == "": continue - vid = visit_id_map.get(old_vid) aid = activity_id_map.get(old_aid) - if vid and aid: + if not aid: + skipped_cells += 1 + continue + if c.get("instance_id") is not None: + inst_uid = snap_instance_uid.get(c.get("instance_id")) + inst_id = live_instance_id.get(inst_uid) + if inst_id is None: + skipped_missing_instance += 1 + skipped_cells += 1 + continue + cols = ["soa_id", "instance_id", "activity_id", "status"] + vals = [soa_id, inst_id, aid, status] + if has_cell_superscript and c.get("superscript") is not None: + cols.append("superscript") + vals.append(c.get("superscript")) + cur.execute( + f"INSERT INTO matrix_cells ({','.join(cols)}) " + f"VALUES ({','.join('?' * len(cols))})", + vals, + ) + inserted_cells += 1 + continue + # Legacy snapshot: cell keyed on visit_id only + vid = visit_id_map.get(c.get("visit_id")) + if vid: cur.execute( "INSERT INTO matrix_cells (soa_id, visit_id, activity_id, status)" " VALUES (?,?,?,?)", (soa_id, vid, aid, status), ) inserted_cells += 1 + else: + skipped_cells += 1 elements_restored = 0 for el in sorted(elements, key=lambda x: x.get("order_index", 0)): cur.execute( @@ -1285,6 +1338,8 @@ def _rollback_freeze(soa_id: int, freeze_id: int) -> dict: "visits_restored": len(visits), "activities_restored": len(activities), "cells_restored": inserted_cells, + "cells_skipped_missing_instance": skipped_missing_instance, + "cells_skipped": skipped_cells, "concept_mappings_restored": inserted_concepts, "elements_restored": elements_restored, "epochs_restored": len(snap_epochs), @@ -1296,6 +1351,21 @@ def _rollback_freeze(soa_id: int, freeze_id: int) -> dict: } +# Columns returned by _list_rollback_audit, in order; also the header +# row for the audit exports when there are no rows. +ROLLBACK_AUDIT_COLUMNS = ( + "id", + "freeze_id", + "performed_at", + "visits_restored", + "activities_restored", + "cells_restored", + "concepts_restored", + "elements_restored", + "cells_skipped", +) + + def _record_rollback_audit(soa_id: int, freeze_id: int, stats: dict): conn = _connect() cur = conn.cursor() @@ -1303,7 +1373,7 @@ def _record_rollback_audit(soa_id: int, freeze_id: int, stats: dict): "INSERT INTO rollback_audit" " (soa_id, freeze_id, performed_at, visits_restored," " activities_restored, cells_restored, concepts_restored," - " elements_restored) VALUES (?,?,?,?,?,?,?,?)", + " elements_restored, cells_skipped) VALUES (?,?,?,?,?,?,?,?,?)", ( soa_id, freeze_id, @@ -1313,6 +1383,7 @@ def _record_rollback_audit(soa_id: int, freeze_id: int, stats: dict): stats.get("cells_restored"), stats.get("concept_mappings_restored"), stats.get("elements_restored"), + stats.get("cells_skipped"), ), ) conn.commit() @@ -1324,7 +1395,8 @@ def _list_rollback_audit(soa_id: int) -> list: cur = conn.cursor() cur.execute( "SELECT id, freeze_id, performed_at, visits_restored," - " activities_restored, cells_restored, concepts_restored" + " activities_restored, cells_restored, concepts_restored," + " elements_restored, cells_skipped" " FROM rollback_audit WHERE soa_id=? ORDER BY id DESC", (soa_id,), ) @@ -1337,6 +1409,8 @@ def _list_rollback_audit(soa_id: int) -> list: "activities_restored": r[4], "cells_restored": r[5], "concepts_restored": r[6], + "elements_restored": r[7], + "cells_skipped": r[8], } for r in cur.fetchall() ] @@ -1365,7 +1439,7 @@ def _rollback_preview(soa_id: int, freeze_id: int) -> dict: snap = freeze["snapshot"] visits = snap.get("visits", []) activities = snap.get("activities", []) - cells = [c for c in snap.get("cells", []) if c.get("status", "").strip() != ""] + cells = [c for c in snap.get("cells", []) if (c.get("status") or "").strip() != ""] concepts_map = snap.get("activity_concepts", {}) or {} return { "freeze_id": freeze_id, diff --git a/src/soa_builder/web/routers/freezes.py b/src/soa_builder/web/routers/freezes.py index 1f061c4b..5c39ac77 100644 --- a/src/soa_builder/web/routers/freezes.py +++ b/src/soa_builder/web/routers/freezes.py @@ -273,6 +273,7 @@ def ui_freeze_rollback(request: Request, soa_id: int, freeze_id: int): "cells_restored": result["cells_restored"], "concept_mappings_restored": result["concept_mappings_restored"], "elements_restored": result.get("elements_restored"), + "cells_skipped": result.get("cells_skipped"), }, ) if request.headers.get("HX-Request") == "true": diff --git a/src/soa_builder/web/routers/rollback.py b/src/soa_builder/web/routers/rollback.py index 1a6f66a5..5a764769 100644 --- a/src/soa_builder/web/routers/rollback.py +++ b/src/soa_builder/web/routers/rollback.py @@ -68,22 +68,12 @@ def export_rollback_audit_xlsx(soa_id: int): if not soa_exists(soa_id): raise HTTPException(404, "SOA not found") from ..app import _list_rollback_audit # type: ignore + from ._freeze_helpers import ROLLBACK_AUDIT_COLUMNS rows = _list_rollback_audit(soa_id) df = pd.DataFrame(rows) if df.empty: - df = pd.DataFrame( - columns=[ - "id", - "freeze_id", - "performed_at", - "visits_restored", - "activities_restored", - "cells_restored", - "concepts_restored", - "elements_restored", - ] - ) + df = pd.DataFrame(columns=list(ROLLBACK_AUDIT_COLUMNS)) bio = io.BytesIO() with pd.ExcelWriter(bio, engine="openpyxl") as writer: df.to_excel(writer, index=False, sheet_name="RollbackAudit") diff --git a/src/soa_builder/web/templates/base.html b/src/soa_builder/web/templates/base.html index be95ec64..e963c8d8 100644 --- a/src/soa_builder/web/templates/base.html +++ b/src/soa_builder/web/templates/base.html @@ -51,7 +51,7 @@
  • Concept Categories
  • Unassigned Concepts
  • Concept Groups
    • -
  • Dataset Specializations
    • +
  • SDTM Specializations
  • CRF Specializations
    • diff --git a/src/soa_builder/web/templates/rollback_audit_modal.html b/src/soa_builder/web/templates/rollback_audit_modal.html index b225f827..ff8b4e8b 100644 --- a/src/soa_builder/web/templates/rollback_audit_modal.html +++ b/src/soa_builder/web/templates/rollback_audit_modal.html @@ -14,7 +14,9 @@

    Rollback Audit Log

    Visits Activities Cells + Skipped Concepts + Elements @@ -26,7 +28,9 @@

    Rollback Audit Log

    {{ r.visits_restored }} {{ r.activities_restored }} {{ r.cells_restored }} + {{ r.cells_skipped if r.cells_skipped is not none else "–" }} {{ r.concepts_restored }} + {{ r.elements_restored if r.elements_restored is not none else "–" }} {% endfor %} diff --git a/src/soa_builder/web/utils.py b/src/soa_builder/web/utils.py index cae608ec..11dbd35c 100644 --- a/src/soa_builder/web/utils.py +++ b/src/soa_builder/web/utils.py @@ -818,6 +818,24 @@ def _seed() -> int: return _next_uid_from_counter(cur, soa_id, "BiomedicalConcept_", _seed) +def resolve_instance_id_for_visit(cur, soa_id: int, visit_id: int) -> Optional[int]: + """Return the single instance id behind a visit, else None. + + A visit maps to instances through its encounter_uid. The result is + None when the visit has no encounter or when zero or several + instances share it (ambiguous), so callers never guess. + """ + cur.execute( + "SELECT i.id FROM visit v " + "INNER JOIN instances i " + "ON i.encounter_uid = v.encounter_uid AND i.soa_id = v.soa_id " + "WHERE v.soa_id=? AND v.id=? AND v.encounter_uid IS NOT NULL", + (soa_id, visit_id), + ) + rows = cur.fetchall() + return rows[0][0] if len(rows) == 1 else None + + def soa_exists(soa_id: int) -> bool: """Return True if an SOA row exists with the given id.""" try: diff --git a/src/usdm/generate_biomedical_concept_properties.py b/src/usdm/generate_biomedical_concept_properties.py index 58fb579f..8143a78f 100644 --- a/src/usdm/generate_biomedical_concept_properties.py +++ b/src/usdm/generate_biomedical_concept_properties.py @@ -27,6 +27,7 @@ _fetch_dss_spec, _get_biomedical_concept_data, _get_latest_bc_package_version, + _get_latest_sdtmct_version, ) logger = logging.getLogger("usdm.generate_biomedical_concept_properties") @@ -725,7 +726,7 @@ def _build_response_codes(cur, soa_id: int, bcp_uid: str) -> List[Dict[str, Any] "extensionAttributes": [], "code": ncit_code or "", "codeSystem": BCP_CODE_SYSTEM, - "codeSystemVersion": code_system_version or "", + "codeSystemVersion": _get_latest_sdtmct_version(), "decode": decode or "", "instanceType": "Code", } @@ -802,7 +803,7 @@ def build_usdm_biomedical_concept_properties( "extensionAttributes": [], "code": ncit_code, "codeSystem": BCP_CODE_SYSTEM, - "codeSystemVersion": code_system_version or "", + "codeSystemVersion": _get_latest_sdtmct_version(), "decode": decode, "instanceType": "Code", }, diff --git a/src/usdm/generate_biomedical_concepts.py b/src/usdm/generate_biomedical_concepts.py index f6ac7682..ffebebba 100644 --- a/src/usdm/generate_biomedical_concepts.py +++ b/src/usdm/generate_biomedical_concepts.py @@ -4,6 +4,7 @@ from soa_builder.web.db import _connect from .usdm_utils import ( + _get_latest_sdtmct_version, _get_biomedical_concept_synonyms as _get_biomedical_concept_synonyms, _get_biomedical_concept_reference as _get_biomedical_concept_reference, ) @@ -87,7 +88,8 @@ def build_usdm_biomedical_concepts(soa_id: int) -> List[Dict[str, Any]]: alias_code = r[3] concept_code = r[4] code_uid = r[5] - code_system_version = r[6] + # BC package dates are not CT releases; pin to the latest CT + code_system_version = _get_latest_sdtmct_version() decode = r[7] synonyms = synonyms_map[concept_code] reference = reference_map[concept_code] diff --git a/src/usdm/generate_scheduled_activity_instances.py b/src/usdm/generate_scheduled_activity_instances.py index 1cac13df..4348ef0d 100644 --- a/src/usdm/generate_scheduled_activity_instances.py +++ b/src/usdm/generate_scheduled_activity_instances.py @@ -4,7 +4,7 @@ import sys from soa_builder.web.db import _connect from soa_builder.web.utils import _nz -from .usdm_utils import _get_activity_ids +from .usdm_utils import _get_activity_ids_by_instance def build_usdm_instances( @@ -50,6 +50,7 @@ def build_usdm_instances( rows = cur.fetchall() conn.close() out: List[Dict[str, Any]] = [] + activity_ids = _get_activity_ids_by_instance(soa_id) for i, r in enumerate(rows): ( @@ -87,7 +88,7 @@ def build_usdm_instances( "instanceType": "ScheduledActivityInstance", "timelineId": _nz(timeline_id), "timelineExitId": _nz(timeline_exit_id), - "activityIds": _get_activity_ids(soa_id, encounter_uid), + "activityIds": activity_ids.get(instance_uid, []), "encounterId": _nz(encounter_uid), } out.append(instances) diff --git a/src/usdm/usdm_utils.py b/src/usdm/usdm_utils.py index 966ec060..8ed5d327 100644 --- a/src/usdm/usdm_utils.py +++ b/src/usdm/usdm_utils.py @@ -1,13 +1,19 @@ # utils for usdm generators import os import functools +import re import requests import logging from typing import List, Dict, Optional, Any, Tuple from soa_builder.web.codelist_config import EPOCH_TYPE_CODELIST from soa_builder.web.db import _connect -from soa_builder.web.utils import get_latest_sdtm_ct_href as _get_latest_sdtm_ct_href +from soa_builder.web.utils import ( + get_latest_ddf_ct_href as _get_latest_ddf_ct_href, + get_latest_sdtm_ct_href as _get_latest_sdtm_ct_href, +) + +_ISO_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") logger = logging.getLogger("usdm.usdm_utils") URL_PREFIX = ( @@ -201,6 +207,16 @@ def _get_latest_sdtmct_version() -> str: return (_get_latest_sdtm_ct_href() or "").removeprefix("sdtmct-") +@functools.lru_cache(maxsize=1) +def _get_latest_ddfct_version() -> str: + """Return the latest DDF CT package date string (e.g. '2026-03-27'). + + Counterpart of ``_get_latest_sdtmct_version`` for DDF codelists. + Falls back to '' on any error. + """ + return (_get_latest_ddf_ct_href() or "").removeprefix("ddfct-") + + @functools.lru_cache(maxsize=1) def _get_latest_bc_package_version() -> str: """Return the latest CDISC BC package date string (e.g. '2024-09-27'). @@ -663,6 +679,9 @@ def _build_level_code( parts = slug.split("-") if len(parts) >= 4: version = f"{parts[-3]}-{parts[-2]}-{parts[-1]}" + if not _ISO_DATE_RE.match(version): + # codelist_table may be an undated "/mdr/ct/packages" path + version = _get_latest_ddfct_version() return { "id": code_uid, "extensionAttributes": [], @@ -746,24 +765,47 @@ def _load_generate_decision_instances(): # Helpers for Scheduled Activity Instances -def _get_activity_ids(soa_id: int, encounter_uid: str) -> List[str]: +def _get_activity_ids_by_instance(soa_id: int) -> Dict[str, List[str]]: + """Map instance_uid to the activity UIDs marked in the matrix. + + Cells keyed on instance_id are matched directly. Legacy cells with + only visit_id are matched through visit.encounter_uid, but only + when that encounter maps to exactly one instance; otherwise the + cell is ambiguous and is excluded. Instances with no activities + are absent from the result. + """ conn = _connect() cur = conn.cursor() cur.execute( - "SELECT a.activity_uid from activity a " - "INNER JOIN matrix_cells m ON a.id = m.activity_id AND a.soa_id = m.soa_id " - "INNER JOIN visit v ON m.visit_id = v.id AND m.soa_id = v.soa_id " - "INNER JOIN instances i ON v.encounter_uid = i.encounter_uid AND v.soa_id = i.soa_id " - "WHERE i.soa_id=? and i.encounter_uid=?", - ( - soa_id, - encounter_uid, - ), + "SELECT i.instance_uid, a.activity_uid, a.id " + "FROM matrix_cells m " + "INNER JOIN activity a " + "ON a.id = m.activity_id AND a.soa_id = m.soa_id " + "INNER JOIN instances i " + "ON i.id = m.instance_id AND i.soa_id = m.soa_id " + "WHERE m.soa_id=? " + "UNION " + "SELECT i.instance_uid, a.activity_uid, a.id " + "FROM matrix_cells m " + "INNER JOIN activity a " + "ON a.id = m.activity_id AND a.soa_id = m.soa_id " + "INNER JOIN visit v " + "ON v.id = m.visit_id AND v.soa_id = m.soa_id " + "INNER JOIN instances i " + "ON i.encounter_uid = v.encounter_uid AND i.soa_id = m.soa_id " + "WHERE m.soa_id=? AND m.instance_id IS NULL " + "AND v.encounter_uid IN (" + "SELECT encounter_uid FROM instances WHERE soa_id=? " + "GROUP BY encounter_uid HAVING COUNT(*) = 1) " + "ORDER BY 3", + (soa_id, soa_id, soa_id), ) rows = cur.fetchall() conn.close() - activity_uids = [r[0] for r in rows] or [] - return activity_uids + by_instance: Dict[str, List[str]] = {} + for instance_uid, activity_uid, _ in rows: + by_instance.setdefault(instance_uid, []).append(activity_uid) + return by_instance # Helpers for Scheduled Decision Instances diff --git a/tests/test_freeze_rollback_instance_cells.py b/tests/test_freeze_rollback_instance_cells.py new file mode 100644 index 00000000..d28c2193 --- /dev/null +++ b/tests/test_freeze_rollback_instance_cells.py @@ -0,0 +1,391 @@ +"""Freeze rollback restores instance-keyed matrix cells.""" + +import json + +from fastapi.testclient import TestClient + +from soa_builder.web.app import app +from soa_builder.web.db import _connect +from soa_builder.web.routers._freeze_helpers import _rollback_freeze + +client = TestClient(app) + + +def _new_soa(): + return client.post("/soa", json={"name": "Rollback Cells"}).json()["id"] + + +def _make_instance(soa_id, uid): + resp = client.post( + f"/soa/{soa_id}/instances", + json={"name": uid, "instance_uid": uid}, + ) + assert resp.status_code in (200, 201), resp.text + return resp.json()["id"] + + +def _make_activity(soa_id, name): + resp = client.post(f"/soa/{soa_id}/activities", json={"name": name}) + assert resp.status_code in (200, 201), resp.text + return resp.json()["activity_id"] + + +def _toggle(soa_id, instance_id, activity_id): + resp = client.post( + f"/ui/soa/{soa_id}/toggle_cell", + data={"instance_id": instance_id, "activity_id": activity_id}, + ) + assert resp.status_code == 200, resp.text + + +def _freeze(soa_id): + client.post(f"/ui/soa/{soa_id}/freeze", data={"version_label": "v1"}) + conn = _connect() + row = conn.execute( + "SELECT id FROM soa_freeze WHERE soa_id=? ORDER BY id DESC", + (soa_id,), + ).fetchone() + conn.close() + return row[0] + + +def _cells(soa_id): + conn = _connect() + rows = conn.execute( + "SELECT m.instance_id, m.visit_id, a.activity_uid, m.status " + "FROM matrix_cells m LEFT JOIN activity a " + "ON a.id = m.activity_id WHERE m.soa_id=?", + (soa_id,), + ).fetchall() + conn.close() + return rows + + +def test_rollback_restores_instance_cells_with_activity_uid(): + soa_id = _new_soa() + inst = _make_instance(soa_id, "ScheduledActivityInstance_RB1") + act = _make_activity(soa_id, "Vitals") + conn = _connect() + original_uid = conn.execute( + "SELECT activity_uid FROM activity WHERE id=?", (act,) + ).fetchone()[0] + conn.close() + _toggle(soa_id, inst, act) + freeze_id = _freeze(soa_id) + _toggle(soa_id, inst, act) # clear the cell after the freeze + assert _cells(soa_id) == [] + + stats = _rollback_freeze(soa_id, freeze_id) + + assert stats["cells_restored"] == 1 + assert _cells(soa_id) == [(inst, None, original_uid, "X")] + + +def test_rollback_skips_cell_whose_instance_was_deleted(): + soa_id = _new_soa() + keep = _make_instance(soa_id, "ScheduledActivityInstance_RB2") + gone = _make_instance(soa_id, "ScheduledActivityInstance_RB3") + act = _make_activity(soa_id, "ECG") + _toggle(soa_id, keep, act) + _toggle(soa_id, gone, act) + freeze_id = _freeze(soa_id) + conn = _connect() + conn.execute("DELETE FROM matrix_cells WHERE soa_id=?", (soa_id,)) + conn.execute("DELETE FROM instances WHERE id=?", (gone,)) + conn.commit() + conn.close() + + stats = _rollback_freeze(soa_id, freeze_id) + + assert stats["cells_restored"] == 1 + assert stats["cells_skipped_missing_instance"] == 1 + assert [c[0] for c in _cells(soa_id)] == [keep] + + +def test_rollback_of_legacy_visit_snapshot_still_restores_cells(): + soa_id = _new_soa() + snapshot = { + "soa_id": soa_id, + "visits": [{"id": 1, "name": "V1", "order_index": 0}], + "activities": [{"id": 1, "name": "A1", "order_index": 0}], + "cells": [{"visit_id": 1, "activity_id": 1, "status": "X"}], + } + conn = _connect() + cur = conn.cursor() + cur.execute( + "INSERT INTO soa_freeze (soa_id, version_label, created_at, " + "snapshot_json) VALUES (?,?,?,?)", + (soa_id, "legacy", "2025-01-01T00:00:00", json.dumps(snapshot)), + ) + freeze_id = cur.lastrowid + conn.commit() + conn.close() + + stats = _rollback_freeze(soa_id, freeze_id) + + assert stats["cells_restored"] == 1 + cells = _cells(soa_id) + assert len(cells) == 1 + assert cells[0][0] is None and cells[0][1] is not None + + +def _modal_html(soa_id): + resp = client.get(f"/ui/soa/{soa_id}/rollback_audit") + assert resp.status_code == 200, resp.text + return resp.text + + +def test_rollback_audit_records_and_shows_skipped_cells(): + soa_id = _new_soa() + keep = _make_instance(soa_id, "ScheduledActivityInstance_RB4") + gone = _make_instance(soa_id, "ScheduledActivityInstance_RB5") + act = _make_activity(soa_id, "Labs") + _toggle(soa_id, keep, act) + _toggle(soa_id, gone, act) + freeze_id = _freeze(soa_id) + conn = _connect() + conn.execute("DELETE FROM instances WHERE id=?", (gone,)) + conn.commit() + conn.close() + + client.post(f"/ui/soa/{soa_id}/freeze/{freeze_id}/rollback") + + audit = client.get(f"/soa/{soa_id}/rollback_audit").json()["audit"] + assert audit[0]["cells_restored"] == 1 + assert audit[0]["cells_skipped"] == 1 + html = _modal_html(soa_id) + assert "Skipped" in html + + +def test_rollback_counts_unmatched_legacy_cells_as_skipped(): + soa_id = _new_soa() + snapshot = { + "soa_id": soa_id, + "visits": [{"id": 1, "name": "V1", "order_index": 0}], + "activities": [{"id": 1, "name": "A1", "order_index": 0}], + "cells": [ + {"visit_id": 1, "activity_id": 1, "status": "X"}, + {"visit_id": 99, "activity_id": 1, "status": "X"}, + {"visit_id": 1, "activity_id": 77, "status": "X"}, + {"visit_id": 1, "activity_id": 1, "status": ""}, + ], + } + conn = _connect() + cur = conn.cursor() + cur.execute( + "INSERT INTO soa_freeze (soa_id, version_label, created_at, " + "snapshot_json) VALUES (?,?,?,?)", + (soa_id, "legacy2", "2025-01-01T00:00:00", json.dumps(snapshot)), + ) + freeze_id = cur.lastrowid + conn.commit() + conn.close() + + stats = _rollback_freeze(soa_id, freeze_id) + + assert stats["cells_restored"] == 1 + # blank cells are not counted; two non-blank cells could not map + assert stats["cells_skipped"] == 2 + + +def test_rollback_restores_optional_activity_and_cell_fields(): + soa_id = _new_soa() + inst = _make_instance(soa_id, "ScheduledActivityInstance_RB6") + act = _make_activity(soa_id, "Dosing") + conn = _connect() + conn.execute( + "UPDATE activity SET label=?, description=?, superscript=? WHERE id=?", + ("Dose label", "Dose description", "b", act), + ) + conn.commit() + original_uid = conn.execute( + "SELECT activity_uid FROM activity WHERE id=?", (act,) + ).fetchone()[0] + conn.close() + _toggle(soa_id, inst, act) + resp = client.post( + f"/ui/soa/{soa_id}/cell_superscript/{inst}/{act}", + data={"superscript": "a"}, + ) + assert resp.status_code == 200, resp.text + freeze_id = _freeze(soa_id) + conn = _connect() + conn.execute( + "UPDATE activity SET label=NULL, description=NULL, superscript=NULL WHERE id=?", + (act,), + ) + conn.execute("DELETE FROM matrix_cells WHERE soa_id=?", (soa_id,)) + conn.commit() + conn.close() + + _rollback_freeze(soa_id, freeze_id) + + conn = _connect() + activity = conn.execute( + "SELECT activity_uid, label, description, superscript " + "FROM activity WHERE soa_id=?", + (soa_id,), + ).fetchall() + cell = conn.execute( + "SELECT superscript FROM matrix_cells WHERE soa_id=?", (soa_id,) + ).fetchall() + conn.close() + assert activity == [(original_uid, "Dose label", "Dose description", "b")] + assert cell == [("a",)] + + +def test_cells_skipped_migration_adds_column_once(tmp_path, monkeypatch): + import sqlite3 + + from soa_builder.web import migrate_database + + path = str(tmp_path / "old_schema.db") + conn = sqlite3.connect(path) + conn.execute( + "CREATE TABLE rollback_audit (id INTEGER PRIMARY KEY, " + "soa_id INTEGER, cells_restored INTEGER)" + ) + conn.commit() + conn.close() + monkeypatch.setattr(migrate_database, "_connect", lambda: sqlite3.connect(path)) + + migrate_database._migrate_rollback_add_cells_skipped() + migrate_database._migrate_rollback_add_cells_skipped() + + conn = sqlite3.connect(path) + cols = [r[1] for r in conn.execute("PRAGMA table_info(rollback_audit)")] + conn.close() + assert cols.count("cells_skipped") == 1 + + +def _legacy_freeze(soa_id, cells, label="legacy3"): + snapshot = { + "soa_id": soa_id, + "visits": [{"id": 1, "name": "V1", "order_index": 0}], + "activities": [{"id": 1, "name": "A1", "order_index": 0}], + "cells": cells, + } + conn = _connect() + cur = conn.cursor() + cur.execute( + "INSERT INTO soa_freeze (soa_id, version_label, created_at, " + "snapshot_json) VALUES (?,?,?,?)", + (soa_id, label, "2025-01-01T00:00:00", json.dumps(snapshot)), + ) + freeze_id = cur.lastrowid + conn.commit() + conn.close() + return freeze_id + + +def test_rollback_tolerates_null_cell_status(): + soa_id = _new_soa() + freeze_id = _legacy_freeze( + soa_id, + [ + {"visit_id": 1, "activity_id": 1, "status": "X"}, + {"visit_id": 1, "activity_id": 1, "status": None}, + ], + ) + + stats = _rollback_freeze(soa_id, freeze_id) + + # a null status is blank: neither restored nor counted as skipped + assert stats["cells_restored"] == 1 + assert stats["cells_skipped"] == 0 + + +def test_rollback_preview_tolerates_null_cell_status(): + from soa_builder.web.routers._freeze_helpers import _rollback_preview + + soa_id = _new_soa() + freeze_id = _legacy_freeze( + soa_id, + [ + {"visit_id": 1, "activity_id": 1, "status": "X"}, + {"visit_id": 1, "activity_id": 1, "status": None}, + ], + label="legacy4", + ) + + assert _rollback_preview(soa_id, freeze_id)["cells_to_restore"] == 1 + + +def test_skipped_count_covers_every_unrestorable_cell_kind(): + soa_id = _new_soa() + freeze_id = _legacy_freeze( + soa_id, + [ + {"visit_id": 1, "activity_id": 1, "status": "X"}, + {"visit_id": 99, "activity_id": 1, "status": "X"}, + {"visit_id": 1, "activity_id": 77, "status": "X"}, + {"visit_id": 1, "activity_id": 1, "status": ""}, + {"visit_id": 1, "activity_id": 1, "status": None}, + ], + label="legacy5", + ) + + stats = _rollback_freeze(soa_id, freeze_id) + + assert stats["cells_restored"] == 1 + assert stats["cells_skipped"] == 2 + + +def test_audit_list_includes_elements_restored(): + from soa_builder.web.routers._freeze_helpers import ( + _list_rollback_audit, + _record_rollback_audit, + ) + + soa_id = _new_soa() + freeze_id = _legacy_freeze(soa_id, [], label="legacy6") + _record_rollback_audit( + soa_id, + freeze_id, + {"cells_restored": 2, "elements_restored": 3, "cells_skipped": 1}, + ) + + row = _list_rollback_audit(soa_id)[0] + + assert row["elements_restored"] == 3 + assert "Elements" in _modal_html(soa_id) + + +def test_audit_columns_constant_matches_list_and_empty_export(): + import io + + import pandas as pd + + from soa_builder.web.routers._freeze_helpers import ( + ROLLBACK_AUDIT_COLUMNS, + _list_rollback_audit, + _record_rollback_audit, + ) + + soa_id = _new_soa() + freeze_id = _legacy_freeze(soa_id, [], label="legacy7") + resp = client.get(f"/soa/{soa_id}/rollback_audit/export/xlsx") + empty = pd.read_excel(io.BytesIO(resp.content)) + assert list(empty.columns) == list(ROLLBACK_AUDIT_COLUMNS) + + _record_rollback_audit(soa_id, freeze_id, {"cells_restored": 1}) + assert list(_list_rollback_audit(soa_id)[0]) == list(ROLLBACK_AUDIT_COLUMNS) + + +def test_audit_modal_renders_dash_for_null_counts_on_old_rows(): + soa_id = _new_soa() + conn = _connect() + conn.execute( + "INSERT INTO rollback_audit (soa_id, freeze_id, performed_at, " + "visits_restored, activities_restored, cells_restored, " + "concepts_restored, elements_restored, cells_skipped) " + "VALUES (?,?,?,?,?,?,?,NULL,NULL)", + (soa_id, 1, "2025-01-01T00:00:00", 1, 2, 3, 4), + ) + conn.commit() + conn.close() + + html = _modal_html(soa_id) + + assert ">None<" not in html + assert html.count("–") >= 2 diff --git a/tests/test_generate_biomedical_concept_properties.py b/tests/test_generate_biomedical_concept_properties.py index b430159c..769e97b1 100644 --- a/tests/test_generate_biomedical_concept_properties.py +++ b/tests/test_generate_biomedical_concept_properties.py @@ -542,10 +542,14 @@ def test_build_returns_usdm_shape(): "usdm.generate_biomedical_concept_properties._get_dss_href_for_bc", return_value="", ), + patch( + "usdm.generate_biomedical_concept_properties._get_latest_sdtmct_version", + return_value="2026-03-27", + ), ): populate_biomedical_concept_properties(SOA_ID) - out = build_usdm_biomedical_concept_properties(SOA_ID, BC_UID) + out = build_usdm_biomedical_concept_properties(SOA_ID, BC_UID) assert len(out) == 3 p = out[0] assert p["instanceType"] == "BiomedicalConceptProperty" @@ -562,7 +566,8 @@ def test_build_returns_usdm_shape(): sc = p["code"]["standardCode"] assert sc["code"] == "C25347" assert sc["codeSystem"] == "http://www.cdisc.org" - assert sc["codeSystemVersion"] == "2024-09-27" + # CT release date, not the BC package date stored on the row (#296) + assert sc["codeSystemVersion"] == "2026-03-27" assert sc["decode"] == "Height" diff --git a/tests/test_matrix_cells_instance_backfill.py b/tests/test_matrix_cells_instance_backfill.py new file mode 100644 index 00000000..67bf943a --- /dev/null +++ b/tests/test_matrix_cells_instance_backfill.py @@ -0,0 +1,276 @@ +"""Legacy visit-only matrix cells gain an instance_id.""" + +from fastapi.testclient import TestClient + +from soa_builder.web.app import _fetch_matrix, app +from soa_builder.web.db import _connect +from soa_builder.web.migrate_database import ( + _migrate_matrix_cells_backfill_instance_id, +) + +client = TestClient(app) + + +def _new_soa(): + resp = client.post("/soa", json={"name": "Backfill Test"}) + return resp.json()["id"] + + +def _seed(soa_id): + """Encounter_1 -> one instance; Encounter_2 -> two instances.""" + conn = _connect() + cur = conn.cursor() + ids = {"visit": {}, "instance": {}, "activity": {}} + for enc in ("Encounter_1", "Encounter_2", "Encounter_3"): + cur.execute( + "INSERT INTO visit (soa_id, name, encounter_uid) VALUES (?,?,?)", + (soa_id, enc, enc), + ) + ids["visit"][enc] = cur.lastrowid + layout = [ + ("ScheduledActivityInstance_1", "Encounter_1"), + ("ScheduledActivityInstance_2", "Encounter_2"), + ("ScheduledActivityInstance_3", "Encounter_2"), + ] + for uid, enc in layout: + cur.execute( + "INSERT INTO instances (soa_id, instance_uid, name, " + "encounter_uid) VALUES (?,?,?,?)", + (soa_id, uid, uid, enc), + ) + ids["instance"][uid] = cur.lastrowid + for num in (1, 2): + cur.execute( + "INSERT INTO activity (soa_id, name, activity_uid) VALUES (?,?,?)", + (soa_id, f"A{num}", f"Activity_{num}"), + ) + ids["activity"][num] = cur.lastrowid + conn.commit() + conn.close() + return ids + + +def _legacy_cell(soa_id, visit_id, activity_id): + conn = _connect() + cur = conn.cursor() + cur.execute( + "INSERT INTO matrix_cells (soa_id, visit_id, activity_id, " + "status) VALUES (?,?,?,?)", + (soa_id, visit_id, activity_id, "X"), + ) + conn.commit() + cell_id = cur.lastrowid + conn.close() + return cell_id + + +def _instance_of(cell_id): + conn = _connect() + row = conn.execute( + "SELECT instance_id FROM matrix_cells WHERE id=?", (cell_id,) + ).fetchone() + conn.close() + return row[0] + + +def test_backfill_resolves_cell_on_unique_encounter(): + soa_id = _new_soa() + ids = _seed(soa_id) + cell = _legacy_cell(soa_id, ids["visit"]["Encounter_1"], ids["activity"][1]) + _migrate_matrix_cells_backfill_instance_id() + assert _instance_of(cell) == ids["instance"]["ScheduledActivityInstance_1"] + + +def test_backfill_leaves_ambiguous_and_unmatched_cells_alone(): + soa_id = _new_soa() + ids = _seed(soa_id) + shared = _legacy_cell(soa_id, ids["visit"]["Encounter_2"], ids["activity"][1]) + no_instance = _legacy_cell(soa_id, ids["visit"]["Encounter_3"], ids["activity"][1]) + _migrate_matrix_cells_backfill_instance_id() + assert _instance_of(shared) is None + assert _instance_of(no_instance) is None + + +def test_backfill_skips_cell_that_would_duplicate_instance_cell(): + soa_id = _new_soa() + ids = _seed(soa_id) + inst = ids["instance"]["ScheduledActivityInstance_1"] + conn = _connect() + conn.execute( + "INSERT INTO matrix_cells (soa_id, instance_id, activity_id, " + "status) VALUES (?,?,?,?)", + (soa_id, inst, ids["activity"][1], "X"), + ) + conn.commit() + conn.close() + legacy = _legacy_cell(soa_id, ids["visit"]["Encounter_1"], ids["activity"][1]) + _migrate_matrix_cells_backfill_instance_id() + assert _instance_of(legacy) is None + + +def test_backfill_is_idempotent(): + soa_id = _new_soa() + ids = _seed(soa_id) + _legacy_cell(soa_id, ids["visit"]["Encounter_1"], ids["activity"][1]) + first = _migrate_matrix_cells_backfill_instance_id() + second = _migrate_matrix_cells_backfill_instance_id() + assert first >= 1 + assert second == 0 + + +def test_set_cell_writes_instance_id_when_visit_resolves(): + soa_id = _new_soa() + ids = _seed(soa_id) + resp = client.post( + f"/soa/{soa_id}/cells", + json={ + "visit_id": ids["visit"]["Encounter_1"], + "activity_id": ids["activity"][1], + "status": "X", + }, + ) + assert resp.status_code == 200, resp.text + assert ( + _instance_of(resp.json()["cell_id"]) + == ids["instance"]["ScheduledActivityInstance_1"] + ) + + +def test_set_cell_ambiguous_visit_still_succeeds_without_instance(): + soa_id = _new_soa() + ids = _seed(soa_id) + resp = client.post( + f"/soa/{soa_id}/cells", + json={ + "visit_id": ids["visit"]["Encounter_2"], + "activity_id": ids["activity"][1], + "status": "X", + }, + ) + assert resp.status_code == 200, resp.text + assert _instance_of(resp.json()["cell_id"]) is None + + +def test_legacy_toggle_writes_instance_id_when_visit_resolves(): + soa_id = _new_soa() + ids = _seed(soa_id) + resp = client.post( + f"/ui/soa/{soa_id}/toggle_cell", + data={ + "visit_id": ids["visit"]["Encounter_1"], + "activity_id": ids["activity"][1], + }, + ) + assert resp.status_code == 200, resp.text + conn = _connect() + row = conn.execute( + "SELECT instance_id FROM matrix_cells WHERE soa_id=? AND activity_id=?", + (soa_id, ids["activity"][1]), + ).fetchone() + conn.close() + assert row[0] == ids["instance"]["ScheduledActivityInstance_1"] + + +def _cell_count(soa_id): + conn = _connect() + n = conn.execute( + "SELECT COUNT(*) FROM matrix_cells WHERE soa_id=?", (soa_id,) + ).fetchone()[0] + conn.close() + return n + + +def test_legacy_toggle_rejects_ambiguous_visit_and_writes_nothing(): + soa_id = _new_soa() + ids = _seed(soa_id) + resp = client.post( + f"/ui/soa/{soa_id}/toggle_cell", + data={ + "visit_id": ids["visit"]["Encounter_2"], + "activity_id": ids["activity"][1], + }, + ) + assert resp.status_code == 409, resp.text + assert _cell_count(soa_id) == 0 + + +def test_legacy_toggle_rejects_visit_without_instances(): + soa_id = _new_soa() + ids = _seed(soa_id) + resp = client.post( + f"/ui/soa/{soa_id}/toggle_cell", + data={ + "visit_id": ids["visit"]["Encounter_3"], + "activity_id": ids["activity"][1], + }, + ) + assert resp.status_code == 409, resp.text + assert _cell_count(soa_id) == 0 + + +def test_set_cell_flags_whether_the_cell_will_render(): + soa_id = _new_soa() + ids = _seed(soa_id) + resolved = client.post( + f"/soa/{soa_id}/cells", + json={ + "visit_id": ids["visit"]["Encounter_1"], + "activity_id": ids["activity"][1], + "status": "X", + }, + ).json() + orphan = client.post( + f"/soa/{soa_id}/cells", + json={ + "visit_id": ids["visit"]["Encounter_2"], + "activity_id": ids["activity"][1], + "status": "X", + }, + ).json() + assert resolved["instance_resolved"] is True + assert orphan["instance_resolved"] is False + + +def test_set_cell_update_reports_existing_resolution(): + soa_id = _new_soa() + ids = _seed(soa_id) + payload = { + "visit_id": ids["visit"]["Encounter_2"], + "activity_id": ids["activity"][1], + "status": "X", + } + client.post(f"/soa/{soa_id}/cells", json=payload) + payload["status"] = "Y" + again = client.post(f"/soa/{soa_id}/cells", json=payload).json() + assert again["instance_resolved"] is False + + +def test_matrix_read_shows_resolved_but_not_orphan_cells(): + soa_id = _new_soa() + ids = _seed(soa_id) + for enc in ("Encounter_1", "Encounter_2"): + client.post( + f"/soa/{soa_id}/cells", + json={ + "visit_id": ids["visit"][enc], + "activity_id": ids["activity"][1], + "status": "X", + }, + ) + _, _, cells = _fetch_matrix(soa_id) + assert [c["instance_id"] for c in cells] == [ + ids["instance"]["ScheduledActivityInstance_1"] + ] + + +def test_backfill_creates_encounter_lookup_indexes(): + _migrate_matrix_cells_backfill_instance_id() + conn = _connect() + names = { + r[1] + for table in ("instances", "visit") + for r in conn.execute(f"PRAGMA index_list({table})") + } + conn.close() + assert "idx_instances_soa_encounter" in names + assert "idx_visit_soa_encounter" in names diff --git a/tests/test_usdm_code_system_version.py b/tests/test_usdm_code_system_version.py new file mode 100644 index 00000000..db80a5a7 --- /dev/null +++ b/tests/test_usdm_code_system_version.py @@ -0,0 +1,170 @@ +"""codeSystemVersion on cdisc.org codes is a CT release date (#296).""" + +import re + +from fastapi.testclient import TestClient + +from soa_builder.web.app import app +from soa_builder.web.db import _connect +from usdm import generate_biomedical_concept_properties as bcp_mod +from usdm import generate_biomedical_concepts as bc_mod +from usdm import usdm_utils + +SOA_ID = 987296 +CT_DATE = "2026-03-27" +BC_PACKAGE_DATE = "2026-05-26" +ISO_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$") +client = TestClient(app) + + +def _clean(cur): + for table in ( + "bcp_response_code", + "biomedical_concept_property", + "biomedical_concept", + "activity_concept", + "alias_code", + "code", + "code_association", + ): + cur.execute(f"DELETE FROM {table} WHERE soa_id=?", (SOA_ID,)) + + +def _seed_bcp(): + client.get("/soa") # ensure schema is initialized + conn = _connect() + cur = conn.cursor() + _clean(cur) + for num, code in ((1, "C82515"), (2, "C16809")): + cur.execute( + "INSERT INTO code (code_uid, soa_id, code, code_system, " + "code_system_version, decode) VALUES (?,?,?,?,?,?)", + ( + f"Code_{num}", + SOA_ID, + code, + "http://www.cdisc.org", + BC_PACKAGE_DATE, + "decode", + ), + ) + cur.execute( + "INSERT INTO alias_code (alias_code_uid, soa_id, " + "standard_code) VALUES (?,?,?)", + (f"AliasCode_{num}", SOA_ID, f"Code_{num}"), + ) + cur.execute( + "INSERT INTO biomedical_concept_property (soa_id, " + "biomedical_concept_uid, biomedical_concept_property_uid, " + "name, label, isRequired, isEnabled, datatype, code) " + "VALUES (?,?,?,?,?,?,?,?,?)", + ( + SOA_ID, + "BiomedicalConcept_1", + "BiomedicalConceptProperty_1", + "n", + "n", + 1, + 1, + "string", + "AliasCode_1", + ), + ) + cur.execute( + "INSERT INTO bcp_response_code (soa_id, " + "biomedical_concept_property_uid, response_code_uid, name, " + "label, is_enabled, code) VALUES (?,?,?,?,?,?,?)", + ( + SOA_ID, + "BiomedicalConceptProperty_1", + "ResponseCode_1", + "n", + "n", + 1, + "AliasCode_2", + ), + ) + conn.commit() + conn.close() + + +def test_bcp_codes_use_ct_release_date(monkeypatch): + _seed_bcp() + monkeypatch.setattr(bcp_mod, "_get_latest_sdtmct_version", lambda: CT_DATE) + props = bcp_mod.build_usdm_biomedical_concept_properties( + SOA_ID, "BiomedicalConcept_1" + ) + code = props[0]["code"]["standardCode"] + assert code["codeSystemVersion"] == CT_DATE + rc_code = props[0]["responseCodes"][0]["code"] + assert rc_code["codeSystemVersion"] == CT_DATE + + +def test_bc_standard_code_uses_ct_release_date(monkeypatch): + _seed_bcp() + conn = _connect() + cur = conn.cursor() + cur.execute( + "INSERT INTO biomedical_concept (soa_id, " + "biomedical_concept_uid, name, label, code) " + "VALUES (?,?,?,?,?)", + (SOA_ID, "BiomedicalConcept_1", "bc", "bc", "AliasCode_1"), + ) + cur.execute( + "INSERT INTO activity_concept (soa_id, activity_uid, " + "concept_code, concept_uid) VALUES (?,?,?,?)", + (SOA_ID, 1, "C82515", "BiomedicalConcept_1"), + ) + conn.commit() + conn.close() + monkeypatch.setattr(bc_mod, "_get_latest_sdtmct_version", lambda: CT_DATE) + monkeypatch.setattr(bc_mod, "_get_biomedical_concept_synonyms", lambda code: []) + monkeypatch.setattr(bc_mod, "_get_biomedical_concept_reference", lambda code: "") + monkeypatch.setattr(bc_mod, "_ensure_bcp_populated", lambda *a, **k: None) + monkeypatch.setattr(bc_mod, "populate_extension_attributes", lambda *a, **k: None) + monkeypatch.setattr( + bc_mod, "populate_crf_extension_attributes", lambda *a, **k: None + ) + out = bc_mod.build_usdm_biomedical_concepts(SOA_ID) + version = out[0]["code"]["standardCode"]["codeSystemVersion"] + assert version == CT_DATE + + +def test_level_code_without_dated_slug_falls_back(monkeypatch): + client.get("/soa") + conn = _connect() + cur = conn.cursor() + _clean(cur) + cur.execute( + "INSERT INTO code_association (soa_id, code_uid, codelist_table, " + "codelist_code, code) VALUES (?,?,?,?,?)", + (SOA_ID, "Code_9", "/mdr/ct/packages", "C188725", "C85826"), + ) + conn.commit() + conn.close() + monkeypatch.setattr(usdm_utils, "_get_latest_ddfct_version", lambda: CT_DATE) + out = usdm_utils._build_level_code(SOA_ID, "Code_9", "Objective") + assert out["codeSystemVersion"] == CT_DATE + assert ISO_DATE.match(out["codeSystemVersion"]) + + +def test_level_code_keeps_dated_slug(): + client.get("/soa") + conn = _connect() + cur = conn.cursor() + _clean(cur) + cur.execute( + "INSERT INTO code_association (soa_id, code_uid, codelist_table, " + "codelist_code, code) VALUES (?,?,?,?,?)", + ( + SOA_ID, + "Code_9", + "/mdr/ct/packages/ddfct-2025-09-26", + "C188725", + "C85826", + ), + ) + conn.commit() + conn.close() + out = usdm_utils._build_level_code(SOA_ID, "Code_9", "Objective") + assert out["codeSystemVersion"] == "2025-09-26" diff --git a/tests/test_usdm_instance_activity_ids.py b/tests/test_usdm_instance_activity_ids.py new file mode 100644 index 00000000..4cf7710f --- /dev/null +++ b/tests/test_usdm_instance_activity_ids.py @@ -0,0 +1,161 @@ +"""activityIds on ScheduledActivityInstance come from instance cells.""" + +from fastapi.testclient import TestClient + +from soa_builder.web.app import app +from soa_builder.web.db import _connect +from usdm.generate_scheduled_activity_instances import ( + build_usdm_instances, +) + +SOA_ID = 987001 +client = TestClient(app) + + +def _seed(): + client.get("/soa") # ensure schema is initialized + conn = _connect() + cur = conn.cursor() + for table in ("matrix_cells", "instances", "activity"): + cur.execute(f"DELETE FROM {table} WHERE soa_id=?", (SOA_ID,)) + for num in (1, 2): + cur.execute( + "INSERT INTO activity (soa_id, name, activity_uid) VALUES (?,?,?)", + (SOA_ID, f"A{num}", f"Activity_{num}"), + ) + # two instances share one encounter + for num in (1, 2): + cur.execute( + "INSERT INTO instances (soa_id, instance_uid, name, " + "encounter_uid) VALUES (?,?,?,?)", + (SOA_ID, f"ScheduledActivityInstance_{num}", f"I{num}", "Encounter_1"), + ) + ids = { + r[0]: r[1] + for r in cur.execute( + "SELECT instance_uid, id FROM instances WHERE soa_id=?", + (SOA_ID,), + ) + } + act = { + r[0]: r[1] + for r in cur.execute( + "SELECT activity_uid, id FROM activity WHERE soa_id=?", + (SOA_ID,), + ) + } + cur.execute( + "INSERT INTO matrix_cells (soa_id, instance_id, activity_id, " + "status) VALUES (?,?,?,?)", + (SOA_ID, ids["ScheduledActivityInstance_1"], act["Activity_1"], "X"), + ) + conn.commit() + conn.close() + + +def test_activity_ids_populated_from_instance_cells(): + _seed() + out = {i["id"]: i for i in build_usdm_instances(SOA_ID)} + first = out["ScheduledActivityInstance_1"] + second = out["ScheduledActivityInstance_2"] + assert first["activityIds"] == ["Activity_1"] + assert second["activityIds"] == [] + + +def _seed_legacy(): + """Legacy visit-only cells: one unique encounter, one shared.""" + client.get("/soa") + conn = _connect() + cur = conn.cursor() + soa = SOA_ID + 1 + for table in ("matrix_cells", "instances", "activity", "visit"): + cur.execute(f"DELETE FROM {table} WHERE soa_id=?", (soa,)) + for num in (1, 2, 3): + cur.execute( + "INSERT INTO activity (soa_id, name, activity_uid) VALUES (?,?,?)", + (soa, f"A{num}", f"Activity_{num}"), + ) + # Encounter_1 -> one instance; Encounter_2 -> two instances + layout = [ + ("ScheduledActivityInstance_1", "Encounter_1"), + ("ScheduledActivityInstance_2", "Encounter_2"), + ("ScheduledActivityInstance_3", "Encounter_2"), + ] + for uid, enc in layout: + cur.execute( + "INSERT INTO instances (soa_id, instance_uid, name, " + "encounter_uid) VALUES (?,?,?,?)", + (soa, uid, uid, enc), + ) + visit_ids = {} + for enc in ("Encounter_1", "Encounter_2"): + cur.execute( + "INSERT INTO visit (soa_id, name, encounter_uid) VALUES (?,?,?)", + (soa, enc, enc), + ) + visit_ids[enc] = cur.lastrowid + act = { + r[0]: r[1] + for r in cur.execute( + "SELECT activity_uid, id FROM activity WHERE soa_id=?", + (soa,), + ) + } + for enc, act_uid in ( + ("Encounter_1", "Activity_1"), + ("Encounter_2", "Activity_2"), + ): + cur.execute( + "INSERT INTO matrix_cells (soa_id, visit_id, activity_id, " + "status) VALUES (?,?,?,?)", + (soa, visit_ids[enc], act[act_uid], "X"), + ) + conn.commit() + conn.close() + return soa + + +def test_legacy_visit_cell_resolving_to_one_instance_is_kept(): + soa = _seed_legacy() + out = {i["id"]: i for i in build_usdm_instances(soa)} + assert out["ScheduledActivityInstance_1"]["activityIds"] == ["Activity_1"] + + +def test_legacy_visit_cell_on_shared_encounter_is_excluded(): + soa = _seed_legacy() + out = {i["id"]: i for i in build_usdm_instances(soa)} + assert out["ScheduledActivityInstance_2"]["activityIds"] == [] + assert out["ScheduledActivityInstance_3"]["activityIds"] == [] + + +def test_batch_lookup_returns_all_instances_in_one_call(): + from usdm.usdm_utils import _get_activity_ids_by_instance + + soa = _seed_legacy() + result = _get_activity_ids_by_instance(soa) + assert result == {"ScheduledActivityInstance_1": ["Activity_1"]} + + +def test_batch_lookup_orders_by_activity_id_and_dedupes(): + from usdm.usdm_utils import _get_activity_ids_by_instance + + _seed() + conn = _connect() + cur = conn.cursor() + iid = cur.execute( + "SELECT id FROM instances WHERE soa_id=? AND instance_uid=?", + (SOA_ID, "ScheduledActivityInstance_1"), + ).fetchone()[0] + aid = cur.execute( + "SELECT id FROM activity WHERE soa_id=? AND activity_uid=?", + (SOA_ID, "Activity_2"), + ).fetchone()[0] + cur.execute( + "INSERT INTO matrix_cells (soa_id, instance_id, activity_id, " + "status) VALUES (?,?,?,?)", + (SOA_ID, iid, aid, "X"), + ) + conn.commit() + conn.close() + result = _get_activity_ids_by_instance(SOA_ID) + assert result == {"ScheduledActivityInstance_1": ["Activity_1", "Activity_2"]}