From e316703d774b74eed303c89dec4939ff5d025a73 Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Mon, 27 Jul 2026 13:40:52 +0100 Subject: [PATCH 1/8] shared flow endpoints need to not be blocked by locks --- .../datamanager/controllers/preview.py | 13 ++- application/blueprints/datamanager/router.py | 33 ++++++ .../datamanager/test_flagged_resources.py | 100 ++++++++++++++++++ 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/application/blueprints/datamanager/controllers/preview.py b/application/blueprints/datamanager/controllers/preview.py index 7d722bc..f0aafaa 100644 --- a/application/blueprints/datamanager/controllers/preview.py +++ b/application/blueprints/datamanager/controllers/preview.py @@ -90,6 +90,13 @@ def _build_entity_organisation_summary(new_entities, authoritative, pipeline_sum entity_org_error_warning, ) +# Used by router to try and variable lock/unlock this page +def determine_source_flow(params: dict) -> str: + params = params or {} + if params.get("resource") and not params.get("url"): + return "assign_entities" + return "add_data" + def _load_json_list(value: str | None) -> list: if not value: @@ -258,11 +265,7 @@ 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" - ) + source_flow = determine_source_flow(params) return_endpoint = params.get("return_endpoint") if return_endpoint: return_url = url_for(return_endpoint) diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 69b83fe..6b05e15 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -38,6 +38,7 @@ handle_check_resubmit, ) from .controllers.preview import ( + determine_source_flow, handle_entities_preview, handle_add_data_confirm, ) @@ -52,6 +53,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" @@ -125,6 +134,23 @@ def _require_assign_entities_unlocked(): url_for("base.index", assign_entities_blocked_by=lock.locked_by) ) +def _request_is_assign_entities_flow(): + """Best-effort detection of whether the current request is part of the + assign-entities flow rather than add-data. + """ + form_flow = request.form.get("source_flow") + if form_flow: + return form_flow == "assign_entities" + + request_id = (request.view_args or {}).get("request_id") + if not request_id: + return False + try: + req = fetch_request(request_id) + except AsyncAPIError: + return False + return determine_source_flow(req.get("params") or {}) == "assign_entities" + @datamanager_bp.before_request def require_login(): @@ -133,6 +159,13 @@ def require_login(): if login_response: return login_response + # The entities preview is used by both add-data and assign-entities flows, so the applicable lock depends on which flow the request belongs to. + 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/tests/acceptance/blueprints/datamanager/test_flagged_resources.py b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py index 287d790..fe18e0c 100644 --- a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py +++ b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py @@ -140,6 +140,106 @@ 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, + ) + + +@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"}, + ) + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + 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: + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + db.session.commit() + + assert response.status_code == 200 + + +@rsps.activate +def test_assign_entities_lock_blocks_assign_entities_preview(client): + _register_preview_request( + "assign-preview-2", + {"dataset": "tree", "organisation": "local-authority:ABC", "resource": "resource-a"}, + ) + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + 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: + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + db.session.commit() + + 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 (carries a source url) must remain gated by Add Data. + _register_preview_request( + "add-preview-1", + { + "dataset": "tree", + "organisation": "local-authority:ABC", + "url": "https://example.com/data.csv", + }, + ) + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + 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: + db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() + db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + db.session.commit() + + 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() From 64f883ce51dd9f79fdbbeae79fbf3b4dd783ffb2 Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Mon, 27 Jul 2026 13:43:36 +0100 Subject: [PATCH 2/8] lint --- .../datamanager/controllers/preview.py | 1 + application/blueprints/datamanager/router.py | 1 + .../datamanager/test_flagged_resources.py | 20 +++++++++++++++---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/application/blueprints/datamanager/controllers/preview.py b/application/blueprints/datamanager/controllers/preview.py index f0aafaa..87d9ff5 100644 --- a/application/blueprints/datamanager/controllers/preview.py +++ b/application/blueprints/datamanager/controllers/preview.py @@ -90,6 +90,7 @@ def _build_entity_organisation_summary(new_entities, authoritative, pipeline_sum entity_org_error_warning, ) + # Used by router to try and variable lock/unlock this page def determine_source_flow(params: dict) -> str: params = params or {} diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 6b05e15..410dba7 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -134,6 +134,7 @@ def _require_assign_entities_unlocked(): url_for("base.index", assign_entities_blocked_by=lock.locked_by) ) + def _request_is_assign_entities_flow(): """Best-effort detection of whether the current request is part of the assign-entities flow rather than add-data. diff --git a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py index fe18e0c..02d8432 100644 --- a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py +++ b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py @@ -166,12 +166,18 @@ def test_add_data_lock_does_not_block_assign_entities_preview(client): # 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"}, + { + "dataset": "tree", + "organisation": "local-authority:ABC", + "resource": "resource-a", + }, ) db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() db.session.add( - ServiceLock(name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow()) + ServiceLock( + name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow() + ) ) db.session.commit() @@ -189,7 +195,11 @@ def test_add_data_lock_does_not_block_assign_entities_preview(client): def test_assign_entities_lock_blocks_assign_entities_preview(client): _register_preview_request( "assign-preview-2", - {"dataset": "tree", "organisation": "local-authority:ABC", "resource": "resource-a"}, + { + "dataset": "tree", + "organisation": "local-authority:ABC", + "resource": "resource-a", + }, ) db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() @@ -225,7 +235,9 @@ def test_add_data_lock_still_blocks_add_data_preview(client): db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() db.session.add( - ServiceLock(name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow()) + ServiceLock( + name=ADD_DATA_LOCK, locked_by="someone", locked_at=datetime.utcnow() + ) ) db.session.commit() From 2015fd98a974fb7e74fbcff6099a9e29e91b08d3 Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Mon, 27 Jul 2026 13:47:03 +0100 Subject: [PATCH 3/8] lint --- application/blueprints/datamanager/router.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 410dba7..e804a13 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -160,7 +160,7 @@ def require_login(): if login_response: return login_response - # The entities preview is used by both add-data and assign-entities flows, so the applicable lock depends on which flow the request belongs to. + # 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() From 652de7584503ebb3aba787966ddfc2cfbbfbd497 Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Mon, 27 Jul 2026 13:54:41 +0100 Subject: [PATCH 4/8] use endpoint for query if assign entities flow --- application/blueprints/datamanager/router.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index e804a13..7a12bb2 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -139,10 +139,10 @@ def _request_is_assign_entities_flow(): """Best-effort detection of whether the current request is part of the assign-entities flow rather than add-data. """ - form_flow = request.form.get("source_flow") - if form_flow: - return form_flow == "assign_entities" - + # The confirm POST always carries the originating flow as a hidden field + if request.endpoint == "datamanager.add_data_confirm_async": + return request.form.get("source_flow") == "assign_entities" + request_id = (request.view_args or {}).get("request_id") if not request_id: return False From 1c095304a20f064d3406ea7c978544d108cdf539 Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Mon, 27 Jul 2026 14:05:04 +0100 Subject: [PATCH 5/8] lint --- application/blueprints/datamanager/router.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 7a12bb2..f99501d 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -142,7 +142,7 @@ def _request_is_assign_entities_flow(): # The confirm POST always carries the originating flow as a hidden field if request.endpoint == "datamanager.add_data_confirm_async": return request.form.get("source_flow") == "assign_entities" - + request_id = (request.view_args or {}).get("request_id") if not request_id: return False From 71e2a2a311448061ccafc18bbbd5007f561aff5b Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Tue, 28 Jul 2026 14:39:48 +0100 Subject: [PATCH 6/8] simplify this by adding source flow to request meta --- .../controllers/flagged_resources.py | 3 +- .../datamanager/controllers/form.py | 3 +- .../datamanager/controllers/preview.py | 47 +------------ .../datamanager/controllers/request_meta.py | 67 +++++++++++++++++++ application/blueprints/datamanager/router.py | 14 ++-- application/db/models.py | 4 ++ ...c0d1e2f3a4_add_request_meta_source_flow.py | 26 +++++++ .../datamanager/test_flagged_resources.py | 58 +++++++--------- .../datamanager/controllers/test_add.py | 4 ++ 9 files changed, 136 insertions(+), 90 deletions(-) create mode 100644 application/blueprints/datamanager/controllers/request_meta.py create mode 100644 migrations/versions/b9c0d1e2f3a4_add_request_meta_source_flow.py 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 87d9ff5..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 @@ -91,14 +89,6 @@ def _build_entity_organisation_summary(new_entities, authoritative, pipeline_sum ) -# Used by router to try and variable lock/unlock this page -def determine_source_flow(params: dict) -> str: - params = params or {} - if params.get("resource") and not params.get("url"): - return "assign_entities" - return "add_data" - - def _load_json_list(value: str | None) -> list: if not value: return [] @@ -164,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") @@ -266,7 +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 = determine_source_flow(params) + 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) @@ -276,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 f99501d..060512d 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -38,7 +38,6 @@ handle_check_resubmit, ) from .controllers.preview import ( - determine_source_flow, handle_entities_preview, handle_add_data_confirm, ) @@ -136,21 +135,16 @@ def _require_assign_entities_unlocked(): def _request_is_assign_entities_flow(): - """Best-effort detection of whether the current request is part of the - assign-entities flow rather than add-data. + """Whether a shared datamanager route belongs to the assign-entities flow. """ - # The confirm POST always carries the originating flow as a hidden field - if request.endpoint == "datamanager.add_data_confirm_async": - return request.form.get("source_flow") == "assign_entities" - request_id = (request.view_args or {}).get("request_id") if not request_id: return False try: - req = fetch_request(request_id) - except AsyncAPIError: + meta = db.session.get(RequestMeta, request_id) + except SQLAlchemyError: return False - return determine_source_flow(req.get("params") or {}) == "assign_entities" + return bool(meta and meta.source_flow == "assign_entities") @datamanager_bp.before_request diff --git a/application/db/models.py b/application/db/models.py index c443058..a3d6272 100644 --- a/application/db/models.py +++ b/application/db/models.py @@ -454,6 +454,10 @@ 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 class Filter(DateModel, VersionedMixin): 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..93eeda3 --- /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: 7b8c9d0e1f2a +Create Date: 2026-07-28 00:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +revision = "b9c0d1e2f3a4" +down_revision = "7b8c9d0e1f2a" +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 02d8432..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 @@ -160,6 +160,20 @@ def _register_preview_request(request_id, params): ) +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 @@ -172,8 +186,8 @@ def test_add_data_lock_does_not_block_assign_entities_preview(client): "resource": "resource-a", }, ) - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + _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() @@ -184,25 +198,15 @@ def test_add_data_lock_does_not_block_assign_entities_preview(client): try: response = client.get("/datamanager/add-data/assign-preview-1/entities") finally: - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() - db.session.commit() + _clear_locks_and_meta("assign-preview-1") assert response.status_code == 200 @rsps.activate def test_assign_entities_lock_blocks_assign_entities_preview(client): - _register_preview_request( - "assign-preview-2", - { - "dataset": "tree", - "organisation": "local-authority:ABC", - "resource": "resource-a", - }, - ) - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + _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() @@ -213,9 +217,7 @@ def test_assign_entities_lock_blocks_assign_entities_preview(client): try: response = client.get("/datamanager/add-data/assign-preview-2/entities") finally: - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() - db.session.commit() + _clear_locks_and_meta("assign-preview-2") assert response.status_code == 302 assert "assign_entities_blocked_by=someone" in response.headers["Location"] @@ -223,17 +225,9 @@ def test_assign_entities_lock_blocks_assign_entities_preview(client): @rsps.activate def test_add_data_lock_still_blocks_add_data_preview(client): - # An add-data request (carries a source url) must remain gated by Add Data. - _register_preview_request( - "add-preview-1", - { - "dataset": "tree", - "organisation": "local-authority:ABC", - "url": "https://example.com/data.csv", - }, - ) - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() + # 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() @@ -244,9 +238,7 @@ def test_add_data_lock_still_blocks_add_data_preview(client): try: response = client.get("/datamanager/add-data/add-preview-1/entities") finally: - db.session.query(ServiceLock).filter_by(name=ADD_DATA_LOCK).delete() - db.session.query(ServiceLock).filter_by(name=ASSIGN_ENTITIES_LOCK).delete() - db.session.commit() + _clear_locks_and_meta("add-preview-1") assert response.status_code == 302 assert "add_data_blocked_by=someone" in response.headers["Location"] diff --git a/tests/unit/blueprints/datamanager/controllers/test_add.py b/tests/unit/blueprints/datamanager/controllers/test_add.py index b0e46ce..799faf5 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_add.py +++ b/tests/unit/blueprints/datamanager/controllers/test_add.py @@ -57,6 +57,10 @@ 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, From e10ff6faa936662c9e7977cc759559e6e5db249e Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Tue, 28 Jul 2026 15:07:35 +0100 Subject: [PATCH 7/8] lint --- application/blueprints/datamanager/router.py | 3 +-- tests/unit/blueprints/datamanager/controllers/test_add.py | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 060512d..a7445ad 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -135,8 +135,7 @@ def _require_assign_entities_unlocked(): def _request_is_assign_entities_flow(): - """Whether a shared datamanager route belongs to the 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 diff --git a/tests/unit/blueprints/datamanager/controllers/test_add.py b/tests/unit/blueprints/datamanager/controllers/test_add.py index 799faf5..d5192de 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_add.py +++ b/tests/unit/blueprints/datamanager/controllers/test_add.py @@ -57,9 +57,7 @@ def test_renders_old_entity_redirect_table(self, client): } }, } - db.session.add( - RequestMeta(request_id="test-id", source_flow="assign_entities") - ) + db.session.add(RequestMeta(request_id="test-id", source_flow="assign_entities")) db.session.commit() with patch( "application.blueprints.datamanager.router.fetch_request", From 26972c7ae3a1db7698226182d8d86501e89a092e Mon Sep 17 00:00:00 2001 From: Matt Poole Date: Tue, 28 Jul 2026 15:10:11 +0100 Subject: [PATCH 8/8] docs --- docs/datamanager/architecture.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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`