diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index ff915c9..56cbebc 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -19,10 +19,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Set up Python 3.10 + - name: Set up Python 3.13 uses: actions/setup-python@v3 with: - python-version: "3.10" + python-version: "3.13" - name: Install dependencies run: | python -m pip install --upgrade pip diff --git a/requirements.txt b/requirements.txt index 3e9ea6f..1f82b74 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,25 +1,38 @@ +annotated-doc==0.0.4 annotated-types==0.7.0 +anyio==4.12.1 beautifulsoup4==4.14.3 certifi==2026.1.4 charset-normalizer==3.4.4 +click==8.3.0 docraptor==3.1.0 +dotenv==0.9.9 et_xmlfile==2.0.0 +fastapi==0.128.5 fhir.resources==8.2.0 fhir_core==1.1.5 +h11==0.16.0 +httpcore==1.0.9 +httpx==0.28.1 idna==3.11 +Jinja2==3.1.6 numpy==2.4.2 openpyxl==3.1.5 pandas==3.0.0 pydantic==2.12.5 pydantic_core==2.41.5 python-dateutil==2.9.0.post0 +python-dotenv==1.2.1 +python-multipart==0.0.22 PyYAML==6.0.3 requests==2.32.5 six==1.17.0 soupsieve==2.8.3 +starlette==0.52.1 stringcase==1.2.0 typing-inspection==0.4.2 typing_extensions==4.15.0 urllib3==2.6.3 usdm==0.66.0 +uvicorn==0.38.0 yattag==1.16.1 diff --git a/src/soa_builder/web/routers/timings.py b/src/soa_builder/web/routers/timings.py index b30370b..ef02813 100644 --- a/src/soa_builder/web/routers/timings.py +++ b/src/soa_builder/web/routers/timings.py @@ -3,6 +3,8 @@ import os from typing import Optional +from pydantic import ValidationError + from fastapi import APIRouter, HTTPException, Request, Form from fastapi.responses import JSONResponse, HTMLResponse, RedirectResponse from fastapi.templating import Jinja2Templates @@ -291,21 +293,25 @@ def ui_create_timing( conn_c2.close() except Exception: rtf_code_uid = None - payload = TimingCreate( - name=name, - label=label, - description=description, - type=code_uid, - value=value, - value_label=value_label, - relative_to_from=rtf_code_uid, - relative_from_schedule_instance=relative_from_schedule_instance, - relative_to_schedule_instance=relative_to_schedule_instance, - window_label=window_label, - window_upper=window_upper, - window_lower=window_lower, - member_of_timeline=member_of_timeline, - ) + try: + payload = TimingCreate( + name=name, + label=label, + description=description, + type=code_uid, + value=value, + value_label=value_label, + relative_to_from=rtf_code_uid, + relative_from_schedule_instance=relative_from_schedule_instance, + relative_to_schedule_instance=relative_to_schedule_instance, + window_label=window_label, + window_upper=window_upper, + window_lower=window_lower, + member_of_timeline=member_of_timeline, + ) + except ValidationError as exc: + msgs = "; ".join(e["msg"] for e in exc.errors()) + raise HTTPException(400, f"Validation error: {msgs}") create_timing(soa_id, payload) return RedirectResponse(url=f"/ui/soa/{int(soa_id)}/timings", status_code=303) @@ -623,21 +629,25 @@ def ui_update_timing( # On any error, fall back to previous behavior (leave fields unchanged) mapped_type = None if type_submission_value not in ("",) else "" mapped_rtf = None if relative_to_from_submission_value not in ("",) else "" - payload = TimingUpdate( - name=name, - label=label, - description=description, - type=mapped_type, - value=value, - value_label=value_label, - relative_to_from=mapped_rtf, - relative_from_schedule_instance=relative_from_schedule_instance, - relative_to_schedule_instance=relative_to_schedule_instance, - window_label=window_label, - window_upper=window_upper, - window_lower=window_lower, - member_of_timeline=member_of_timeline, - ) + try: + payload = TimingUpdate( + name=name, + label=label, + description=description, + type=mapped_type, + value=value, + value_label=value_label, + relative_to_from=mapped_rtf, + relative_from_schedule_instance=relative_from_schedule_instance, + relative_to_schedule_instance=relative_to_schedule_instance, + window_label=window_label, + window_upper=window_upper, + window_lower=window_lower, + member_of_timeline=member_of_timeline, + ) + except ValidationError as exc: + msgs = "; ".join(e["msg"] for e in exc.errors()) + raise HTTPException(400, f"Validation error: {msgs}") update_timing(soa_id, timing_id, payload) return RedirectResponse(url=f"/ui/soa/{int(soa_id)}/timings", status_code=303) diff --git a/src/soa_builder/web/schemas.py b/src/soa_builder/web/schemas.py index 67695bf..8b837ec 100644 --- a/src/soa_builder/web/schemas.py +++ b/src/soa_builder/web/schemas.py @@ -1,6 +1,50 @@ +import re from typing import List, Optional -from pydantic import BaseModel +from pydantic import BaseModel, field_validator, model_validator + +# ISO 8601 duration pattern supporting both standard (-P2D) and USDM (P-2D) conventions +_ISO8601_DURATION_RE = re.compile( + r"^-?P-?(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)W)?(?:(\d+)D)?(?:T(?=\d)(\d+H)?(\d+M)?(\d+S)?)?$" +) + + +def _validate_iso8601_duration(v: Optional[str]) -> Optional[str]: + if v is None: + return v + v = v.strip() + if not v: + return None + m = _ISO8601_DURATION_RE.match(v) + if not m or not any(m.group(i) is not None for i in range(1, 8)): + raise ValueError( + f"'{v}' is not a valid ISO 8601 duration (e.g. P1D, P2W, PT8H, -P2D)" + ) + return v + + +def _validate_window_all_or_none( + window_lower: Optional[str], + window_upper: Optional[str], + window_label: Optional[str], +) -> None: + """Enforce that window_lower, window_upper, and window_label are all provided or all absent.""" + + def _present(v: Optional[str]) -> bool: + return v is not None and v.strip() != "" + + provided = [_present(window_lower), _present(window_upper), _present(window_label)] + if any(provided) and not all(provided): + missing = [] + if not _present(window_lower): + missing.append("window_lower") + if not _present(window_upper): + missing.append("window_upper") + if not _present(window_label): + missing.append("window_label") + raise ValueError( + f"Window fields are all-or-nothing: missing {', '.join(missing)}" + ) class InstanceUpdate(BaseModel): @@ -42,6 +86,18 @@ class TimingCreate(BaseModel): window_lower: Optional[str] = None member_of_timeline: Optional[str] = None + @field_validator("value", "window_lower", "window_upper", mode="before") + @classmethod + def check_iso8601_duration(cls, v: Optional[str]) -> Optional[str]: + return _validate_iso8601_duration(v) + + @model_validator(mode="after") + def check_window_all_or_none(self) -> "TimingCreate": + _validate_window_all_or_none( + self.window_lower, self.window_upper, self.window_label + ) + return self + class TimingUpdate(BaseModel): name: str @@ -58,6 +114,18 @@ class TimingUpdate(BaseModel): window_lower: Optional[str] = None member_of_timeline: Optional[str] = None + @field_validator("value", "window_lower", "window_upper", mode="before") + @classmethod + def check_iso8601_duration(cls, v: Optional[str]) -> Optional[str]: + return _validate_iso8601_duration(v) + + @model_validator(mode="after") + def check_window_all_or_none(self) -> "TimingUpdate": + _validate_window_all_or_none( + self.window_lower, self.window_upper, self.window_label + ) + return self + class ScheduleTimelineCreate(BaseModel): name: str diff --git a/src/soa_builder/web/templates/instances.html b/src/soa_builder/web/templates/instances.html index 0d376f2..93b529e 100644 --- a/src/soa_builder/web/templates/instances.html +++ b/src/soa_builder/web/templates/instances.html @@ -24,7 +24,7 @@

Scheduled Activity Instances for SoA {{ soa_id }}

- +
- - + + {% for name, instance_uid in (instance_options or {}).items() %} + {% endfor %}
- +
- - - {% for sv in relative_to_from_options or [] %} + {% for sv in timing_type_options or [] %} {% endfor %}
-
- - -
- - + +
- +
- - + +
@@ -87,6 +78,15 @@

Timings for SoA {{ soa_id }}

{% endfor %}
+
+ + +
@@ -97,16 +97,16 @@

Timings for SoA {{ soa_id }}

Name Label Description - Type + Rel From Instance Value Value Label - Rel To/From - Rel From Instance + Type Rel To Instance - Window Label - Window Upper Window Lower + Window Upper + Window Label Member of Timeline + Rel To/From Save Delete @@ -117,24 +117,6 @@

Timings for SoA {{ soa_id }}

- - - - - - - - + + + + + + + - - + + + @@ -175,7 +175,35 @@

Timings for SoA {{ soa_id }}

{% else %} - No timings yet. + No timings yet. {% endfor %} + + {% endblock %} diff --git a/tests/test_routers_timings.py b/tests/test_routers_timings.py index a02bb49..de52528 100644 --- a/tests/test_routers_timings.py +++ b/tests/test_routers_timings.py @@ -336,3 +336,263 @@ def test_timing_member_of_timeline(): assert resp.status_code == 201 data = resp.json() assert data["member_of_timeline"] == timeline_uid + + +def test_window_lower_rejects_non_iso8601(): + """Test that window_lower rejects non-ISO 8601 values.""" + r = client.post("/soa", json={"name": "Window Validate Lower"}) + soa_id = r.json()["id"] + + timing_data = {"name": "Bad Lower", "window_lower": "2 days"} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 422 + + +def test_window_upper_rejects_non_iso8601(): + """Test that window_upper rejects non-ISO 8601 values.""" + r = client.post("/soa", json={"name": "Window Validate Upper"}) + soa_id = r.json()["id"] + + timing_data = {"name": "Bad Upper", "window_upper": "+3"} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 422 + + +def test_window_accepts_valid_iso8601_durations(): + """Test that various valid ISO 8601 durations are accepted.""" + r = client.post("/soa", json={"name": "Window Valid ISO"}) + soa_id = r.json()["id"] + + valid_durations = [ + ("P1D", "P1D"), + ("P2W", "P2W"), + ("PT8H", "PT8H"), + ("-P2D", "-P2D"), + ("P-2D", "P-2D"), + ("P1Y2M3D", "P1Y2M3D"), + ("PT1H30M", "PT1H30M"), + ] + for i, (lower, upper) in enumerate(valid_durations): + timing_data = { + "name": f"Valid Duration {i}", + "window_lower": lower, + "window_upper": upper, + "window_label": f"Window {i}", + } + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 201, f"Failed for duration: {lower}/{upper}" + data = resp.json() + assert data["window_lower"] == lower + assert data["window_upper"] == upper + + +def test_window_rejects_bare_p(): + """Test that bare 'P' without any components is rejected.""" + r = client.post("/soa", json={"name": "Window Bare P"}) + soa_id = r.json()["id"] + + timing_data = {"name": "Bare P", "window_lower": "P"} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 422 + + +def test_update_timing_rejects_invalid_window(): + """Test that PATCH update also validates window fields.""" + r = client.post("/soa", json={"name": "Update Window Validate"}) + soa_id = r.json()["id"] + + timing_resp = client.post(f"/soa/{soa_id}/timings", json={"name": "Good"}) + timing_id = timing_resp.json()["id"] + + resp = client.patch( + f"/soa/{soa_id}/timings/{timing_id}", + json={"name": "Good", "window_upper": "bad"}, + ) + assert resp.status_code == 422 + + +def test_ui_create_timing_rejects_invalid_window(): + """Test that UI create form rejects non-ISO 8601 window values.""" + r = client.post("/soa", json={"name": "UI Window Validate"}) + soa_id = r.json()["id"] + + form_data = {"name": "Bad Window", "window_lower": "not-iso"} + resp = client.post( + f"/ui/soa/{soa_id}/timings/create", data=form_data, follow_redirects=False + ) + assert resp.status_code == 400 + + +def test_value_rejects_non_iso8601(): + """Test that value rejects non-ISO 8601 values.""" + r = client.post("/soa", json={"name": "Value Validate"}) + soa_id = r.json()["id"] + + timing_data = {"name": "Bad Value", "value": "5 days"} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 422 + + +def test_value_rejects_plain_number(): + """Test that a plain number like '5' is rejected for value.""" + r = client.post("/soa", json={"name": "Value Plain Number"}) + soa_id = r.json()["id"] + + timing_data = {"name": "Plain Num", "value": "5"} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 422 + + +def test_value_accepts_valid_iso8601_durations(): + """Test that various valid ISO 8601 durations are accepted for value.""" + r = client.post("/soa", json={"name": "Value Valid ISO"}) + soa_id = r.json()["id"] + + valid_values = ["P1D", "P2W", "PT8H", "-P2D", "P-2D", "P1Y2M3D", "PT1H30M"] + for i, val in enumerate(valid_values): + timing_data = {"name": f"Valid Value {i}", "value": val} + resp = client.post(f"/soa/{soa_id}/timings", json=timing_data) + assert resp.status_code == 201, f"Failed for value: {val}" + assert resp.json()["value"] == val + + +def test_update_timing_rejects_invalid_value(): + """Test that PATCH update also validates value field.""" + r = client.post("/soa", json={"name": "Update Value Validate"}) + soa_id = r.json()["id"] + + timing_resp = client.post( + f"/soa/{soa_id}/timings", json={"name": "Good", "value": "P1D"} + ) + timing_id = timing_resp.json()["id"] + + resp = client.patch( + f"/soa/{soa_id}/timings/{timing_id}", + json={"name": "Good", "value": "not-a-duration"}, + ) + assert resp.status_code == 422 + + +def test_ui_create_timing_rejects_invalid_value(): + """Test that UI create form rejects non-ISO 8601 value.""" + r = client.post("/soa", json={"name": "UI Value Validate"}) + soa_id = r.json()["id"] + + form_data = {"name": "Bad Value", "value": "two weeks"} + resp = client.post( + f"/ui/soa/{soa_id}/timings/create", data=form_data, follow_redirects=False + ) + assert resp.status_code == 400 + + +def test_window_all_or_none_accepts_all_three(): + """Test that providing all three window fields is accepted.""" + r = client.post("/soa", json={"name": "Window Complete"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={ + "name": "T1", + "window_lower": "-P1D", + "window_upper": "P2D", + "window_label": "Visit Window", + }, + ) + assert resp.status_code == 201 + + +def test_window_all_or_none_accepts_none(): + """Test that providing no window fields is accepted.""" + r = client.post("/soa", json={"name": "Window None"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={"name": "No Window"}, + ) + assert resp.status_code == 201 + data = resp.json() + assert data["window_lower"] is None + assert data["window_upper"] is None + assert data["window_label"] is None + + +def test_window_all_or_none_rejects_lower_only(): + """Test that providing only window_lower is rejected.""" + r = client.post("/soa", json={"name": "Window Lower Only"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={"name": "T1", "window_lower": "-P1D"}, + ) + assert resp.status_code == 422 + + +def test_window_all_or_none_rejects_upper_only(): + """Test that providing only window_upper is rejected.""" + r = client.post("/soa", json={"name": "Window Upper Only"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={"name": "T1", "window_upper": "P2D"}, + ) + assert resp.status_code == 422 + + +def test_window_all_or_none_rejects_label_only(): + """Test that providing only window_label is rejected.""" + r = client.post("/soa", json={"name": "Window Label Only"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={"name": "T1", "window_label": "Visit Window"}, + ) + assert resp.status_code == 422 + + +def test_window_all_or_none_rejects_two_of_three(): + """Test that providing two of three window fields is rejected.""" + r = client.post("/soa", json={"name": "Window Two of Three"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={"name": "T1", "window_lower": "-P1D", "window_upper": "P2D"}, + ) + assert resp.status_code == 422 + + +def test_window_all_or_none_update_rejects_partial(): + """Test that PATCH update also enforces all-or-nothing window rule.""" + r = client.post("/soa", json={"name": "Window Update Partial"}) + soa_id = r.json()["id"] + + timing_resp = client.post(f"/soa/{soa_id}/timings", json={"name": "T1"}) + timing_id = timing_resp.json()["id"] + + resp = client.patch( + f"/soa/{soa_id}/timings/{timing_id}", + json={"name": "T1", "window_lower": "-P1D"}, + ) + assert resp.status_code == 422 + + +def test_window_all_or_none_rejects_whitespace_only_label(): + """Test that whitespace-only window_label counts as absent.""" + r = client.post("/soa", json={"name": "Window Whitespace Label"}) + soa_id = r.json()["id"] + + resp = client.post( + f"/soa/{soa_id}/timings", + json={ + "name": "T1", + "window_lower": "-P1D", + "window_upper": "P2D", + "window_label": " ", + }, + ) + assert resp.status_code == 422 diff --git a/tests/test_timings.py b/tests/test_timings.py index 92f4b1e..415014c 100644 --- a/tests/test_timings.py +++ b/tests/test_timings.py @@ -52,14 +52,14 @@ def test_update_timing_mutable_fields_and_updated_fields(): "label": " Label X ", "description": " Desc Y ", "type": " relative ", - "value": " 5 ", + "value": " P5D ", "value_label": " days ", "relative_to_from": " from ", "relative_from_schedule_instance": " Arm A ", "relative_to_schedule_instance": " Epoch 1 ", "window_label": " Window ", - "window_upper": " +2 ", - "window_lower": " -1 ", + "window_upper": " P2D ", + "window_lower": " -P1D ", } r = client.patch(f"/soa/{soa_id}/timings/{tid}", json=payload) assert r.status_code == 200, r.text @@ -70,14 +70,14 @@ def test_update_timing_mutable_fields_and_updated_fields(): assert data["label"] == "Label X" assert data["description"] == "Desc Y" assert data["type"] == "relative" - assert data["value"] == "5" + assert data["value"] == "P5D" assert data["value_label"] == "days" assert data["relative_to_from"] == "from" assert data["relative_from_schedule_instance"] == "Arm A" assert data["relative_to_schedule_instance"] == "Epoch 1" assert data["window_label"] == "Window" - assert data["window_upper"] == "+2" - assert data["window_lower"] == "-1" + assert data["window_upper"] == "P2D" + assert data["window_lower"] == "-P1D" # updated_fields must include changed keys uf = set(data["updated_fields"])