From a42dbc4de9fb2c666d31712510806731a7525f43 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Mon, 20 Jul 2026 16:21:33 +0100 Subject: [PATCH 1/6] Refactor Assign Entities workflow and update related documentation - Added detailed architecture documentation for the Assign Entities process, outlining the flow from user input to async processing and GitHub commit. - Updated existing documentation to reference the new Assign Entities architecture. - Modified the entities preview template to ensure consistent display of entity organisation CSV and old entity summary. - Enhanced acceptance tests to validate the correct handling of selected entities and redirects in the Assign Entities flow. - Adjusted unit tests to reflect changes in the handling of entity redirects, ensuring they are not passed to the GitHub workflow. - Improved logic in the duplicate candidate preparation to correctly manage auto-selection and locking of redirects based on entity selection. --- .../controllers/flagged_resources.py | 11 +- .../datamanager/controllers/preview.py | 52 ++- .../datamanager/controllers/transform.py | 260 ++++++++++- application/blueprints/datamanager/router.py | 188 ++++++++ .../blueprints/datamanager/services/github.py | 7 - .../components/check-transform-base.html | 168 +++++++ .../assign-entities-check-results.html | 83 +++- .../datamanager/entities_preview.html | 16 +- docs/assign-entities/architecture.md | 370 +++++++++++++++ docs/datamanager/architecture.md | 3 +- docs/datamanager/github-add.md | 8 +- .../datamanager/test_flagged_resources.py | 420 +++++++++++++++++- .../datamanager/controllers/test_add.py | 50 ++- .../datamanager/controllers/test_transform.py | 90 +++- .../datamanager/services/test_github.py | 18 +- 15 files changed, 1626 insertions(+), 118 deletions(-) create mode 100644 docs/assign-entities/architecture.md diff --git a/application/blueprints/datamanager/controllers/flagged_resources.py b/application/blueprints/datamanager/controllers/flagged_resources.py index 5671c2d..81fcb48 100644 --- a/application/blueprints/datamanager/controllers/flagged_resources.py +++ b/application/blueprints/datamanager/controllers/flagged_resources.py @@ -348,7 +348,12 @@ def _get_resource_organisation(resource, dataset_id): def _submit_assign_entities_request( - dataset_input, resource, organisation=None, return_endpoint=None + dataset_input, + resource, + organisation=None, + return_endpoint=None, + selected_entities=None, + selected_redirects=None, ): dataset_id, collection_id = _resolve_dataset_and_collection(dataset_input) organisation = organisation or _get_resource_organisation(resource, dataset_id) @@ -365,6 +370,10 @@ def _submit_assign_entities_request( params["organisation"] = organisation if return_endpoint: params["return_endpoint"] = return_endpoint + if selected_entities is not None: + params["selected_entities"] = selected_entities + if selected_redirects is not None: + params["selected_redirects"] = selected_redirects preview_id = submit_request(params) record_branch_baseline(preview_id, params["github_branch"]) return preview_id diff --git a/application/blueprints/datamanager/controllers/preview.py b/application/blueprints/datamanager/controllers/preview.py index 927b7d2..bccf683 100644 --- a/application/blueprints/datamanager/controllers/preview.py +++ b/application/blueprints/datamanager/controllers/preview.py @@ -1,6 +1,5 @@ import json import logging -from datetime import date from flask import ( render_template, @@ -102,8 +101,8 @@ def _load_json_list(value: str | None) -> list: return loaded if isinstance(loaded, list) else [] -def build_old_entity_redirect_table(entity_redirects: list[dict]) -> dict | None: - if not entity_redirects: +def build_old_entity_redirect_table(old_entity_rows: list[dict]) -> dict | None: + if not old_entity_rows: return None columns = [ @@ -115,22 +114,30 @@ def build_old_entity_redirect_table(entity_redirects: list[dict]) -> dict | None "entry-date", "start-date", ] - entry_date = date.today().isoformat() rows = [] - for redirect in entity_redirects: + for old_entity in old_entity_rows: + if not isinstance(old_entity, dict): + continue row = { - "old-entity": str(redirect.get("old_entity", "")), - "status": "301", - "entity": str(redirect.get("entity", "")), - "notes": str( - redirect.get("notes") - or "Redirect duplicate entity selected in Assign Entities" + "old-entity": str( + old_entity.get("old-entity", "") or old_entity.get("old_entity", "") + ), + "status": str(old_entity.get("status", "")), + "entity": str(old_entity.get("entity", "")), + "notes": str(old_entity.get("notes", "")), + "end-date": str( + old_entity.get("end-date", "") or old_entity.get("end_date", "") + ), + "entry-date": str( + old_entity.get("entry-date", "") or old_entity.get("entry_date", "") + ), + "start-date": str( + old_entity.get("start-date", "") or old_entity.get("start_date", "") ), - "end-date": "", - "entry-date": entry_date, - "start-date": "", } rows.append({"columns": {c: {"value": row[c]} for c in columns}}) + if not rows: + return None return { "columns": columns, @@ -260,10 +267,13 @@ def handle_entities_preview(request_id, req): endpoints_to_retire = ( _load_json_list(request_meta.endpoints_to_retire) if request_meta else [] ) - entity_redirects = ( - _load_json_list(request_meta.entity_redirects) if request_meta else [] + old_entity_rows = pipeline_summary.get("old-entity") or [] + old_entity_redirect_table_params = build_old_entity_redirect_table(old_entity_rows) + old_entity_redirect_count = ( + len(old_entity_redirect_table_params["rows"]) + if old_entity_redirect_table_params + else 0 ) - old_entity_redirect_table_params = build_old_entity_redirect_table(entity_redirects) existing_endpoints = ( source_summary_data.get("existing_endpoint_for_organisation_dataset") or [] ) @@ -306,9 +316,9 @@ def handle_entities_preview(request_id, req): source_flow=source_flow, return_url=return_url, retire_summary=retire_summary, - entity_redirects=entity_redirects, old_entity_redirect_table_params=old_entity_redirect_table_params, - new_count=int(pipeline_summary.get("new-in-resource") or 0), + old_entity_redirect_count=old_entity_redirect_count, + new_count=len(new_entities), existing_count=int(pipeline_summary.get("existing-in-resource") or 0), endpoint_already_exists=endpoint_already_exists, endpoint_url=endpoint_url, @@ -337,9 +347,6 @@ def handle_add_data_confirm( endpoints_to_retire = ( _load_json_list(request_meta.endpoints_to_retire) if request_meta else [] ) - entity_redirects = ( - _load_json_list(request_meta.entity_redirects) if request_meta else [] - ) # Stale-assessment guard: if the config branch has advanced for this collection # since the assessment was taken, the assigned entity numbers may now collide. @@ -386,7 +393,6 @@ def handle_add_data_confirm( triggered_by=f"{session.get('user', {}).get('login', 'unknown')}", github_branch=github_branch, endpoints_to_retire=endpoints_to_retire, - entity_redirects=entity_redirects, ) except GitHubWorkflowError as e: logger.exception(f"GitHub async workflow error: {e}") diff --git a/application/blueprints/datamanager/controllers/transform.py b/application/blueprints/datamanager/controllers/transform.py index b6bec4a..14bf854 100644 --- a/application/blueprints/datamanager/controllers/transform.py +++ b/application/blueprints/datamanager/controllers/transform.py @@ -220,6 +220,129 @@ def _build_entities_data(resp_details: list, platform_entities: list) -> dict: return {"columns": columns, "rows": rows} +def _entity_candidate_reference_key(entity: dict) -> tuple: + return ( + str(entity.get("organisation", "")).strip(), + str(entity.get("reference", "")).strip(), + ) + + +def _add_all_entity_candidates(entities_data: dict, all_entities: list) -> dict: + if not all_entities: + return entities_data + + columns = list(entities_data.get("columns") or []) + for priority_col in _ENTITY_COL_PRIORITY: + if priority_col not in columns: + columns.append(priority_col) + for entity in all_entities: + if not isinstance(entity, dict): + continue + for col in entity: + if col not in _ENTITY_COL_EXCLUDE and col not in columns: + columns.append(col) + + existing_entity_ids = { + _normalise_entity_id((row.get("fields") or {}).get("entity", "")) + for row in entities_data.get("rows", []) + if _normalise_entity_id((row.get("fields") or {}).get("entity", "")) + } + existing_reference_keys = { + _entity_candidate_reference_key(row.get("fields") or {}) + for row in entities_data.get("rows", []) + if all(_entity_candidate_reference_key(row.get("fields") or {})) + } + rows = list(entities_data.get("rows", [])) + for entity in all_entities: + if not isinstance(entity, dict): + continue + entity_id = _normalise_entity_id(entity.get("entity", "")) + reference_key = _entity_candidate_reference_key(entity) + if entity_id and entity_id in existing_entity_ids: + continue + if all(reference_key) and reference_key in existing_reference_keys: + continue + rows.append( + { + "fields": {col: str(entity.get(col, "")) for col in columns}, + "category": "new", + "changed_fields": {}, + } + ) + if entity_id: + existing_entity_ids.add(entity_id) + if all(reference_key): + existing_reference_keys.add(reference_key) + + return {"columns": columns, "rows": rows} + + +def _normalise_selected_entities(selected_entities) -> set: + """Return selected entity keys from async request params. + + A missing or empty param is handled by callers as "all new entities"; this + function returns only explicit organisation/reference selections. + """ + if not selected_entities: + return set() + if not isinstance(selected_entities, list): + return set() + return { + ( + str(entity.get("organisation", "")).strip(), + str(entity.get("reference", "")).strip(), + ) + for entity in selected_entities + if isinstance(entity, dict) + and entity.get("organisation") + and entity.get("reference") + } + + +def _entity_selection_form_value(organisation: str, reference: str) -> str: + return json.dumps( + { + "organisation": organisation, + "reference": reference, + }, + separators=(",", ":"), + ) + + +def _add_assign_entities_selection_metadata( + entities_data: dict, + organisation: str, + selected_entities, +) -> dict: + """Add checkbox metadata to Assign Entities rows. + + Only rows categorised as new can be assigned entity numbers. If async did + not receive explicit selected_entities, every selectable new row starts + checked; otherwise only matching organisation/reference pairs start checked. + """ + selected_entity_keys = _normalise_selected_entities(selected_entities) + select_all_new = not selected_entity_keys + + for row in entities_data.get("rows", []): + fields = row.get("fields") or {} + reference = str(fields.get("reference", "")).strip() + can_select = row.get("category") == "new" and bool(reference) + selected = can_select and ( + select_all_new or (organisation, reference) in selected_entity_keys + ) + row["entity_selection"] = { + "can_select": can_select, + "selected": selected, + "form_value": ( + _entity_selection_form_value(organisation, reference) + if reference + else "" + ), + } + + return entities_data + + def _entity_row_matches_search(row: dict, search_query: str) -> bool: if not search_query: return True @@ -235,26 +358,59 @@ def _entity_row_matches_filter(row: dict, category_filter: str) -> bool: return row.get("category") == category_filter -def _name_similarity_percent(value) -> float: - if value in (None, ""): - return 0 - try: - similarity = float(str(value).strip().rstrip("%")) - except (TypeError, ValueError): - return 0 - return similarity * 100 if 0 < similarity <= 1 else similarity +def _dedup_candidate_redirect_key(candidate: dict) -> tuple[str, str]: + return ( + str(candidate.get("old_entity", "") or "").strip(), + str(candidate.get("entity", "") or "").strip(), + ) -def _dedup_candidate_auto_select(candidate: dict) -> bool: - if candidate.get("old_entity_redirects"): - return False +def _old_entity_redirect_key(row: dict) -> tuple[str, str]: + return ( + str(row.get("old-entity", "") or row.get("old_entity", "") or "").strip(), + str(row.get("entity", "") or "").strip(), + ) + - match_type = str(candidate.get("match_type", "")).replace(" ", "_").lower() - if match_type == "complete_match": - return True - if match_type != "single_match": - return False - return _name_similarity_percent(candidate.get("name_similarity")) > 85 +def _dedup_candidate_selected_entity_key( + candidate: dict, organisation: str +) -> tuple[str, str]: + return ( + str( + candidate.get("new_organisation") + or candidate.get("organisation") + or organisation + or "" + ).strip(), + str( + candidate.get("new_reference", "") or candidate.get("reference", "") or "" + ).strip(), + ) + + +def _dedup_candidate_selected_redirect_key( + candidate: dict, organisation: str +) -> tuple[str, str, str]: + """Return the key used to compare a Dedup candidate with selected_redirects.""" + entity_key = _dedup_candidate_selected_entity_key(candidate, organisation) + return ( + entity_key[0], + entity_key[1], + str(candidate.get("old_entity", "") or "").strip(), + ) + + +def _selected_redirect_key(redirect: dict) -> tuple[str, str, str]: + """Return the async selected_redirects key for a submitted redirect param.""" + return ( + str(redirect.get("organisation", "") or "").strip(), + str(redirect.get("reference", "") or "").strip(), + str( + redirect.get("old_entity_number", "") + or redirect.get("old_entity", "") + or "" + ).strip(), + ) def _dedup_candidate_form_value(candidate: dict) -> str: @@ -276,11 +432,50 @@ def _dedup_candidate_form_value(candidate: dict) -> str: ) -def _prepare_duplicate_candidates(candidates: list[dict]) -> list[dict]: +def _prepare_duplicate_candidates( + candidates: list[dict], + old_entity_rows: list[dict] | None = None, + organisation: str = "", + selected_entities=None, + selected_redirects=None, +) -> list[dict]: + """Prepare Dedup candidates for rendering and selection. + + Async's ``pipeline-summary.old-entity`` is the source of initial redirect + preselection. Rows present in old-entity but absent from request + ``selected_redirects`` are inferred to be async auto-selected and are locked + in the UI so users cannot untick redirects generated by async policy. + """ + preselected_redirects = { + key + for key in ( + _old_entity_redirect_key(row) + for row in (old_entity_rows or []) + if isinstance(row, dict) + ) + if all(key) + } + selected_redirect_keys = { + _selected_redirect_key(redirect) + for redirect in (selected_redirects or []) + if isinstance(redirect, dict) and all(_selected_redirect_key(redirect)) + } + selected_entity_keys = _normalise_selected_entities(selected_entities) + select_all_new = not selected_entity_keys return [ { **candidate, - "auto_select": _dedup_candidate_auto_select(candidate), + "auto_select": _dedup_candidate_redirect_key(candidate) + in preselected_redirects, + "redirect_locked": _dedup_candidate_redirect_key(candidate) + in preselected_redirects + and _dedup_candidate_selected_redirect_key(candidate, organisation) + not in selected_redirect_keys, + "redirect_can_select": ( + select_all_new + or _dedup_candidate_selected_entity_key(candidate, organisation) + in selected_entity_keys + ), "form_value": _dedup_candidate_form_value(candidate), } for candidate in candidates @@ -337,9 +532,17 @@ def _paginate_entity_data( entity_page: int, entity_search: str, entity_filter: str = "", + include_selection: bool = False, + organisation: str = "", + selected_entities=None, + all_entities=None, ) -> tuple: entity_start_offset = (entity_page - 1) * _ROWS_PER_PAGE entities_data_full = _build_entities_data(all_resp_details, platform_entities) + if include_selection: + entities_data_full = _add_all_entity_candidates( + entities_data_full, all_entities or [] + ) # Counts cover every entity, independent of the current search/filter, so the # summary boxes always show the full picture. category_counts = _count_categories(entities_data_full["rows"]) @@ -366,6 +569,12 @@ def _paginate_entity_data( "columns": entities_data_full["columns"], "rows": entity_page_rows, } + if include_selection: + entities_data = _add_assign_entities_selection_metadata( + entities_data, + organisation, + selected_entities, + ) return ( entities_data, has_next_entity_page, @@ -618,6 +827,7 @@ def handle_check_transform( """ params = req.get("params") or {} organisation_code = params.get("organisationName") or params.get("organisation", "") + selected_entities = params.get("selected_entities") dataset_id = params.get("dataset", "") is_assign_entities = transform_endpoint == "assign_entities.flagged_resource_detail" resource_hash = params.get("resource", "") @@ -662,9 +872,14 @@ def handle_check_transform( existing_endpoints = _resolve_existing_endpoints(source_summary) pipelines_append_required = source_summary.get("pipelines_append_required") pipeline_summary = response_data.get("pipeline-summary") or {} + all_entities = pipeline_summary.get("all-entities") or [] show_dedup_tab = is_assign_entities and dataset_id == "conservation-area" duplicate_candidates = _prepare_duplicate_candidates( - pipeline_summary.get("duplicate-candidates") or [] if show_dedup_tab else [] + pipeline_summary.get("duplicate-candidates") or [] if show_dedup_tab else [], + pipeline_summary.get("old-entity") or [], + organisation=organisation_code, + selected_entities=selected_entities, + selected_redirects=params.get("selected_redirects"), ) # Calculate pagination for transformed facts and issue logs, and for entities. @@ -693,6 +908,10 @@ def handle_check_transform( entity_page, entity_search, entity_filter, + include_selection=is_assign_entities, + organisation=organisation_code, + selected_entities=selected_entities, + all_entities=all_entities, ) transformed_table = _build_transform_table(resp_details) issue_log_table = _build_issue_log_table(resp_details) @@ -746,6 +965,7 @@ def handle_check_transform( endpoint_url=endpoint_url, documentation_url=documentation_url, resource_hash=resource_hash, + is_assign_entities=is_assign_entities, show_dedup_tab=show_dedup_tab, duplicate_candidates=duplicate_candidates, planning_entity_base_url=( diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 1578524..51d58fb 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -25,6 +25,7 @@ ) from .controllers.flagged_resources import ( REQUIRED_COLUMNS, + _submit_assign_entities_request, handle_flagged_resource_detail, handle_flagged_resource_submit, handle_flagged_resources_import, @@ -311,14 +312,201 @@ def flagged_resource_detail(request_id): return render_template("datamanager/error.html", message=e.message) +def _selected_entity_key(entity): + """Return the Assign Entities selection identity for an entity row.""" + return ( + str(entity.get("organisation", "")).strip(), + str(entity.get("reference", "")).strip(), + ) + + +def _parse_selected_entities(values, new_entities): + """Validate submitted entity checkbox values against async candidate rows. + + The browser submits JSON values with only organisation and reference. Those + values are accepted only when the same pair exists in the current async + response, so hidden/form tampering cannot add extra references. + """ + valid_entities_by_key = {} + for entity in new_entities: + if not isinstance(entity, dict): + continue + key = _selected_entity_key(entity) + if all(key): + valid_entities_by_key.setdefault( + key, + { + "organisation": key[0], + "reference": key[1], + }, + ) + + selected = [] + seen = set() + for value in values: + try: + submitted = json.loads(value) + except (TypeError, json.JSONDecodeError): + continue + if not isinstance(submitted, dict): + continue + key = _selected_entity_key(submitted) + if key not in valid_entities_by_key or key in seen: + continue + selected.append(valid_entities_by_key[key]) + seen.add(key) + + return selected, list(valid_entities_by_key.values()) + + +def _current_selected_entities(current_selected_entities, all_entities): + """Expand request-param selected_entities into the current candidate rows.""" + if not current_selected_entities: + return all_entities + + current_keys = { + _selected_entity_key(entity) + for entity in current_selected_entities + if isinstance(entity, dict) + } + return [ + entity + for entity in all_entities + if _selected_entity_key(entity) in current_keys + ] + + +def _merge_visible_selected_entities( + current_entities, visible_entities, selected_visible +): + """Merge current full selection with selections from the visible page. + + Entity search and pagination mean the browser only posts rows rendered on + the current page. This keeps selections from other pages/search states and + replaces only the visible subset with the user's latest checkbox state. + """ + visible_keys = {_selected_entity_key(entity) for entity in visible_entities} + selected_visible_keys = { + _selected_entity_key(entity) for entity in selected_visible + } + selected_keys = { + _selected_entity_key(entity) + for entity in current_entities + if _selected_entity_key(entity) not in visible_keys + } | selected_visible_keys + merged = [] + seen = set() + for entity in current_entities + selected_visible: + key = _selected_entity_key(entity) + if key in selected_keys and key not in seen: + merged.append(entity) + seen.add(key) + return merged + + +def _selected_redirects_for_async(redirects, organisation, selected_entities=None): + """Map validated Dedup rows to async selected_redirects params. + + Config-manager's Dedup form values use old/new entity field names, while + async expects ``organisation``, ``reference`` and ``old_entity_number``. + When selected_entities is supplied, redirects for unselected entities are + dropped because async can only redirect entities being assigned. + """ + selected_entity_keys = ( + {_selected_entity_key(entity) for entity in selected_entities} + if selected_entities is not None + else None + ) + selected_redirects = [] + seen = set() + for redirect_row in redirects: + reference = str( + redirect_row.get("new_reference") or redirect_row.get("reference") or "" + ).strip() + redirect_organisation = str( + redirect_row.get("new_organisation") + or redirect_row.get("organisation") + or organisation + or "" + ).strip() + old_entity_number = str( + redirect_row.get("old_entity") + or redirect_row.get("old_entity_number") + or "" + ).strip() + if ( + selected_entity_keys is not None + and (redirect_organisation, reference) not in selected_entity_keys + ): + continue + key = (redirect_organisation, reference, old_entity_number) + if not all(key) or key in seen: + continue + selected_redirects.append( + { + "organisation": redirect_organisation, + "reference": reference, + "old_entity_number": old_entity_number, + } + ) + seen.add(key) + return selected_redirects + + def flagged_resource_detail_post(request_id): req = fetch_request(request_id) + params = req.get("params") or {} response_data = (req.get("response") or {}).get("data") or {} pipeline_summary = response_data.get("pipeline-summary") or {} + organisation = params.get("organisation") or params.get("organisationName") or None duplicate_candidates = pipeline_summary.get("duplicate-candidates") or [] redirects = parse_selected_redirects( request.form.getlist("entity_redirects"), duplicate_candidates ) + selectable_entities = ( + pipeline_summary.get("all-entities") + or pipeline_summary.get("new-entities") + or [] + ) + selected_entities, all_selectable_entities = _parse_selected_entities( + request.form.getlist("selected_entities"), selectable_entities + ) + if request.form.get("entity_selection_changed") == "true": + visible_entities, _ = _parse_selected_entities( + request.form.getlist("visible_selected_entities"), selectable_entities + ) + current_entities = _current_selected_entities( + params.get("selected_entities"), all_selectable_entities + ) + selected_entities = _merge_visible_selected_entities( + current_entities, visible_entities, selected_entities + ) + if not selected_entities: + raise ControllerError( + "Select at least one new entity. The async service treats an empty selection as selecting all entities." + ) + try: + new_request_id = _submit_assign_entities_request( + params.get("dataset", ""), + params.get("resource", ""), + organisation=organisation, + return_endpoint=params.get("return_endpoint") + or "assign_entities.flagged_resources_start", + selected_entities=selected_entities, + selected_redirects=_selected_redirects_for_async( + redirects, organisation, selected_entities + ), + ) + except AsyncAPIError as e: + raise ControllerError( + f"Assign entities submission failed: {e.detail}" + ) from e + return redirect( + url_for( + "assign_entities.flagged_resource_detail", request_id=new_request_id + ) + ) + meta = db.session.get(RequestMeta, request_id) if meta is None: meta = RequestMeta( diff --git a/application/blueprints/datamanager/services/github.py b/application/blueprints/datamanager/services/github.py index c98bc9f..fff8eb1 100644 --- a/application/blueprints/datamanager/services/github.py +++ b/application/blueprints/datamanager/services/github.py @@ -2,7 +2,6 @@ GitHub App service for authenticating and triggering workflows. """ -import json import time import logging import requests @@ -279,7 +278,6 @@ def trigger_add_data_async_workflow( triggered_by: str = "config-manager", github_branch: str = None, endpoints_to_retire: list = None, - entity_redirects: list = None, ) -> dict: """ Trigger the 'add-data-async-script' workflow in the digital-land/config repository. @@ -299,11 +297,6 @@ def trigger_add_data_async_workflow( "retire_endpoints": ( ",".join(endpoints_to_retire or []) if endpoints_to_retire else "" ), - "entity_redirects": ( - json.dumps(entity_redirects, separators=(",", ":")) - if entity_redirects - else "" - ), "environment": current_app.config.get("ENVIRONMENT"), }, } diff --git a/application/templates/components/check-transform-base.html b/application/templates/components/check-transform-base.html index 6014bba..b0c9625 100644 --- a/application/templates/components/check-transform-base.html +++ b/application/templates/components/check-transform-base.html @@ -131,6 +131,18 @@

{% block heading %}{% + {% if is_assign_entities %} + + {% endif %} {% for col in entities_data.columns %} {% endfor %} @@ -143,6 +155,22 @@

{% block heading %}{% {% elif row.category == 'new' %}{% set _row_bg = '#d4edda' %} {% else %}{% set _row_bg = '#d0e4f7' %}{% endif %}

+ {% if is_assign_entities %} + {% set selection = row.entity_selection or {} %} + + {% endif %} {% for col in entities_data.columns %} {% set _changed = row.changed_fields and col in row.changed_fields %}
+
+
+ + +
+
+ 		{{ col }}
+
+
+ {% if selection.can_select %} + + {% endif %} + + +
+
+ 		@@ -155,6 +183,17 @@

{% block heading %}{%

+ {% if is_assign_entities %} + {% set selected_entity_count = entities_data.rows | selectattr('entity_selection.selected') | list | length %} + {% set selectable_entity_count = entities_data.rows | selectattr('entity_selection.can_select') | list | length %} +

+ {% if selectable_entity_count %} + {{ selected_entity_count }} of {{ selectable_entity_count }} {{ 'entity' if selectable_entity_count == 1 else 'entities' }} selected for assignment + {% else %} + No entities available for assignment + {% endif %} +

+ {% endif %} {% elif entity_search or entity_filter %}

No entities match your search or filter.

{% else %} @@ -330,6 +369,135 @@

{% block heading %}{% {% endblock %} {% block pageScripts %}{{ super() }} + + {% if geometries %} {% endblock %} diff --git a/application/templates/datamanager/entities_preview.html b/application/templates/datamanager/entities_preview.html index 6a92164..ae86b66 100644 --- a/application/templates/datamanager/entities_preview.html +++ b/application/templates/datamanager/entities_preview.html @@ -30,6 +30,14 @@

Entity Summary

{{ new_count }}
+ {% if source_flow == 'assign_entities' %} +
+
+ Rows that will NOT create new entities +
+
{{ excluded_count }}
+
+ {% endif %} diff --git a/docs/assign-entities/architecture.md b/docs/assign-entities/architecture.md index 64e8c2b..9dc226f 100644 --- a/docs/assign-entities/architecture.md +++ b/docs/assign-entities/architecture.md @@ -82,7 +82,7 @@ The page is rendered from: - `response-details` rows fetched by `fetch_response_details(request_id)` - platform entities from the planning data API, for comparison and row categories - `response-details.transformed_row`, for Assign Entities rows -- request param `selected_entities`, for Assign Entities checked state +- request param `excluded_references`, for Assign Entities checked state - `pipeline-summary.duplicate-candidates`, for the Dedup tab - `pipeline-summary.old-entity`, for preselected duplicate redirects @@ -97,19 +97,19 @@ The Entities tab includes a checkbox column only in the Assign Entities flow. | `in_both` | Entity exists and matches | disabled | | `existing` | Entity exists on the platform only | disabled | -Each enabled checkbox submits this JSON value: +Each enabled checkbox submits the row reference as its value: -```json -{ "organisation": "local-authority:ABC", "reference": "REF-1" } +```text +REF-1 ``` -This organisation/reference pair is the selection identity. Entity numbers are not used as the -selection key because the point of the flow is to decide which references should receive numbers. +The reference is the selection identity. Entity numbers are not used as the selection key because +the point of the flow is to decide which references should receive numbers. Initial checkbox state comes from async request params: -- missing or empty `selected_entities`: all selectable `new` rows are checked -- non-empty `selected_entities`: only matching organisation/reference pairs are checked +- missing or empty `excluded_references`: all selectable `new` rows are checked +- non-empty `excluded_references`: matching references are unchecked Search and pagination render only a subset of rows. To avoid losing selections from other pages, the template posts the visible row values separately and `router.py` merges the visible changes back @@ -138,12 +138,12 @@ Changing either the Entities tab or the Dedup tab changes the submit button text On POST, `flagged_resource_detail_post`: 1. Fetches the current async response. -2. Parses selected entity checkbox values. -3. Validates selected entities against the current async response. +2. Parses selected entity checkbox reference values. +3. Calculates excluded references from the visible rows and current request params. 4. Parses selected Dedup checkbox values against `duplicate-candidates`. -5. Merges visible entity selections with current full selection. -6. Filters Dedup redirects to only selected entity references. -7. Submits a replacement async request with `selected_entities` and `selected_redirects`. +5. Merges visible entity selections with the current excluded references. +6. Filters Dedup redirects to exclude references that will not receive entity numbers. +7. Submits a replacement async request with `excluded_references` and `selected_redirects`. 8. Redirects to the new request's Assign Entities check-results page. The replacement request looks like: @@ -159,12 +159,11 @@ The replacement request looks like: "organisationName": "local-authority:ABC", "organisation": "local-authority:ABC", "return_endpoint": "assign_entities.flagged_resources_start", - "selected_entities": [ - { "organisation": "local-authority:ABC", "reference": "REF-1" } + "excluded_references": [ + "REF-2" ], "selected_redirects": [ { - "organisation": "local-authority:ABC", "reference": "REF-1", "old_entity_number": "100" } @@ -172,9 +171,8 @@ The replacement request looks like: } ``` -config-manager blocks submitting no selected entities from the UI. This is deliberate: async treats -`selected_entities: []` as assign all, so an empty UI selection would otherwise do the opposite of -what the user intended. +`excluded_references` contains only references that should not receive entity numbers. +An empty list means nothing was excluded from assignment. ### 6. Preview @@ -184,6 +182,7 @@ completed async output. It does not recalculate Assign Entities rows. | Preview section | Source | | --- | --- | | Rows that will create new entities | `pipeline-summary.new-entities` | +| Rows that will NOT create new entities | `params.excluded_references` | | Entity organisation CSV | `pipeline-summary.entity-organisation` | | Old Entity Summary / `old-entity.csv` | `pipeline-summary.old-entity` | @@ -212,7 +211,7 @@ the `request_id` to fetch the completed async result and appends: Entity number generation happens in async, before the result is returned to config-manager. -Selection is applied before async adds lookup entries, so unselected references do not consume +Selection is applied before async adds lookup entries, so excluded references do not consume numbers. That means selection should not create gaps in the assigned range. If gaps appear in a preview, debug async's generated `pipeline-summary.new-entities` and the branch it assessed against, not config-manager's preview code. @@ -225,10 +224,10 @@ authoritative and valid. ## Gotchas -### Empty selection means assign all in async +### Empty excluded references means assign all -Async treats missing `selected_entities` and `selected_entities: []` as "assign all new entities". -The UI prevents processing an empty selected set to avoid accidental all-entity assignment. +Async treats missing or empty `excluded_references` as "exclude nothing". The UI can send +an empty list when every selectable reference is selected. ### Transformed rows vs `new-entities` @@ -285,16 +284,16 @@ the user to re-run. Check the replacement async request params: -- Does it include a non-empty `selected_entities` array? -- Are entries shaped as `{ "organisation": "...", "reference": "..." }`? -- Do the organisation/reference values match the transformed row references? +- Does `excluded_references` contain the references that were unchecked? +- Is it a list of reference strings? +- Do the reference values match the transformed row references? -If `selected_entities` is missing or empty, async will process all new entities. +If `excluded_references` is missing or empty, async will process all new entities. ### A selected row disappeared from the Entities tab Check whether async returned the row in `response-details.transformed_row`. The Assign Entities tab -expects transformed rows for all resource entities, including unselected candidates. The preview is +expects transformed rows for all resource entities, including excluded candidates. The preview is allowed to contain only assigned rows in `pipeline-summary.new-entities`. ### A selected row does not appear in preview @@ -313,7 +312,7 @@ The preview uses those fields directly. If the row is not there, the issue is ea Check: - The candidate's `new_reference` or `reference` -- The request param `selected_entities` +- The request param `excluded_references` - The rendered row's `redirect_can_select` - Whether the row is locked because it is in `pipeline-summary.old-entity` but absent from `selected_redirects` @@ -325,7 +324,7 @@ Check the POST body from the Assign Entities page: - `entity_selection_changed` should be `true` - selected duplicate checkboxes should post as `entity_redirects` - parsed redirects must match a row in `pipeline-summary.duplicate-candidates` -- redirects for unselected entity references are deliberately filtered out +- redirects for excluded references are deliberately filtered out ### Preview old-entity count is wrong diff --git a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py index 307cb1f..005aea6 100644 --- a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py +++ b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py @@ -17,10 +17,10 @@ def _selected_entity_checkbox(response_data, reference): response_text = response_data.decode() match = re.search( - rf']*name="selected_entities"[^>]*value="[^"]*{re.escape(reference)}[^"]*"[^>]*>', + rf']*name="selected_entity_references"[^>]*value="{re.escape(reference)}"[^>]*>', response_text, ) - assert match, f"Could not find selected_entities checkbox for {reference}" + assert match, f"Could not find selected_entity_references checkbox for {reference}" return match.group(0) @@ -490,13 +490,13 @@ def test_assign_entities_check_results_does_not_show_retire_endpoints(client): assert b'action="/assign-entities/check-results/assign-id-1"' in response.data assert b'form="duplicate-redirect-form"' in response.data assert b"entity-select-all" in response.data - assert b'name="selected_entities"' in response.data - assert b'ref-2"}" form="duplicate-redirect-form" checked' in response.data + assert b'name="selected_entity_references"' in response.data + assert b'value="ref-2" form="duplicate-redirect-form" checked' in response.data assert b"1 of 1 entity selected for assignment" in response.data @rsps.activate -def test_assign_entities_check_results_uses_selected_entities_param(client): +def test_assign_entities_check_results_uses_excluded_references_param(client): rsps.add( rsps.GET, f"{ASYNC_BASE}/assign-selected-id", @@ -506,9 +506,7 @@ def test_assign_entities_check_results_uses_selected_entities_param(client): "dataset": "tree", "organisation": "local-authority:ABC", "resource": "resource-a", - "selected_entities": [ - {"organisation": "local-authority:ABC", "reference": "ref-2"} - ], + "excluded_references": ["ref-1"], }, "response": { "data": { @@ -522,18 +520,6 @@ def test_assign_entities_check_results_uses_selected_entities_param(client): "reference": "ref-2", } ], - "all-entities": [ - { - "entity": "1", - "organisation": "local-authority:ABC", - "reference": "ref-1", - }, - { - "entity": "2", - "organisation": "local-authority:ABC", - "reference": "ref-2", - }, - ], }, } }, @@ -806,6 +792,9 @@ def test_assign_entities_check_results_shows_duplicate_candidates(client): assert b"disabled" in first_checkbox assert b"entity-redirect-select-all" in response.data assert b"1 of 2 entities selected for redirection" in response.data + assert b"JSON.parse(checkbox.value" not in response.data + assert b"function entitySelectionReference(checkbox)" in response.data + assert b"entitySelectAllCheckbox.addEventListener" in response.data @rsps.activate @@ -940,9 +929,7 @@ def test_assign_entities_check_results_post_stores_redirects(client): def test_assign_entities_check_results_post_resubmits_changed_entity_selection(client): request_id = "assign-selection-id" - selected_value = json.dumps( - {"organisation": "local-authority:ABC", "reference": "ref-2"} - ) + selected_value = "ref-2" selected_redirect = json.dumps( { "old_entity": "100", @@ -953,7 +940,7 @@ def test_assign_entities_check_results_post_resubmits_changed_entity_selection(c "match_type": "complete_match", } ) - unselected_entity_redirect = json.dumps( + excluded_redirect = json.dumps( { "old_entity": "101", "entity": "201", @@ -981,16 +968,6 @@ def test_assign_entities_check_results_post_resubmits_changed_entity_selection(c "reference": "ref-2", }, ], - "all-entities": [ - { - "organisation": "local-authority:ABC", - "reference": "ref-1", - }, - { - "organisation": "local-authority:ABC", - "reference": "ref-2", - }, - ], "duplicate-candidates": [ { "old_entity": "100", @@ -1016,19 +993,14 @@ def test_assign_entities_check_results_post_resubmits_changed_entity_selection(c f"/assign-entities/check-results/{request_id}", data={ "entity_selection_changed": "true", - "visible_selected_entities": [ - json.dumps( - { - "organisation": "local-authority:ABC", - "reference": "ref-1", - } - ), + "visible_entity_references": [ + "ref-1", selected_value, ], - "selected_entities": [selected_value], + "selected_entity_references": [selected_value], "entity_redirects": [ selected_redirect, - unselected_entity_redirect, + excluded_redirect, json.dumps( { "old_entity": "999", @@ -1050,12 +1022,9 @@ def test_assign_entities_check_results_post_resubmits_changed_entity_selection(c "resource-a", organisation="local-authority:ABC", return_endpoint="assign_entities.flagged_resources_summary", - selected_entities=[ - {"organisation": "local-authority:ABC", "reference": "ref-2"} - ], + excluded_references=["ref-1"], selected_redirects=[ { - "organisation": "local-authority:ABC", "reference": "ref-2", "old_entity_number": "100", } @@ -1068,10 +1037,7 @@ def test_assign_entities_check_results_post_continues_for_unchanged_entity_selec client, ): request_id = "assign-unchanged-id" - selected_values = [ - json.dumps({"organisation": "local-authority:ABC", "reference": "ref-1"}), - json.dumps({"organisation": "local-authority:ABC", "reference": "ref-2"}), - ] + selected_values = ["ref-1", "ref-2"] with patch( "application.blueprints.datamanager.router.fetch_request", return_value={ @@ -1093,16 +1059,6 @@ def test_assign_entities_check_results_post_continues_for_unchanged_entity_selec "reference": "ref-2", }, ], - "all-entities": [ - { - "organisation": "local-authority:ABC", - "reference": "ref-1", - }, - { - "organisation": "local-authority:ABC", - "reference": "ref-2", - }, - ], "duplicate-candidates": [], } } @@ -1114,7 +1070,7 @@ def test_assign_entities_check_results_post_continues_for_unchanged_entity_selec ) as submit_request: response = client.post( f"/assign-entities/check-results/{request_id}", - data={"selected_entities": selected_values}, + data={"selected_entity_references": selected_values}, ) assert response.status_code == 302 @@ -1130,10 +1086,7 @@ def test_assign_entities_check_results_post_resubmits_changed_redirect_selection client, ): request_id = "assign-redirect-selection-id" - selected_values = [ - json.dumps({"organisation": "local-authority:ABC", "reference": "ref-1"}), - json.dumps({"organisation": "local-authority:ABC", "reference": "ref-2"}), - ] + selected_values = ["ref-1", "ref-2"] selected_redirect = json.dumps( { "old_entity": "100", @@ -1163,16 +1116,6 @@ def test_assign_entities_check_results_post_resubmits_changed_redirect_selection "reference": "ref-2", }, ], - "all-entities": [ - { - "organisation": "local-authority:ABC", - "reference": "ref-1", - }, - { - "organisation": "local-authority:ABC", - "reference": "ref-2", - }, - ], "duplicate-candidates": [ { "old_entity": "100", @@ -1193,8 +1136,8 @@ def test_assign_entities_check_results_post_resubmits_changed_redirect_selection f"/assign-entities/check-results/{request_id}", data={ "entity_selection_changed": "true", - "visible_selected_entities": selected_values, - "selected_entities": selected_values, + "visible_entity_references": selected_values, + "selected_entity_references": selected_values, "entity_redirects": [selected_redirect], }, ) @@ -1208,13 +1151,9 @@ def test_assign_entities_check_results_post_resubmits_changed_redirect_selection "resource-a", organisation="local-authority:ABC", return_endpoint="assign_entities.flagged_resources_start", - selected_entities=[ - {"organisation": "local-authority:ABC", "reference": "ref-1"}, - {"organisation": "local-authority:ABC", "reference": "ref-2"}, - ], + excluded_references=[], selected_redirects=[ { - "organisation": "local-authority:ABC", "reference": "ref-1", "old_entity_number": "100", } diff --git a/tests/unit/blueprints/datamanager/controllers/test_add.py b/tests/unit/blueprints/datamanager/controllers/test_add.py index 177076e..ff23138 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_add.py +++ b/tests/unit/blueprints/datamanager/controllers/test_add.py @@ -25,7 +25,12 @@ def test_renders_loading_template_when_pending(self, client): def test_renders_old_entity_redirect_table(self, client): result = { "status": "COMPLETE", - "params": {"dataset": "conservation-area", "authoritative": False}, + "params": { + "dataset": "conservation-area", + "authoritative": False, + "resource": "resource-a", + "excluded_references": ["not-selected", "not-selected", ""], + }, "response": { "data": { "pipeline-summary": { @@ -72,6 +77,13 @@ def test_renders_old_entity_redirect_table(self, client): ) assert b"Rows that will create new entities" in response.data assert b'
2
' in response.data + assert b"Rows that will" in response.data + assert b"NOT create new entities" in response.data + assert re.search( + rb"Rows that will.*?NOT create new entities.*?
1
", + response.data, + re.S, + ) class TestAddDataConfirmRoute: diff --git a/tests/unit/blueprints/datamanager/controllers/test_transform.py b/tests/unit/blueprints/datamanager/controllers/test_transform.py index 8e144cf..8b090ec 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_transform.py +++ b/tests/unit/blueprints/datamanager/controllers/test_transform.py @@ -90,7 +90,6 @@ def test_prepare_duplicate_candidates_does_not_lock_selected_redirect_rows(): organisation="local-authority:ABC", selected_redirects=[ { - "organisation": "local-authority:ABC", "reference": "ref-1", "old_entity_number": "100", } @@ -133,7 +132,7 @@ def test_prepare_duplicate_candidates_keeps_old_entity_field_alias(): assert candidates[0]["auto_select"] is True -def test_prepare_duplicate_candidates_disables_redirects_for_unselected_entities(): +def test_prepare_duplicate_candidates_disables_redirects_for_excluded_references(): candidates = _prepare_duplicate_candidates( [ { @@ -148,9 +147,7 @@ def test_prepare_duplicate_candidates_disables_redirects_for_unselected_entities }, ], organisation="local-authority:ABC", - selected_entities=[ - {"organisation": "local-authority:ABC", "reference": "ref-2"} - ], + excluded_references=["ref-1"], ) assert candidates[0]["redirect_can_select"] is False From 064b494fb16a5326e6b62c7cc1c7053c604932d9 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 22 Jul 2026 13:07:51 +0100 Subject: [PATCH 6/6] Remove entity_redirects from RequestMeta model and related logic, update architecture documentation, and adjust tests accordingly --- application/blueprints/datamanager/router.py | 10 ------- application/db/models.py | 1 - docs/assign-entities/architecture.md | 20 +++++--------- ...1f2a_drop_request_meta_entity_redirects.py | 26 +++++++++++++++++++ .../datamanager/test_flagged_resources.py | 19 ++------------ .../datamanager/controllers/test_add.py | 10 ------- 6 files changed, 34 insertions(+), 52 deletions(-) create mode 100644 migrations/versions/7b8c9d0e1f2a_drop_request_meta_entity_redirects.py diff --git a/application/blueprints/datamanager/router.py b/application/blueprints/datamanager/router.py index 8650b67..69b83fe 100644 --- a/application/blueprints/datamanager/router.py +++ b/application/blueprints/datamanager/router.py @@ -411,16 +411,6 @@ def flagged_resource_detail_post(request_id): ) ) - meta = db.session.get(RequestMeta, request_id) - if meta is None: - meta = RequestMeta( - request_id=request_id, - entity_redirects=json.dumps(redirects), - ) - db.session.add(meta) - else: - meta.entity_redirects = json.dumps(redirects) - db.session.commit() return redirect(url_for("datamanager.entities_preview", request_id=request_id)) diff --git a/application/db/models.py b/application/db/models.py index 2186cc7..c443058 100644 --- a/application/db/models.py +++ b/application/db/models.py @@ -448,7 +448,6 @@ class RequestMeta(db.Model): endpoints_to_retire = db.Column( db.Text, nullable=True ) # JSON list of endpoint hashes - entity_redirects = db.Column(db.Text, nullable=True) # JSON list of old-entity rows branch_sha = db.Column( db.Text, nullable=True ) # config-manager-update HEAD SHA when the assessment was submitted diff --git a/docs/assign-entities/architecture.md b/docs/assign-entities/architecture.md index 9dc226f..0034bf6 100644 --- a/docs/assign-entities/architecture.md +++ b/docs/assign-entities/architecture.md @@ -1,8 +1,8 @@ # Assign Entities - Architecture This document describes the Assign Entities process end to end: how config-manager starts async -processing, how entity and redirect selections are represented, what the preview shows, what is -committed to config, and where to look when the flow behaves unexpectedly. +processing, how entity and redirect selections are represented, what the preview shows, which +async result fields become config rows, and where to look when the flow behaves unexpectedly. Assign Entities is not a separate async request type. It is a specialised config-manager journey that submits an async `add_data` request for an existing resource hash, then reuses the add-data @@ -198,8 +198,7 @@ On confirmation, config-manager calls `trigger_add_data_async_workflow` with: - `retire_endpoints` - `environment` -It does not send entity selections or redirect selections to GitHub. The config repo workflow uses -the `request_id` to fetch the completed async result and appends: +The config repo workflow uses the `request_id` to fetch the completed async result and appends: - `pipeline/{collection}/lookup.csv` from `pipeline-summary.new-entities` - `pipeline/{collection}/entity-organisation.csv` from `pipeline-summary.entity-organisation` @@ -264,12 +263,6 @@ The preview only shows async output. If a row is missing from preview, go back t Entities check-results page and process a new selection. The preview should not be used to add or remove selected references. -### GitHub does not receive redirects - -Redirect selections are only sent to async as `selected_redirects` during **Process entities**. -GitHub confirmation sends only a request id; `old-entity.csv` rows are read by the config repo from -`pipeline-summary.old-entity`. - ### Branch state can make results stale Assign Entities uses the same stale-assessment protection as add-data. If the shared config branch @@ -329,13 +322,12 @@ Check the POST body from the Assign Entities page: ### Preview old-entity count is wrong The count is the number of rows rendered from `pipeline-summary.old-entity`. Check the async result -first. Stored `RequestMeta.entity_redirects` is not used for this preview summary. +first. ### GitHub PR does not contain old-entity rows -Check the async request fetched by the config repo workflow. The GitHub payload does not include -redirect rows. The config repo should append `old-entity.csv` from -`pipeline-summary.old-entity`. +Check `pipeline-summary.old-entity` in the completed async result fetched by the config repo +workflow. ### Entity numbers look stale or collide diff --git a/migrations/versions/7b8c9d0e1f2a_drop_request_meta_entity_redirects.py b/migrations/versions/7b8c9d0e1f2a_drop_request_meta_entity_redirects.py new file mode 100644 index 0000000..1cc5da5 --- /dev/null +++ b/migrations/versions/7b8c9d0e1f2a_drop_request_meta_entity_redirects.py @@ -0,0 +1,26 @@ +"""drop request_meta entity_redirects column + +Revision ID: 7b8c9d0e1f2a +Revises: f6a7b8c9d0e1 +Create Date: 2026-07-22 00:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +revision = "7b8c9d0e1f2a" +down_revision = "f6a7b8c9d0e1" +branch_labels = None +depends_on = None + + +def upgrade(): + op.drop_column("request_meta", "entity_redirects") + + +def downgrade(): + op.add_column( + "request_meta", + sa.Column("entity_redirects", sa.Text(), nullable=True), + ) diff --git a/tests/acceptance/blueprints/datamanager/test_flagged_resources.py b/tests/acceptance/blueprints/datamanager/test_flagged_resources.py index 005aea6..287d790 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 RequestMeta, ServiceLock +from application.db.models import ServiceLock from application.extensions import db from config.config import get_request_api_endpoint @@ -861,7 +861,7 @@ def test_assign_entities_check_results_hides_dedup_for_other_datasets(client): assert b'id="duplicates-table"' not in response.data -def test_assign_entities_check_results_post_stores_redirects(client): +def test_assign_entities_check_results_post_continues_without_storing_redirects(client): request_id = "assign-post-id" with patch( "application.blueprints.datamanager.router.fetch_request", @@ -913,18 +913,6 @@ def test_assign_entities_check_results_post_stores_redirects(client): assert response.headers["Location"].endswith( f"/datamanager/add-data/{request_id}/entities" ) - meta = db.session.get(RequestMeta, request_id) - assert json.loads(meta.entity_redirects) == [ - { - "old_entity": "100", - "entity": "200", - "dataset": "tree", - "old_reference": "old-ref", - "new_reference": "new-ref", - "match_type": "complete_match", - "notes": "Redirect duplicate entity selected in Assign Entities", - } - ] def test_assign_entities_check_results_post_resubmits_changed_entity_selection(client): @@ -1030,7 +1018,6 @@ def test_assign_entities_check_results_post_resubmits_changed_entity_selection(c } ], ) - assert db.session.get(RequestMeta, request_id) is None def test_assign_entities_check_results_post_continues_for_unchanged_entity_selection( @@ -1078,8 +1065,6 @@ def test_assign_entities_check_results_post_continues_for_unchanged_entity_selec f"/datamanager/add-data/{request_id}/entities" ) submit_request.assert_not_called() - meta = db.session.get(RequestMeta, request_id) - assert json.loads(meta.entity_redirects) == [] def test_assign_entities_check_results_post_resubmits_changed_redirect_selection( diff --git a/tests/unit/blueprints/datamanager/controllers/test_add.py b/tests/unit/blueprints/datamanager/controllers/test_add.py index ff23138..b0e46ce 100644 --- a/tests/unit/blueprints/datamanager/controllers/test_add.py +++ b/tests/unit/blueprints/datamanager/controllers/test_add.py @@ -188,16 +188,6 @@ def test_returns_error_when_workflow_raises(self, client): assert b"govuk-error-summary" in response.data def test_confirm_does_not_pass_entity_redirects_to_workflow(self, client): - db.session.add( - RequestMeta( - request_id="confirm-redirect-id", - entity_redirects=( - '[{"old_entity":"100","entity":"200",' - '"dataset":"conservation-area"}]' - ), - ) - ) - db.session.commit() with client.session_transaction() as sess: sess["user"] = {"login": "test-user"} with patch(