diff --git a/application/blueprints/datamanager/controllers/preview.py b/application/blueprints/datamanager/controllers/preview.py index 2e7406a..8ea42ac 100644 --- a/application/blueprints/datamanager/controllers/preview.py +++ b/application/blueprints/datamanager/controllers/preview.py @@ -29,6 +29,64 @@ logger = logging.getLogger(__name__) +def _build_entity_organisation_summary(new_entities, authoritative, pipeline_summary): + """ + Build entity-organisation CSV preview context - only relevant when new + entities were actually created; otherwise there is nothing to map. + + Returns (entity_org_table_params, has_entity_org, entity_org_warning, + entity_org_overlap_info, entity_org_error_warning) + """ + entity_org_table_params = None + has_entity_org = False + entity_org_warning = None + entity_org_overlap_info = None + entity_org_error_warning = None + + if not new_entities: + return ( + entity_org_table_params, + has_entity_org, + entity_org_warning, + entity_org_overlap_info, + entity_org_error_warning, + ) + + if not authoritative: + entity_org_warning = "Non-authoritative data being submitted" + return ( + entity_org_table_params, + has_entity_org, + entity_org_warning, + entity_org_overlap_info, + entity_org_error_warning, + ) + + entity_organisation_data = pipeline_summary.get("entity-organisation") or [] + if entity_organisation_data: + entry = entity_organisation_data[0] + if entry.get("overlap"): + entity_org_overlap_info = "Entity org already exists - no action needed" + elif entry.get("error"): + entity_org_error_warning = ( + "An error occurred creating the entity-organisation csv, " + "please re-run if you believe this is required" + ) + else: + ( + entity_org_table_params, + has_entity_org, + ) = build_entity_organisation_csv(entity_organisation_data) + + return ( + entity_org_table_params, + has_entity_org, + entity_org_warning, + entity_org_overlap_info, + entity_org_error_warning, + ) + + def _load_json_list(value: str | None) -> list: if not value: return [] @@ -191,23 +249,17 @@ def handle_entities_preview(request_id, req): } ) - # Build entity-organisation CSV preview (only for authoritative data) + # Build entity-organisation CSV preview authoritative = params.get("authoritative", False) - entity_org_table_params = None - has_entity_org = False - entity_org_warning = None - - if authoritative: - entity_organisation_data = pipeline_summary.get("entity-organisation") or [] - if entity_organisation_data: - ( - entity_org_table_params, - has_entity_org, - ) = build_entity_organisation_csv(entity_organisation_data) - else: - entity_org_warning = ( - "This must be manually created currently for non-authoritative data" - ) + ( + entity_org_table_params, + has_entity_org, + entity_org_warning, + entity_org_overlap_info, + entity_org_error_warning, + ) = _build_entity_organisation_summary( + new_entities, authoritative, pipeline_summary + ) return render_template( "datamanager/entities_preview.html", @@ -232,6 +284,8 @@ def handle_entities_preview(request_id, req): entity_org_table_params=entity_org_table_params, has_entity_org=has_entity_org, entity_org_warning=entity_org_warning, + entity_org_overlap_info=entity_org_overlap_info, + entity_org_error_warning=entity_org_error_warning, ) diff --git a/application/blueprints/datamanager/services/github-add.md b/application/blueprints/datamanager/services/github-add.md index 2d22a65..f393dcc 100644 --- a/application/blueprints/datamanager/services/github-add.md +++ b/application/blueprints/datamanager/services/github-add.md @@ -88,7 +88,7 @@ The workflow fetches request data from: {ASYNC_API_BASE_URL}/requests/{request_id} ``` -**Default base URL:** `http://development-pub-async-api-lb-69142969.eu-west-2.elb.amazonaws.com` +**Default base URL:** `https://pub-async.development.planning.data.gov.uk` To override, set the `ASYNC_API_BASE_URL` repository variable in GitHub Settings > Secrets and variables > Actions > Variables. diff --git a/application/templates/datamanager/entities_preview.html b/application/templates/datamanager/entities_preview.html index 0c592a1..5ed6cc6 100644 --- a/application/templates/datamanager/entities_preview.html +++ b/application/templates/datamanager/entities_preview.html @@ -135,7 +135,7 @@

column.csv (new mappings)

{% endif %} - {% if entity_org_warning or (has_entity_org and entity_org_table_params) %} + {% if entity_org_warning or entity_org_overlap_info or entity_org_error_warning or (has_entity_org and entity_org_table_params) %}

Entity Org Summary

@@ -148,7 +148,17 @@

entity-organisation.csv

{{ entity_org_warning }}
- {% else %} + {% elif entity_org_overlap_info %} +
+ {{ entity_org_overlap_info }} +
+ {% elif entity_org_error_warning %} + + {% elif has_entity_org and entity_org_table_params %}
{{ table(entity_org_table_params) }}
diff --git a/config/config.py b/config/config.py index 2cf1d1d..7648c3e 100644 --- a/config/config.py +++ b/config/config.py @@ -101,8 +101,8 @@ def get_request_api_endpoint(): mapping = { "local": "http://localhost:8000", "development": "https://pub-async.development.planning.data.gov.uk", - "staging": "http://staging-pub-async-api-lb-12493311.eu-west-2.elb.amazonaws.com", - "production": "http://production-pub-async-api-lb-636110663.eu-west-2.elb.amazonaws.com", + "staging": "https://pub-async.staging.planning.data.gov.uk", + "production": "https://pub-async.planning.data.gov.uk", } return mapping.get(env, mapping["local"]) diff --git a/tests/unit/blueprints/datamanager/controllers/test_preview.py b/tests/unit/blueprints/datamanager/controllers/test_preview.py new file mode 100644 index 0000000..83efff4 --- /dev/null +++ b/tests/unit/blueprints/datamanager/controllers/test_preview.py @@ -0,0 +1,126 @@ +from application.blueprints.datamanager.controllers.preview import ( + _build_entity_organisation_summary, +) + +NEW_ENTITIES = [{"entity": "10100002", "reference": "REF001"}] + + +def test_no_new_entities_hides_section(): + result = _build_entity_organisation_summary([], True, {"entity-organisation": []}) + + assert result == (None, False, None, None, None) + + +def test_non_authoritative_is_informational_only(): + """Non-authoritative just flags the data as such - nothing needs to be created.""" + ( + table_params, + has_entity_org, + warning, + overlap_info, + error_warning, + ) = _build_entity_organisation_summary( + NEW_ENTITIES, False, {"entity-organisation": []} + ) + + assert table_params is None + assert has_entity_org is False + assert warning == "Non-authoritative data being submitted" + assert overlap_info is None + assert error_warning is None + + +def test_authoritative_overlap_shows_info_message_only(): + """Overlap is informational, not a warning, and the table is skipped.""" + pipeline_summary = { + "entity-organisation": [ + { + "dataset": "nature-improvement-area", + "organisation": "government-organisation:PB202", + "overlap": True, + "error": False, + } + ] + } + + ( + table_params, + has_entity_org, + warning, + overlap_info, + error_warning, + ) = _build_entity_organisation_summary(NEW_ENTITIES, True, pipeline_summary) + + assert has_entity_org is False + assert table_params is None + assert warning is None + assert overlap_info == "Entity org already exists - no action needed" + assert error_warning is None + + +def test_authoritative_error_shows_error_message_only(): + """Error skips the table too, since there's no trustworthy range to show.""" + pipeline_summary = { + "entity-organisation": [ + { + "dataset": "nature-improvement-area", + "organisation": "government-organisation:PB202", + "overlap": False, + "error": True, + } + ] + } + + ( + table_params, + has_entity_org, + warning, + overlap_info, + error_warning, + ) = _build_entity_organisation_summary(NEW_ENTITIES, True, pipeline_summary) + + assert has_entity_org is False + assert table_params is None + assert warning is None + assert overlap_info is None + assert error_warning == ( + "An error occurred creating the entity-organisation csv, " + "please re-run if you believe this is required" + ) + + +def test_authoritative_no_overlap_or_error_shows_table(): + pipeline_summary = { + "entity-organisation": [ + { + "dataset": "nature-improvement-area", + "entity-minimum": 10100002, + "entity-maximum": 10100002, + "organisation": "government-organisation:PB202", + "overlap": False, + "error": False, + } + ] + } + + ( + table_params, + has_entity_org, + warning, + overlap_info, + error_warning, + ) = _build_entity_organisation_summary(NEW_ENTITIES, True, pipeline_summary) + + assert has_entity_org is True + assert table_params is not None + assert warning is None + assert overlap_info is None + assert error_warning is None + + +def test_authoritative_no_entity_organisation_data_hides_section(): + result = _build_entity_organisation_summary( + NEW_ENTITIES, True, {"entity-organisation": []} + ) + + assert result == (None, False, None, None, None)