diff --git a/application/blueprints/datamanager/controllers/flagged_resources.py b/application/blueprints/datamanager/controllers/flagged_resources.py index da3ec12..b144b53 100644 --- a/application/blueprints/datamanager/controllers/flagged_resources.py +++ b/application/blueprints/datamanager/controllers/flagged_resources.py @@ -11,7 +11,7 @@ from application.data_access.overview.digital_land_queries import get_resource from . import ControllerError -from .preview import record_branch_baseline +from .request_meta import record_branch_baseline, record_source_flow from .transform import handle_check_transform from ..services.async_api import AsyncAPIError, fetch_request, submit_request from ..services.dataset import get_collection_id, get_dataset_id, get_dataset_name @@ -375,6 +375,7 @@ def _submit_assign_entities_request( if selected_redirects is not None: params["selected_redirects"] = selected_redirects preview_id = submit_request(params) + record_source_flow(preview_id, "assign_entities") record_branch_baseline(preview_id, params["github_branch"]) return preview_id diff --git a/application/blueprints/datamanager/controllers/form.py b/application/blueprints/datamanager/controllers/form.py index 84d9d5f..670215d 100644 --- a/application/blueprints/datamanager/controllers/form.py +++ b/application/blueprints/datamanager/controllers/form.py @@ -14,7 +14,7 @@ url_for, ) -from .preview import record_branch_baseline +from .request_meta import record_branch_baseline, record_source_flow from ..services.async_api import ( AsyncAPIError, fetch_request, @@ -390,6 +390,7 @@ def _submit_add_data_preview(request_id, add_data_fields): } preview_id = submit_request(params) + record_source_flow(preview_id, "add_data") record_branch_baseline( preview_id, params["github_branch"], check_request_id=request_id ) diff --git a/application/blueprints/datamanager/controllers/preview.py b/application/blueprints/datamanager/controllers/preview.py index 7d722bc..726b9e6 100644 --- a/application/blueprints/datamanager/controllers/preview.py +++ b/application/blueprints/datamanager/controllers/preview.py @@ -14,10 +14,8 @@ from ..services.async_api import fetch_request from ..services.github import ( config_branch_changed_for_collection, - get_config_baseline_sha, trigger_add_data_async_workflow, wait_for_add_data_workflow_idle, - GitHubAppError, GitHubWorkflowError, ) from ..services.dataset import get_dataset_name @@ -156,39 +154,6 @@ def build_old_entity_redirect_table(old_entity_rows: list[dict]) -> dict | None: } -def record_branch_baseline(request_id, github_branch, check_request_id=None): - """ - Capture the config branch HEAD at assessment-submission time so that, when the - user later confirms, we can detect whether the branch advanced underneath the - assessment (which would make the assigned entity numbers stale). - """ - if not github_branch: - return - try: - # Quick HEAD read only - the workflow-idle wait happens at confirm time (the - # decision point), so submission stays fast. - sha = get_config_baseline_sha(github_branch) - except GitHubAppError as e: - logger.warning("Could not capture branch baseline for %s: %s", request_id, e) - return - if not sha: - return - - meta = db.session.get(RequestMeta, request_id) - if meta is None: - meta = RequestMeta( - request_id=request_id, - branch_sha=sha, - check_request_id=check_request_id, - ) - db.session.add(meta) - else: - meta.branch_sha = sha - if check_request_id: - meta.check_request_id = check_request_id - db.session.commit() - - def handle_entities_preview(request_id, req): # Check State status = req.get("status") @@ -258,11 +223,8 @@ def handle_entities_preview(request_id, req): ) = build_column_csv_preview(column_mapping, dataset_id, endpoint_summary) github_branch = params.get("github_branch") or None - source_flow = ( - "assign_entities" - if params.get("resource") and not params.get("url") - else "add_data" - ) + request_meta = db.session.get(RequestMeta, request_id) + source_flow = (request_meta.source_flow if request_meta else None) or "add_data" return_endpoint = params.get("return_endpoint") if return_endpoint: return_url = url_for(return_endpoint) @@ -272,7 +234,6 @@ def handle_entities_preview(request_id, req): return_url = url_for("datamanager.dashboard_get") # Retire endpoint details - request_meta = db.session.get(RequestMeta, request_id) endpoints_to_retire = ( _load_json_list(request_meta.endpoints_to_retire) if request_meta else [] ) diff --git a/application/blueprints/datamanager/controllers/request_meta.py b/application/blueprints/datamanager/controllers/request_meta.py new file mode 100644 index 0000000..e59b9f4 --- /dev/null +++ b/application/blueprints/datamanager/controllers/request_meta.py @@ -0,0 +1,67 @@ +"""Submission-time writers for the config-manager-owned RequestMeta table. + +These record per-request metadata (which flow created it, the config branch +baseline) that the async request's own params can't carry, so downstream pages +can behave correctly without re-inferring it. +""" + +import logging + +from application.db.models import RequestMeta +from application.extensions import db + +from ..services.github import GitHubAppError, get_config_baseline_sha + +logger = logging.getLogger(__name__) + + +def record_source_flow(request_id, source_flow): + """Persist which flow (``add_data`` / ``assign_entities``) created this request. + + The entities-preview and confirm pages live under the datamanager blueprint + but are shared with the assign-entities flow. Recording the originating flow + at submission time lets those shared pages apply the correct process lock + without inferring it from the request's params shape. + """ + if not request_id: + return + meta = db.session.get(RequestMeta, request_id) + if meta is None: + meta = RequestMeta(request_id=request_id, source_flow=source_flow) + db.session.add(meta) + else: + meta.source_flow = source_flow + db.session.commit() + + +def record_branch_baseline(request_id, github_branch, check_request_id=None): + """ + Capture the config branch HEAD at assessment-submission time so that, when the + user later confirms, we can detect whether the branch advanced underneath the + assessment (which would make the assigned entity numbers stale). + """ + if not github_branch: + return + try: + # Quick HEAD read only - the workflow-idle wait happens at confirm time (the + # decision point), so submission stays fast. + sha = get_config_baseline_sha(github_branch) + except GitHubAppError as e: + logger.warning("Could not capture branch baseline for %s: %s", request_id, e) + return + if not sha: + return + + meta = db.session.get(RequestMeta, request_id) + if meta is None: + meta = RequestMeta( + request_id=request_id, + branch_sha=sha, + check_request_id=check_request_id, + ) + db.session.add(meta) + else: + meta.branch_sha = sha + if check_request_id: + meta.check_request_id = check_request_id + db.session.commit() diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 69b83fe..a7445ad 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -52,6 +52,14 @@ inject_now, ) +# Routes that physically live under datamanager but are also used by the +# assign-entities flow. For these the applicable process lock depends on which +# flow the request belongs to, not on the URL prefix. +_SHARED_FLOW_ENDPOINTS = { + "datamanager.entities_preview", + "datamanager.add_data_confirm_async", +} + datamanager_bp = Blueprint("datamanager", __name__, url_prefix="/datamanager") assign_entities_bp = Blueprint( "assign_entities", __name__, url_prefix="/assign-entities" @@ -126,6 +134,18 @@ def _require_assign_entities_unlocked(): ) +def _request_is_assign_entities_flow(): + """Whether a shared datamanager route belongs to the assign-entities flow.""" + request_id = (request.view_args or {}).get("request_id") + if not request_id: + return False + try: + meta = db.session.get(RequestMeta, request_id) + except SQLAlchemyError: + return False + return bool(meta and meta.source_flow == "assign_entities") + + @datamanager_bp.before_request def require_login(): """Require login for all datamanager routes""" @@ -133,6 +153,13 @@ def require_login(): if login_response: return login_response + # The entities preview is used by both add-data and assign-entities flows + if ( + request.endpoint in _SHARED_FLOW_ENDPOINTS + and _request_is_assign_entities_flow() + ): + return _require_assign_entities_unlocked() + return _require_add_data_unlocked() diff --git a/application/db/models.py b/application/db/models.py index ea7edb9..5adf74a 100644 --- a/application/db/models.py +++ b/application/db/models.py @@ -24,3 +24,7 @@ class RequestMeta(db.Model): check_request_id = db.Column( db.Text, nullable=True ) # check-results request this assessment came from, for re-run routing + source_flow = db.Column( + db.Text, nullable=True + ) # "add_data" or "assign_entities" - which flow created this request, so the + # shared preview/confirm pages apply the correct process lock diff --git a/docs/datamanager/architecture.md b/docs/datamanager/architecture.md index 370ca2a..f2a40de 100644 --- a/docs/datamanager/architecture.md +++ b/docs/datamanager/architecture.md @@ -18,7 +18,8 @@ application/blueprints/datamanager/ │ ├── check.py # Check results (geometry, column mapping) and resubmit │ ├── preview.py # Entities preview and async GitHub confirm │ ├── transform.py # Transformed facts, issue logs, entity growth check -│ └── flagged_resources.py # Assign-entities: import, summary, per-resource submit +│ ├── flagged_resources.py # Assign-entities: import, summary, per-resource submit +│ └── request_meta.py # Submission-time writers for the RequestMeta table ├── services/ │ ├── async_api.py # Async request API client │ ├── dataset.py # Dataset lookups and autocomplete @@ -64,6 +65,11 @@ Controllers receive a request context and orchestrate the workflow: validate inp | `preview.py` | Entities preview loading/result page, add-data confirm (trigger GitHub workflow and show success) | | `transform.py` | Transformed facts and issue log display, entity comparison vs. platform entities, entity growth check; shared between add-data and assign-entities flows | | `flagged_resources.py` | Assign-entities flow: upload/paste flagged-resources CSV, grouped summary view, per-resource submit to async API | +| `request_meta.py` | Records per-request metadata on the `RequestMeta` table at submission time (`source_flow`, config branch baseline) that the async request's params can't carry | + +#### `RequestMeta` and `source_flow` + +The entities preview and confirm routes live under `/datamanager` but are reused by the assign-entities flow (which redirects into them rather than having its own copies). Because those two endpoints are shared, the URL prefix alone can't tell which process lock applies. So at submission time each flow records `source_flow` (`"add_data"` / `"assign_entities"`) on `RequestMeta` via `record_source_flow`; the router's lock guard and the preview render then read it back — the single source of truth for which flow a request belongs to. #### `ControllerError` diff --git a/migrations/versions/b9c0d1e2f3a4_add_request_meta_source_flow.py b/migrations/versions/b9c0d1e2f3a4_add_request_meta_source_flow.py new file mode 100644 index 0000000..e975433 --- /dev/null +++ b/migrations/versions/b9c0d1e2f3a4_add_request_meta_source_flow.py @@ -0,0 +1,26 @@ +"""add request_meta source_flow column + +Revision ID: b9c0d1e2f3a4 +Revises: a7b8c9d0e1f2 +Create Date: 2026-07-28 00:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +revision = "b9c0d1e2f3a4" +down_revision = "a7b8c9d0e1f2" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "request_meta", + sa.Column("source_flow", sa.Text(), nullable=True), + ) + + +def downgrade(): + op.drop_column("request_meta", "source_flow") diff --git a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py index 287d790..e941ff1 100644 --- a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py +++ b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py @@ -7,7 +7,7 @@ import responses as rsps from application.blueprints.base.views import ADD_DATA_LOCK, ASSIGN_ENTITIES_LOCK -from application.db.models import ServiceLock +from application.db.models import RequestMeta, ServiceLock from application.extensions import db from config.config import get_request_api_endpoint @@ -140,6 +140,110 @@ def test_assign_entities_uses_assign_entities_process_lock(client): assert "assign_entities_blocked_by=someone" in response.headers["Location"] +def _register_preview_request(request_id, params): + """Register an async request the entities-preview page can render.""" + rsps.add( + rsps.GET, + f"{ASYNC_BASE}/{request_id}", + json={ + "status": "COMPLETE", + "params": params, + "response": { + "data": { + "pipeline-summary": {}, + "endpoint-summary": {}, + "source-summary": {}, + } + }, + }, + status=200, + ) + + +def _seed_source_flow(request_id, source_flow): + """Record which flow created a request, as submission does.""" + db.session.merge(RequestMeta(request_id=request_id, source_flow=source_flow)) + db.session.commit() + + +def _clear_locks_and_meta(*request_ids): + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + for request_id in request_ids: + db.session.query(RequestMeta).filter_by(request_id=request_id).delete() + db.session.commit() + + +@rsps.activate +def test_add_data_lock_does_not_block_assign_entities_preview(client): + # The entities preview lives under /datamanager but is shared with the + # assign-entities flow. Locking Add Data must not block it for that flow. + _register_preview_request( + "assign-preview-1", + { + "dataset": "tree", + "organisation": "local-authority:ABC", + "resource": "resource-a", + }, + ) + _clear_locks_and_meta("assign-preview-1") + _seed_source_flow("assign-preview-1", "assign_entities") + db.session.add( + ServiceLock( + name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow() + ) + ) + db.session.commit() + + try: + response = client.get("/datamanager/add-data/assign-preview-1/entities") + finally: + _clear_locks_and_meta("assign-preview-1") + + assert response.status_code == 200 + + +@rsps.activate +def test_assign_entities_lock_blocks_assign_entities_preview(client): + _clear_locks_and_meta("assign-preview-2") + _seed_source_flow("assign-preview-2", "assign_entities") + db.session.add( + ServiceLock( + name=ASSIGN_ENTITIES_LOCK, locked_by="someone", locked_at=datetime.utcnow() + ) + ) + db.session.commit() + + try: + response = client.get("/datamanager/add-data/assign-preview-2/entities") + finally: + _clear_locks_and_meta("assign-preview-2") + + assert response.status_code == 302 + assert "assign_entities_blocked_by=someone" in response.headers["Location"] + + +@rsps.activate +def test_add_data_lock_still_blocks_add_data_preview(client): + # An add-data request must remain gated by Add Data. + _clear_locks_and_meta("add-preview-1") + _seed_source_flow("add-preview-1", "add_data") + db.session.add( + ServiceLock( + name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow() + ) + ) + db.session.commit() + + try: + response = client.get("/datamanager/add-data/add-preview-1/entities") + finally: + _clear_locks_and_meta("add-preview-1") + + assert response.status_code == 302 + assert "add_data_blocked_by=someone" in response.headers["Location"] + + def test_assign_entities_card_can_unlock_assign_entities_process(client): db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() diff --git a/tests/unit/blueprints/datamanager/controllers/test_add.py b/tests/unit/blueprints/datamanager/controllers/test_add.py index b0e46ce..d5192de 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_add.py +++ b/tests/unit/blueprints/datamanager/controllers/test_add.py @@ -57,6 +57,8 @@ def test_renders_old_entity_redirect_table(self, client): } }, } + db.session.add(RequestMeta(request_id="test-id", source_flow="assign_entities")) + db.session.commit() with patch( "application.blueprints.datamanager.router.fetch_request", return_value=result,