From 59f7211aa269530059bf8f8c8cee74267bc40a87 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Wed, 24 Apr 2024 14:21:12 +0100 Subject: [PATCH 1/5] Adding json response for 410 when triggered with extention .json --- application/routers/entity.py | 22 +++++++++------ application/templates/entity-gone.html | 39 +++++++++----------------- tests/integration/test_api.py | 2 +- tests/unit/routers/test_entity.py | 22 ++++----------- 4 files changed, 35 insertions(+), 50 deletions(-) diff --git a/application/routers/entity.py b/application/routers/entity.py index b9376650..29d0aad5 100644 --- a/application/routers/entity.py +++ b/application/routers/entity.py @@ -79,13 +79,19 @@ def get_entity( e, old_entity_status, new_entity_id = get_entity_query(session, entity) if old_entity_status == 410: - return templates.TemplateResponse( - "entity-gone.html", - { - "request": request, - "entity": str(entity), - }, - ) + if extension: + raise HTTPException( + detail=f"Entity {entity} has been removed", status_code=410 + ) + else: + return templates.TemplateResponse( + "entity-gone.html", + { + "request": request, + "entity": str(entity), + }, + status_code=410, + ) elif old_entity_status == 301: if extension: return RedirectResponse( @@ -278,7 +284,7 @@ def search_entities( columns = ["dataset", "name", "plural", "typology", "themes", "paint_options"] datasets = [dataset.dict(include=set(columns)) for dataset in response] - local_authorities = get_local_authorities(session, "local-authority") + local_authorities = get_local_authorities(session, "local-authority-eng") local_authorities = [la.dict() for la in local_authorities] if links.get("prev") is not None: diff --git a/application/templates/entity-gone.html b/application/templates/entity-gone.html index b75266bf..824bdbb7 100644 --- a/application/templates/entity-gone.html +++ b/application/templates/entity-gone.html @@ -1,35 +1,24 @@ {% extends "layouts/layout.html" %} +{% set mainClasses = "govuk-main-wrapper--l" %} {% set templateName = "dl-info/entity-gone.html" %} {%- block content %} - {% block recordHead %} - {% block recordTitle -%} - Entity Removed -

Entity Removed

- {%- endblock recordTitle %} - {% endblock recordHead %} +
+
+

Entity Removed

This entity (#{{entity}}) has been removed.

- {%- endblock content %} - - {% block recordEnd %}{% endblock recordEnd %} - - {% block footerStart %} - {%- from "components/feedback/macro.jinja" import dlFeedback %} - {% set subject = "Feedback on removed entity with number " + entity %} - {{ dlFeedback({ - "text": "Spotted an issue? Let us know so we can improve the data.", - "action": { - "text": "There is something wrong with the data", - "href": "mailto:" + templateVar.email + "?subject=" + subject - }, - "container": true - }) - }} - {% endblock footerStart %} - - {%- block pageScripts %}{%- endblock %} + {%- from "components/feedback/macro.jinja" import dlFeedback %} + {% set subject = "Feedback on removed entity with number " + entity %} +
+ Spotted an issue? You can + contact the Planning Data team + if you need to speak to someone about this page. +
+
+
+{%- endblock %} diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index 58d054cd..a7d04539 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -120,7 +120,7 @@ def test_old_entity_gone_shown(test_data_old_entities, client, exclude_middlewar """ old_entity = test_data_old_entities["old_entities"][410][0] response = client.get(f"/entity/{old_entity.old_entity_id}", allow_redirects=False) - assert response.status_code == 200 + assert response.status_code == 410 assert ( f"This entity (#{old_entity.old_entity_id}) has been removed." in response.text ) diff --git a/tests/unit/routers/test_entity.py b/tests/unit/routers/test_entity.py index bdecb330..f067d4a2 100644 --- a/tests/unit/routers/test_entity.py +++ b/tests/unit/routers/test_entity.py @@ -212,16 +212,11 @@ def test_get_entity_old_entity_gone_returned_json(mocker): request = MagicMock() extension = MagicMock() extension.value = "json" - result = get_entity(request=request, entity="11000000", extension=extension) try: - result.template.render(result.context) + get_entity(request=request, entity="11000000", extension=extension) + assert False, "Expected HTTPException to be raised" + except HTTPException: assert True - except Exception: - if hasattr(result, "context"): - logging.warning(f"context:{result.context}") - else: - logging.warning("result has no context") - assert False, "template unable to render, missing variable(s) from context" def test_get_entity_old_entity_gone_returned_geojson(mocker): @@ -231,16 +226,11 @@ def test_get_entity_old_entity_gone_returned_geojson(mocker): request = MagicMock() extension = MagicMock() extension.value = "geojson" - result = get_entity(request=request, entity="11000000", extension=extension) try: - result.template.render(result.context) + get_entity(request=request, entity="11000000", extension=extension) + assert False, "Expected HTTPException to be raised" + except HTTPException: assert True - except Exception: - if hasattr(result, "context"): - logging.warning(f"context:{result.context}") - else: - logging.warning("result has no context") - assert False, "template unable to render, missing variable(s) from context" def test_get_entity_old_entity_redirect_returned_html(mocker): From 74b5c5b45812ab2a8586c9cfffbbc3dfda8f7c10 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Thu, 25 Apr 2024 16:58:52 +0100 Subject: [PATCH 2/5] removing dlfeedback --- application/templates/entity-gone.html | 1 - 1 file changed, 1 deletion(-) diff --git a/application/templates/entity-gone.html b/application/templates/entity-gone.html index 824bdbb7..b348452c 100644 --- a/application/templates/entity-gone.html +++ b/application/templates/entity-gone.html @@ -12,7 +12,6 @@

Entity Removed

This entity (#{{entity}}) has been removed.

- {%- from "components/feedback/macro.jinja" import dlFeedback %} {% set subject = "Feedback on removed entity with number " + entity %}
Spotted an issue? You can From 8fe6e99771fe95d63884a6eb64206c6fdb71b88f Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Fri, 26 Apr 2024 12:09:24 +0100 Subject: [PATCH 3/5] Adding asserts to check the format returned and check the cotnent when 404 is returned --- tests/integration/test_api.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index a7d04539..01952099 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -124,6 +124,24 @@ def test_old_entity_gone_shown(test_data_old_entities, client, exclude_middlewar assert ( f"This entity (#{old_entity.old_entity_id}) has been removed." in response.text ) + assert ( + "text/html" in response.headers["Content-Type"] + ), "Expected response in text/html format" + + +def test_old_entity_gone_json_shown(test_data_old_entities, client, exclude_middleware): + """ + Test entity endpoint returns entity gone content + """ + old_entity = test_data_old_entities["old_entities"][410][0] + response = client.get( + f"/entity/{old_entity.old_entity_id}.json", allow_redirects=False + ) + assert response.status_code == 410 + assert ( + response.headers["Content-Type"] == "application/json" + ), "Expected response in JSON format" + assert f"Entity {old_entity.old_entity_id} has been removed" in response.text def test_dataset_json_endpoint_returns_as_expected(test_data, client): @@ -308,6 +326,9 @@ def test_get_by_curie_redirects_to_entity(test_data, client, exclude_middleware) def test_get_by_curie_404s_for_unknown_reference(test_data, client, exclude_middleware): response = client.get("/curie/not:found", allow_redirects=False) assert response.status_code == 404 + expected_content = "Page not found" + # Check if the expected content is present in the response body + assert expected_content in response.text def test_get_dataset_unknown_returns_404(client, exclude_middleware): From f0ab7704a797c72e6a7b186e96d79de27acb9205 Mon Sep 17 00:00:00 2001 From: Samriti Sadhu Date: Mon, 29 Apr 2024 09:54:43 +0100 Subject: [PATCH 4/5] Correcting code indents and removing eng --- application/routers/entity.py | 5 +++-- application/templates/entity-gone.html | 18 +++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/application/routers/entity.py b/application/routers/entity.py index 29d0aad5..7c7b0388 100644 --- a/application/routers/entity.py +++ b/application/routers/entity.py @@ -81,7 +81,8 @@ def get_entity( if old_entity_status == 410: if extension: raise HTTPException( - detail=f"Entity {entity} has been removed", status_code=410 + detail=f"Entity {entity} has been removed", + status_code=410, ) else: return templates.TemplateResponse( @@ -284,7 +285,7 @@ def search_entities( columns = ["dataset", "name", "plural", "typology", "themes", "paint_options"] datasets = [dataset.dict(include=set(columns)) for dataset in response] - local_authorities = get_local_authorities(session, "local-authority-eng") + local_authorities = get_local_authorities(session, "local-authority") local_authorities = [la.dict() for la in local_authorities] if links.get("prev") is not None: diff --git a/application/templates/entity-gone.html b/application/templates/entity-gone.html index b348452c..7d5161c7 100644 --- a/application/templates/entity-gone.html +++ b/application/templates/entity-gone.html @@ -8,16 +8,16 @@

Entity Removed

-

- This entity (#{{entity}}) has been removed. -

+

+ This entity (#{{ entity }}) has been removed. +

- {% set subject = "Feedback on removed entity with number " + entity %} -
- Spotted an issue? You can - contact the Planning Data team - if you need to speak to someone about this page. + {% set subject = "Feedback on removed entity with number " + entity %} +
+ Spotted an issue? You can + contact the Planning Data team + if you need to speak to someone about this page. +
-
{%- endblock %} From b0a51ee1713490c779a0619f0d5e695678b5dcd9 Mon Sep 17 00:00:00 2001 From: eveleighoj <35256612+eveleighoj@users.noreply.github.com> Date: Mon, 29 Apr 2024 17:09:16 +0100 Subject: [PATCH 5/5] Update entity-gone.html --- application/templates/entity-gone.html | 29 +++++++++++++------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/application/templates/entity-gone.html b/application/templates/entity-gone.html index 7d5161c7..5ef005a6 100644 --- a/application/templates/entity-gone.html +++ b/application/templates/entity-gone.html @@ -3,21 +3,20 @@ {% set templateName = "dl-info/entity-gone.html" %} {%- block content %} - -
-
-

Entity Removed

- -

- This entity (#{{ entity }}) has been removed. -

- - {% set subject = "Feedback on removed entity with number " + entity %} -
- Spotted an issue? You can - contact the Planning Data team - if you need to speak to someone about this page. +
+
+

Entity Removed

+ +

+ This entity (#{{ entity }}) has been removed. +

+ + {% set subject = "Feedback on removed entity with number " + entity %} +
+ Spotted an issue? You can + contact the Planning Data team + if you need to speak to someone about this page. +
-
{%- endblock %}