Lock editing pre-session info 24h before session start

Author: gfrn

src/scaup/crud/pre_sessions.py

@@ -1,11 +1,14 @@
 from fastapi import HTTPException, status
+from fastapi.security import HTTPAuthorizationCredentials
+from lims_utils.auth import GenericUser
 from sqlalchemy import select
 from sqlalchemy.dialects.postgresql import insert
 
 from ..models.inner_db.tables import PreSession
-from ..models.pre_sessions import PreSessionIn
+from ..models.pre_sessions import PreSessionIn, PreSessionOut
 from ..utils.crud import assert_not_booked
 from ..utils.database import inner_db
+from ..utils.session import check_session_locked
 
 
 @assert_not_booked
@@ -21,11 +24,15 @@ def create_pre_session_info(shipmentId: int, params: PreSessionIn):
     return pre_session
 
 
-def get_pre_session_info(shipmentId: int):
-    pre_session_info = inner_db.session.scalar(select(PreSession).filter(PreSession.shipmentId == shipmentId))
+def get_pre_session_info(shipment_id: int, user: GenericUser, token: HTTPAuthorizationCredentials):
+    pre_session_info = inner_db.session.scalar(select(PreSession).filter(PreSession.shipmentId == shipment_id))
     if not pre_session_info:
         raise HTTPException(
             status.HTTP_404_NOT_FOUND,
             "Shipment does not have a request assigned to it",
         )
-    return pre_session_info
+
+    validated_model = PreSessionOut.model_validate(pre_session_info, from_attributes=True)
+    validated_model.isLocked = check_session_locked(shipment_id, user, token)
+
+    return validated_model

src/scaup/models/pre_sessions.py

@@ -5,6 +5,7 @@
 
 class BasePreSession(BaseModel):
     details: Optional[dict[str, Any]] = None
+    isLocked: bool = False
 
 
 class PreSessionIn(BasePreSession):

src/scaup/routes/shipments.py

@@ -3,9 +3,12 @@
 from fastapi import APIRouter, Body, Depends, Query, status
 from fastapi.responses import RedirectResponse
 from fastapi.security import HTTPAuthorizationCredentials
+from lims_utils.auth import GenericUser
 from lims_utils.models import Paged, pagination
 
-from ..auth import Permissions, auth_scheme
+from scaup.utils.session import session_unlocked
+
+from ..auth import Permissions, User, auth_scheme
 from ..crud import containers as container_crud
 from ..crud import pdf as pdf_crud
 from ..crud import pre_sessions as ps_crud
@@ -159,9 +162,11 @@ def get_shipment_request(shipmentId=Depends(auth)):
 )
 def get_pre_session(
     shipmentId=Depends(auth),
+    user: GenericUser = Depends(User),
+    token: HTTPAuthorizationCredentials = Depends(auth_scheme),
 ):
     """Create new pre session information"""
-    return ps_crud.get_pre_session_info(shipmentId)
+    return ps_crud.get_pre_session_info(shipmentId, user, token)
 
 
 @router.put(
@@ -170,7 +175,7 @@ def get_pre_session(
     status_code=status.HTTP_201_CREATED,
 )
 def create_pre_session(
-    shipmentId=Depends(auth),
+    shipmentId=Depends(session_unlocked),
     parameters: PreSessionIn = Body(),
 ):
     """Upsert new pre session information"""

src/scaup/utils/session.py

@@ -1,13 +1,19 @@
 import re
+from datetime import datetime, timedelta
 
-from fastapi import HTTPException, status
+from fastapi import Depends, HTTPException, status
+from fastapi.security import HTTPAuthorizationCredentials
+from lims_utils.auth import GenericUser
 from lims_utils.logging import app_logger
 from psycopg.errors import ForeignKeyViolation, UniqueViolation
-from sqlalchemy import update
+from sqlalchemy import func, select, update
 from sqlalchemy.exc import IntegrityError
 
+from ..auth import Permissions, User, auth_scheme
 from ..models.inner_db import tables as db_tables
+from ..models.inner_db.tables import Shipment
 from .database import inner_db
+from .external import ExternalRequest
 
 UNIQUE_VIOLATION_REGEX = r"\((.*)\)=\((.*)\)"
 
@@ -70,9 +76,60 @@ def wrap(*args, **kwargs):
                 case ForeignKeyViolation():
                     columns = _get_columns_and_values(e.__cause__.diag.message_detail)
                     raise HTTPException(
-                        status_code=status.HTTP_404_NOT_FOUND, detail=f"Invalid {', '.join(columns.keys())} provided"
+                        status_code=status.HTTP_404_NOT_FOUND,
+                        detail=f"Invalid {', '.join(columns.keys())} provided",
                     )
                 case _:
                     app_logger.warning("Integrity error whilst performing request", exc_info=e)
 
     return wrap
+
+
+def check_session_locked(shipment_id: int, user: GenericUser, token: HTTPAuthorizationCredentials):
+    # Staff are exempt from this limitation
+    if bool({"em_admin", "super_admin"} & set(user.permissions)):
+        return False
+
+    session = inner_db.session.execute(
+        select(
+            func.concat(Shipment.proposalCode, Shipment.proposalNumber).label("proposal"),
+            Shipment.visitNumber,
+        ).filter(Shipment.id == shipment_id)
+    ).one()
+
+    response = ExternalRequest.request(
+        token=token.credentials,
+        method="GET",
+        url=f"/proposals/{session.proposal}/sessions/{session.visitNumber}",
+    )
+
+    if response.status_code != 200:
+        app_logger.warning("Failed to retrieve session from ISPyB at %s: %s", response.url, response.text)
+
+        raise HTTPException(
+            status_code=status.HTTP_424_FAILED_DEPENDENCY,
+            detail="Resource can't be verified upstream",
+        )
+
+    session_start = datetime.strptime(response.json()["startDate"], "%Y-%m-%dT%H:%M:%S")
+
+    if session_start - datetime.now() < timedelta(hours=24):
+        return True
+
+    return False
+
+
+def session_unlocked(
+    shipment_id: int = Depends(Permissions.shipment),
+    user: GenericUser = Depends(User),
+    token: HTTPAuthorizationCredentials = Depends(auth_scheme),
+):
+    """Check if a session is locked, and its resources can't be modified any further"""
+
+    if check_session_locked(shipment_id, user, token):
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Resource can't be modified 24 hours before session",
+        )
+
+    return shipment_id

tests/conftest.py

@@ -1,5 +1,4 @@
 import os
-from datetime import datetime
 from unittest.mock import patch
 
 # FastAPI's server has side-effects on import. This ensures the env. vars are set beforehand
@@ -151,7 +150,7 @@ def register_responses(request):
             "sessionId": 1,
             "beamLineOperator": "John Doe",
             "beamLineName": "m03",
-            "startDate": str(datetime.now()),
-            "endDate": str(datetime.now()),
+            "startDate": "2025-07-21T01:00:00",
+            "endDate": "2025-07-24T01:00:00",
         },
     )

tests/shipments/pre_session/__init__.py

@@ -0,0 +1,31 @@
+import pytest
+import responses
+from freezegun import freeze_time
+
+from ...test_utils.users import user
+
+
+@freeze_time("2025-07-01T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+def test_get(mock_user, client):
+    """Should get pre session information"""
+
+    resp = client.get("/shipments/2/preSession")
+
+    assert resp.status_code == 200
+
+    assert resp.json() == {"details": {"name": "previous"}, "isLocked": False}
+
+
+@freeze_time("2025-07-20T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+def test_get_locked(mock_user, client):
+    """Should get correct lock status for pre session information"""
+
+    resp = client.get("/shipments/2/preSession")
+
+    assert resp.status_code == 200
+
+    assert resp.json() == {"details": {"name": "previous"}, "isLocked": True}

tests/shipments/pre_session/test_get.py

@@ -1,10 +1,19 @@
+import pytest
+import responses
+from freezegun import freeze_time
 from sqlalchemy import select
 
 from scaup.models.inner_db.tables import PreSession
+from scaup.utils.config import Config
 from scaup.utils.database import inner_db
 
+from ...test_utils.users import admin, em_admin, user
 
-def test_create(client):
+
+@freeze_time("2025-07-01T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+def test_create(mock_user, client):
     """Should create new pre session information entry"""
 
     resp = client.put(
@@ -17,7 +26,10 @@ def test_create(client):
     assert inner_db.session.scalar(select(PreSession.details).filter(PreSession.shipmentId == 1)) == {"name": "test"}
 
 
-def test_update(client):
+@freeze_time("2025-07-01T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+def test_update(mock_user, client):
     """Should update pre session information entry"""
 
     resp = client.put(
@@ -28,3 +40,48 @@ def test_update(client):
     assert resp.status_code == 201
 
     assert inner_db.session.scalar(select(PreSession.details).filter(PreSession.shipmentId == 2)) == {"name": "newName"}
+
+
+@freeze_time("2025-07-20T15:00:00")
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+@responses.activate
+def test_locked(client, mock_user):
+    """Should return error if session is locked and user is not staff"""
+    resp = client.put(
+        "/shipments/2/preSession",
+        json={"details": {"name": "newName"}},
+    )
+
+    assert resp.status_code == 400
+    assert resp.json()["detail"] == "Resource can't be modified 24 hours before session"
+
+
+@freeze_time("2025-07-20T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [admin, em_admin], indirect=True)
+def test_locked_staff(mock_user, client):
+    """Should allow staff to modify experimental parameters in locked sessions"""
+    resp = client.put(
+        "/shipments/2/preSession",
+        json={"details": {"name": "newName"}},
+    )
+
+    assert resp.status_code == 201
+
+    assert inner_db.session.scalar(select(PreSession.details).filter(PreSession.shipmentId == 2)) == {"name": "newName"}
+
+
+@pytest.mark.noregister
+@freeze_time("2025-07-20T15:00:00")
+@responses.activate
+@pytest.mark.parametrize("mock_user", [user], indirect=True)
+def test_session_unavailable(mock_user, client):
+    """Should return error if upstream service can't find session"""
+    responses.get(Config.ispyb_api.url + "/proposals/cm2/sessions/1", json={"detail": "Not found"}, status=404)
+
+    resp = client.put(
+        "/shipments/2/preSession",
+        json={"details": {"name": "newName"}},
+    )
+
+    assert resp.status_code == 424

tests/shipments/pre_session/test_upsert.py

@@ -29,7 +29,7 @@ def test_get(client):
 
 
 @pytest.mark.noregister
-@responses.activate()
+@responses.activate
 def test_no_session(client):
     """Should return 404 if session is not found upstream"""
     responses.get(f"{Config.ispyb_api.url}/proposals/cm1/sessions/1", status=404)