From fc92dc1308a16a263aadc6c563b1755e548d4d97 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Feb 2023 10:29:00 +0000 Subject: [PATCH 1/7] Internally rename 'detail' view to 'full text HTML' This makes it clearer what this function is actually doing --- .../templates/includes/judgment_text_toolbar.html | 2 +- .../templates/includes/judgments_list_item.html | 2 +- .../templates/judgment/confirm-unlock.html | 2 +- ds_caselaw_editor_ui/templates/judgment/edit.html | 2 +- judgments/urls.py | 4 ++-- judgments/views/edit_judgment.py | 10 +++++----- judgments/views/{detail.py => full_text.py} | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) rename judgments/views/{detail.py => full_text.py} (98%) diff --git a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html index 2a28ef35e..c2c50c0c6 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html +++ b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html @@ -14,7 +14,7 @@ {% translate "judgment.edit_this_judgment" %} - + {% translate "judgment.view_this_judgment" %} {% else %} diff --git a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html index b179f279d..c2c897941 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html +++ b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html @@ -4,7 +4,7 @@
  • - + {% if not item.is_failure %} {{ item.name | default:"[Untitled Judgment]"}} {% else %} diff --git a/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html b/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html index 280e04dc2..6a7c95725 100644 --- a/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html +++ b/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html @@ -16,6 +16,6 @@ - {% translate "judgment.back_to_judgment" %} + {% translate "judgment.back_to_judgment" %}
    {% endblock content %} diff --git a/ds_caselaw_editor_ui/templates/judgment/edit.html b/ds_caselaw_editor_ui/templates/judgment/edit.html index 020030add..3c19a5b47 100644 --- a/ds_caselaw_editor_ui/templates/judgment/edit.html +++ b/ds_caselaw_editor_ui/templates/judgment/edit.html @@ -73,7 +73,7 @@

    {% translate "judgment.previous_vers
      {% for version in context.judgment.versions %}
    • - + v{{ version.version }} ( {{version.uri}} )
      • diff --git a/judgments/urls.py b/judgments/urls.py index 6b53918f2..4850148f2 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -6,9 +6,9 @@ prioritise_judgment_button, ) from .views.delete import delete -from .views.detail import detail from .views.detail_xml import detail_xml from .views.edit_judgment import EditJudgmentView +from .views.full_text import html_view from .views.index import index from .views.results import results from .views.signed_asset import redirect_to_signed_asset @@ -16,7 +16,7 @@ urlpatterns = [ path("edit", EditJudgmentView.as_view(), name="edit"), - path("detail", detail, name="detail"), + path("detail", html_view, name="full-text-html"), path("xml", detail_xml, name="detail_xml"), path("results", results, name="results"), # buttons that do a thing and redirect diff --git a/judgments/views/edit_judgment.py b/judgments/views/edit_judgment.py index 8454fe5cf..997e89a99 100644 --- a/judgments/views/edit_judgment.py +++ b/judgments/views/edit_judgment.py @@ -73,9 +73,9 @@ def build_jira_create_link(self, request, context): tdr=context["judgment"].consignment_reference, ) - editor_details_url = request.build_absolute_uri( + editor_html_url = request.build_absolute_uri( "{base_url}?{params}".format( - base_url=reverse("detail"), + base_url=reverse("full-text-html"), params=urlencode( { "judgment_uri": context["judgment_uri"], @@ -84,13 +84,13 @@ def build_jira_create_link(self, request, context): ) ) - description_string = "{editor_details_url}".format( - editor_details_url="""{details_url} + description_string = "{editor_html_url}".format( + editor_html_url="""{html_url} {source_name_label}: {source_name} {source_email_label}: {source_email} {consignment_ref_label}: {consignment_ref}""".format( - details_url=editor_details_url, + html_url=editor_html_url, source_name_label=gettext("judgments.submitter"), source_name=context["judgment"].source_name, source_email_label=gettext("judgments.submitteremail"), diff --git a/judgments/views/detail.py b/judgments/views/full_text.py similarity index 98% rename from judgments/views/detail.py rename to judgments/views/full_text.py index db7dca5c3..dcb895c34 100644 --- a/judgments/views/detail.py +++ b/judgments/views/full_text.py @@ -6,7 +6,7 @@ from judgments.utils import extract_version -def detail(request): +def html_view(request): params = request.GET judgment_uri = params.get("judgment_uri", None) version_uri = params.get("version_uri", None) From 2d57f7960cc078fb301e6f0c3d71298f0bf42743 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Feb 2023 11:41:00 +0000 Subject: [PATCH 2/7] Update judgment HTML view URL to new structure This moves the main "show me the content as HTML" view from `/detail` followed by pulling the judgment URI from the query string, into its own `/` URL handler. We use `path` because judgment URLs contain slashes. As part of this we update the behaviour for 404s; the `Judgment` class will now try to preload an NCN when it is initialised, which means that the process of simply initialising a `Judgment` will cause a `MarklogicResourceNotFoundError` to be thrown if the judgment doesn't exist. This means we can wrap initialisation in a `try`/`catch` block to raise 404s before we even try to apply page logic. This in turn means the "judgment not found" test can actually test this behaviour, unlike testing Django's own page not found behaviour as it was before (since `/judgment` didn't exist). --- .../templates/includes/judgment_text_toolbar.html | 2 +- .../templates/includes/judgments_list_item.html | 2 +- .../templates/judgment/confirm-unlock.html | 2 +- ds_caselaw_editor_ui/templates/judgment/edit.html | 2 +- judgments/models.py | 3 +++ judgments/tests/tests.py | 12 +++++++++--- judgments/urls.py | 2 +- judgments/views/edit_judgment.py | 9 +-------- judgments/views/full_text.py | 8 ++++---- 9 files changed, 22 insertions(+), 20 deletions(-) diff --git a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html index c2c50c0c6..1074108b6 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html +++ b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html @@ -14,7 +14,7 @@ {% translate "judgment.edit_this_judgment" %} - + {% translate "judgment.view_this_judgment" %} {% else %} diff --git a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html index c2c897941..a7f01a063 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html +++ b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html @@ -4,7 +4,7 @@
    • - + {% if not item.is_failure %} {{ item.name | default:"[Untitled Judgment]"}} {% else %} diff --git a/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html b/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html index 6a7c95725..88265bbc5 100644 --- a/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html +++ b/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html @@ -16,6 +16,6 @@ - {% translate "judgment.back_to_judgment" %} + {% translate "judgment.back_to_judgment" %}
      {% endblock content %} diff --git a/ds_caselaw_editor_ui/templates/judgment/edit.html b/ds_caselaw_editor_ui/templates/judgment/edit.html index 3c19a5b47..c543db994 100644 --- a/ds_caselaw_editor_ui/templates/judgment/edit.html +++ b/ds_caselaw_editor_ui/templates/judgment/edit.html @@ -73,7 +73,7 @@

      {% translate "judgment.previous_vers
        {% for version in context.judgment.versions %}
      • - + v{{ version.version }} ( {{version.uri}} )
        • diff --git a/judgments/models.py b/judgments/models.py index fb4dabe7f..1bf49b23f 100644 --- a/judgments/models.py +++ b/judgments/models.py @@ -177,6 +177,9 @@ def __init__(self, uri: str, api_client: Optional[MarklogicApiClient] = None): use_https=settings.MARKLOGIC_USE_HTTPS, ) + # As part of initialisation, we preload the NCN so we can generate a MarklogicResourceNotFoundError early + self.neutral_citation + @property def public_uri(self) -> str: return "https://caselaw.nationalarchives.gov.uk/{uri}".format(uri=self.uri) diff --git a/judgments/tests/tests.py b/judgments/tests/tests.py index 6f175a578..655cad0a0 100644 --- a/judgments/tests/tests.py +++ b/judgments/tests/tests.py @@ -1,5 +1,6 @@ from unittest.mock import patch +from caselawclient.Client import MarklogicResourceNotFoundError from django.contrib.auth.models import User from django.test import TestCase from lxml import etree @@ -8,10 +9,15 @@ class TestJudgment(TestCase): - def test_404_response(self): - response = self.client.get("/judgments/ewca/civ/2004/63X") + @patch( + "judgments.models.MarklogicApiClient.get_judgment_citation", + side_effect=MarklogicResourceNotFoundError(), + ) + def test_judgment_not_found_response(self, mock_api_client): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/ewca/civ/2004/63X") decoded_response = response.content.decode("utf-8") - self.assertIn("Page not found", decoded_response) + self.assertIn("Judgment was not found", decoded_response) self.assertEqual(response.status_code, 404) diff --git a/judgments/urls.py b/judgments/urls.py index 4850148f2..161cbd805 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -16,7 +16,6 @@ urlpatterns = [ path("edit", EditJudgmentView.as_view(), name="edit"), - path("detail", html_view, name="full-text-html"), path("xml", detail_xml, name="detail_xml"), path("results", results, name="results"), # buttons that do a thing and redirect @@ -28,4 +27,5 @@ # redirect to signed asset URLs path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), path("", index, name="home"), + path("", html_view, name="full-text-html"), ] diff --git a/judgments/views/edit_judgment.py b/judgments/views/edit_judgment.py index 997e89a99..ee107d7e2 100644 --- a/judgments/views/edit_judgment.py +++ b/judgments/views/edit_judgment.py @@ -74,14 +74,7 @@ def build_jira_create_link(self, request, context): ) editor_html_url = request.build_absolute_uri( - "{base_url}?{params}".format( - base_url=reverse("full-text-html"), - params=urlencode( - { - "judgment_uri": context["judgment_uri"], - } - ), - ) + reverse("full-text-html", kwargs={"judgment_uri": context["judgment_uri"]}) ) description_string = "{editor_html_url}".format( diff --git a/judgments/views/full_text.py b/judgments/views/full_text.py index dcb895c34..b689037bd 100644 --- a/judgments/views/full_text.py +++ b/judgments/views/full_text.py @@ -6,14 +6,14 @@ from judgments.utils import extract_version -def html_view(request): +def html_view(request, judgment_uri): params = request.GET - judgment_uri = params.get("judgment_uri", None) version_uri = params.get("version_uri", None) - judgment = Judgment(judgment_uri) - context = {"judgment_uri": judgment_uri, "judgment": judgment} try: + judgment = Judgment(judgment_uri) + context = {"judgment_uri": judgment_uri, "judgment": judgment} + if not judgment.is_editable: judgment_content = judgment.content_as_xml() metadata_name = judgment_uri From 4d5ec6a193804c7c3804c3a57f7598262ba533db Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Feb 2023 11:55:57 +0000 Subject: [PATCH 3/7] Add redirects from old details view --- judgments/tests/tests.py | 26 ++++++++++++++++++++++++++ judgments/urls.py | 4 +++- judgments/views/full_text.py | 26 +++++++++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/judgments/tests/tests.py b/judgments/tests/tests.py index 655cad0a0..b6872110f 100644 --- a/judgments/tests/tests.py +++ b/judgments/tests/tests.py @@ -1,8 +1,10 @@ from unittest.mock import patch +from urllib.parse import urlencode from caselawclient.Client import MarklogicResourceNotFoundError from django.contrib.auth.models import User from django.test import TestCase +from django.urls import reverse from lxml import etree from judgments.models import Judgment, SearchResult, SearchResultMeta @@ -20,6 +22,30 @@ def test_judgment_not_found_response(self, mock_api_client): self.assertIn("Judgment was not found", decoded_response) self.assertEqual(response.status_code, 404) + def test_judgment_html_view_redirect(self): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/detail?judgment_uri=ewca/civ/2004/63X") + assert response.status_code == 302 + assert response["Location"] == reverse( + "full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"} + ) + + def test_judgment_html_view_redirect_with_version(self): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get( + "/detail?judgment_uri=ewca/civ/2004/63X&version_uri=ewca/civ/2004/63X_xml_versions/1-376" + ) + assert response.status_code == 302 + assert response["Location"] == ( + reverse("full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"}) + + "?" + + urlencode( + { + "version_uri": "ewca/civ/2004/63X_xml_versions/1-376", + } + ) + ) + class TestSearchResults(TestCase): @patch("judgments.utils.view_helpers.perform_advanced_search") diff --git a/judgments/urls.py b/judgments/urls.py index 161cbd805..a730305e9 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -8,7 +8,7 @@ from .views.delete import delete from .views.detail_xml import detail_xml from .views.edit_judgment import EditJudgmentView -from .views.full_text import html_view +from .views.full_text import html_view, html_view_redirect from .views.index import index from .views.results import results from .views.signed_asset import redirect_to_signed_asset @@ -27,5 +27,7 @@ # redirect to signed asset URLs path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), path("", index, name="home"), + # Functions for Judgments + path("detail", html_view_redirect), path("", html_view, name="full-text-html"), ] diff --git a/judgments/views/full_text.py b/judgments/views/full_text.py index b689037bd..dfdebd2b1 100644 --- a/judgments/views/full_text.py +++ b/judgments/views/full_text.py @@ -1,6 +1,9 @@ +from urllib.parse import urlencode + from caselawclient.Client import MarklogicResourceNotFoundError -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.template import loader +from django.urls import reverse from judgments.models import Judgment from judgments.utils import extract_version @@ -30,3 +33,24 @@ def html_view(request, judgment_uri): raise Http404(f"Judgment was not found at uri {judgment_uri}, {e}") template = loader.get_template("judgment/detail.html") return HttpResponse(template.render({"context": context}, request)) + + +def html_view_redirect(request): + params = request.GET + judgment_uri = params.get("judgment_uri", None) + version_uri = params.get("version_uri", None) + + redirect_path = reverse("full-text-html", kwargs={"judgment_uri": judgment_uri}) + + if version_uri: + redirect_path = ( + redirect_path + + "?" + + urlencode( + { + "version_uri": version_uri, + } + ) + ) + + return HttpResponseRedirect(redirect_path) From 3a8835de82ad0ff28fc4d6838829cd4bd2048a8a Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Feb 2023 13:57:44 +0000 Subject: [PATCH 4/7] Slice judgment view tests into their own file The number of tests we have for judgment view behaviours should be a lot higher, so pre-emptively put tests for these views in their own file. --- judgments/tests/test_judgments.py | 44 +++++++++++++++++++++++++++++++ judgments/tests/tests.py | 40 ---------------------------- 2 files changed, 44 insertions(+), 40 deletions(-) create mode 100644 judgments/tests/test_judgments.py diff --git a/judgments/tests/test_judgments.py b/judgments/tests/test_judgments.py new file mode 100644 index 000000000..9d5f4e7fc --- /dev/null +++ b/judgments/tests/test_judgments.py @@ -0,0 +1,44 @@ +from unittest.mock import patch +from urllib.parse import urlencode + +from caselawclient.Client import MarklogicResourceNotFoundError +from django.contrib.auth.models import User +from django.test import TestCase +from django.urls import reverse + + +class TestJudgment(TestCase): + @patch( + "judgments.models.MarklogicApiClient.get_judgment_citation", + side_effect=MarklogicResourceNotFoundError(), + ) + def test_judgment_not_found_response(self, mock_api_client): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/ewca/civ/2004/63X") + decoded_response = response.content.decode("utf-8") + self.assertIn("Judgment was not found", decoded_response) + self.assertEqual(response.status_code, 404) + + def test_judgment_html_view_redirect(self): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/detail?judgment_uri=ewca/civ/2004/63X") + assert response.status_code == 302 + assert response["Location"] == reverse( + "full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"} + ) + + def test_judgment_html_view_redirect_with_version(self): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get( + "/detail?judgment_uri=ewca/civ/2004/63X&version_uri=ewca/civ/2004/63X_xml_versions/1-376" + ) + assert response.status_code == 302 + assert response["Location"] == ( + reverse("full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"}) + + "?" + + urlencode( + { + "version_uri": "ewca/civ/2004/63X_xml_versions/1-376", + } + ) + ) diff --git a/judgments/tests/tests.py b/judgments/tests/tests.py index b6872110f..84c95f02c 100644 --- a/judgments/tests/tests.py +++ b/judgments/tests/tests.py @@ -1,52 +1,12 @@ from unittest.mock import patch -from urllib.parse import urlencode -from caselawclient.Client import MarklogicResourceNotFoundError from django.contrib.auth.models import User from django.test import TestCase -from django.urls import reverse from lxml import etree from judgments.models import Judgment, SearchResult, SearchResultMeta -class TestJudgment(TestCase): - @patch( - "judgments.models.MarklogicApiClient.get_judgment_citation", - side_effect=MarklogicResourceNotFoundError(), - ) - def test_judgment_not_found_response(self, mock_api_client): - self.client.force_login(User.objects.get_or_create(username="testuser")[0]) - response = self.client.get("/ewca/civ/2004/63X") - decoded_response = response.content.decode("utf-8") - self.assertIn("Judgment was not found", decoded_response) - self.assertEqual(response.status_code, 404) - - def test_judgment_html_view_redirect(self): - self.client.force_login(User.objects.get_or_create(username="testuser")[0]) - response = self.client.get("/detail?judgment_uri=ewca/civ/2004/63X") - assert response.status_code == 302 - assert response["Location"] == reverse( - "full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"} - ) - - def test_judgment_html_view_redirect_with_version(self): - self.client.force_login(User.objects.get_or_create(username="testuser")[0]) - response = self.client.get( - "/detail?judgment_uri=ewca/civ/2004/63X&version_uri=ewca/civ/2004/63X_xml_versions/1-376" - ) - assert response.status_code == 302 - assert response["Location"] == ( - reverse("full-text-html", kwargs={"judgment_uri": "ewca/civ/2004/63X"}) - + "?" - + urlencode( - { - "version_uri": "ewca/civ/2004/63X_xml_versions/1-376", - } - ) - ) - - class TestSearchResults(TestCase): @patch("judgments.utils.view_helpers.perform_advanced_search") def test_oldest(self, advanced_search): From b5397587233d09b6085bd11e7f27b584535be845 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Mon, 27 Feb 2023 14:04:33 +0000 Subject: [PATCH 5/7] Migrate judgment XML view to new URL format --- judgments/models.py | 2 +- judgments/tests/test_judgments.py | 21 ++++++++++++++++++++- judgments/tests/test_models_judgment.py | 2 +- judgments/urls.py | 10 ++++++---- judgments/views/detail_xml.py | 18 ------------------ judgments/views/full_text.py | 20 ++++++++++++++++++++ 6 files changed, 48 insertions(+), 25 deletions(-) delete mode 100644 judgments/views/detail_xml.py diff --git a/judgments/models.py b/judgments/models.py index 1bf49b23f..97d664938 100644 --- a/judgments/models.py +++ b/judgments/models.py @@ -244,7 +244,7 @@ def pdf_url(self) -> str: @property def xml_url(self) -> str: - return reverse("detail_xml") + "?judgment_uri=" + self.uri + return reverse("full-text-xml", kwargs={"judgment_uri": self.uri}) @cached_property def assigned_to(self) -> str: diff --git a/judgments/tests/test_judgments.py b/judgments/tests/test_judgments.py index 9d5f4e7fc..cd134e530 100644 --- a/judgments/tests/test_judgments.py +++ b/judgments/tests/test_judgments.py @@ -12,7 +12,7 @@ class TestJudgment(TestCase): "judgments.models.MarklogicApiClient.get_judgment_citation", side_effect=MarklogicResourceNotFoundError(), ) - def test_judgment_not_found_response(self, mock_api_client): + def test_judgment_html_view_not_found_response(self, mock_api_client): self.client.force_login(User.objects.get_or_create(username="testuser")[0]) response = self.client.get("/ewca/civ/2004/63X") decoded_response = response.content.decode("utf-8") @@ -42,3 +42,22 @@ def test_judgment_html_view_redirect_with_version(self): } ) ) + + @patch( + "judgments.models.MarklogicApiClient.get_judgment_citation", + side_effect=MarklogicResourceNotFoundError(), + ) + def test_judgment_xml_view_not_found_response(self, mock_api_client): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/ewca/civ/2004/63X/xml") + decoded_response = response.content.decode("utf-8") + self.assertIn("Judgment was not found", decoded_response) + self.assertEqual(response.status_code, 404) + + def test_judgment_xml_view_redirect(self): + self.client.force_login(User.objects.get_or_create(username="testuser")[0]) + response = self.client.get("/xml?judgment_uri=ewca/civ/2004/63X") + assert response.status_code == 302 + assert response["Location"] == reverse( + "full-text-xml", kwargs={"judgment_uri": "ewca/civ/2004/63X"} + ) diff --git a/judgments/tests/test_models_judgment.py b/judgments/tests/test_models_judgment.py index 837fb748b..a7025cee5 100644 --- a/judgments/tests/test_models_judgment.py +++ b/judgments/tests/test_models_judgment.py @@ -157,7 +157,7 @@ def test_judgment_pdf_url(self, mock_url_generator, mock_api_client): def test_judgment_xml_url(self, mock_api_client): judgment = Judgment("test/1234", mock_api_client) - assert judgment.xml_url == "/xml?judgment_uri=test/1234" + assert judgment.xml_url == "/test/1234/xml" def test_judgment_assigned_to(self, mock_api_client): mock_api_client.get_property.return_value = "testuser" diff --git a/judgments/urls.py b/judgments/urls.py index a730305e9..118cc6d71 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -6,9 +6,8 @@ prioritise_judgment_button, ) from .views.delete import delete -from .views.detail_xml import detail_xml from .views.edit_judgment import EditJudgmentView -from .views.full_text import html_view, html_view_redirect +from .views.full_text import html_view, html_view_redirect, xml_view, xml_view_redirect from .views.index import index from .views.results import results from .views.signed_asset import redirect_to_signed_asset @@ -16,7 +15,6 @@ urlpatterns = [ path("edit", EditJudgmentView.as_view(), name="edit"), - path("xml", detail_xml, name="detail_xml"), path("results", results, name="results"), # buttons that do a thing and redirect path("delete", delete, name="delete"), @@ -27,7 +25,11 @@ # redirect to signed asset URLs path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), path("", index, name="home"), - # Functions for Judgments + # Redirects for legacy judgment URIs path("detail", html_view_redirect), + path("xml", xml_view_redirect), + # Different views on judgments + path("/xml", xml_view, name="full-text-xml"), + # This 'bare judgment' URL must always go last path("", html_view, name="full-text-html"), ] diff --git a/judgments/views/detail_xml.py b/judgments/views/detail_xml.py deleted file mode 100644 index 81bfac3f1..000000000 --- a/judgments/views/detail_xml.py +++ /dev/null @@ -1,18 +0,0 @@ -from caselawclient.Client import MarklogicResourceNotFoundError -from django.http import Http404, HttpResponse - -from judgments.models import Judgment - - -def detail_xml(request): - params = request.GET - judgment_uri = params.get("judgment_uri", None) - try: - judgment = Judgment(judgment_uri) - judgment_xml = judgment.content_as_xml() - except MarklogicResourceNotFoundError as e: - raise Http404(f"Judgment was not found at uri {judgment_uri}, {e}") - - response = HttpResponse(judgment_xml, content_type="application/xml") - response["Content-Disposition"] = f"attachment; filename={judgment_uri}.xml" - return response diff --git a/judgments/views/full_text.py b/judgments/views/full_text.py index dfdebd2b1..d9020e0c2 100644 --- a/judgments/views/full_text.py +++ b/judgments/views/full_text.py @@ -35,6 +35,18 @@ def html_view(request, judgment_uri): return HttpResponse(template.render({"context": context}, request)) +def xml_view(request, judgment_uri): + try: + judgment = Judgment(judgment_uri) + judgment_xml = judgment.content_as_xml() + except MarklogicResourceNotFoundError as e: + raise Http404(f"Judgment was not found at uri {judgment_uri}, {e}") + + response = HttpResponse(judgment_xml, content_type="application/xml") + response["Content-Disposition"] = f"attachment; filename={judgment_uri}.xml" + return response + + def html_view_redirect(request): params = request.GET judgment_uri = params.get("judgment_uri", None) @@ -54,3 +66,11 @@ def html_view_redirect(request): ) return HttpResponseRedirect(redirect_path) + + +def xml_view_redirect(request): + params = request.GET + judgment_uri = params.get("judgment_uri", None) + return HttpResponseRedirect( + reverse("full-text-xml", kwargs={"judgment_uri": judgment_uri}) + ) From 5cf83d398f3e9cba5218b50e02880985bbdfb486 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 28 Feb 2023 11:49:16 +0000 Subject: [PATCH 6/7] Migrate edit judgment view to the new URL format --- ds_caselaw_editor_ui/static/js/src/app.js | 4 ++-- .../includes/judgment_text_toolbar.html | 2 +- .../includes/judgments_list_item.html | 4 ++-- judgments/tests/test_unlock.py | 14 +++++++++++--- judgments/tests/tests.py | 6 +++++- judgments/urls.py | 5 +++-- judgments/views/edit_judgment.py | 19 +++++++++++++++---- judgments/views/unlock.py | 9 ++++++--- 8 files changed, 45 insertions(+), 18 deletions(-) diff --git a/ds_caselaw_editor_ui/static/js/src/app.js b/ds_caselaw_editor_ui/static/js/src/app.js index 0747d77ab..76a90eddf 100644 --- a/ds_caselaw_editor_ui/static/js/src/app.js +++ b/ds_caselaw_editor_ui/static/js/src/app.js @@ -66,9 +66,9 @@ $(".judgments-list__judgment-assign-form").on("submit", function (event) { success: function (data) { const assigned_to = data["assigned_to"]; loading.replaceWith( - "" + + "/edit#assigned_to'>" + assigned_to + "" ); diff --git a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html index 1074108b6..8ff91dfa0 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html +++ b/ds_caselaw_editor_ui/templates/includes/judgment_text_toolbar.html @@ -11,7 +11,7 @@
        {% if context.judgment.is_editable %} - + {% translate "judgment.edit_this_judgment" %} diff --git a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html index a7f01a063..470bb37b9 100644 --- a/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html +++ b/ds_caselaw_editor_ui/templates/includes/judgments_list_item.html @@ -95,7 +95,7 @@ {% if item.meta.assigned_to %} - {{ item.meta.assigned_to }} + {{ item.meta.assigned_to }} {% else %}
        {% csrf_token %} @@ -114,7 +114,7 @@
        {% if item.meta.assigned_to %} - {{ item.meta.assigned_to }} + {{ item.meta.assigned_to }} {% else %} {% csrf_token %} diff --git a/judgments/tests/test_unlock.py b/judgments/tests/test_unlock.py index a7260f63a..b229c0066 100644 --- a/judgments/tests/test_unlock.py +++ b/judgments/tests/test_unlock.py @@ -3,6 +3,9 @@ import pytest from django.contrib.auth.models import User from django.test import Client +from django.urls import reverse + +from judgments.models import Judgment @pytest.mark.django_db @@ -20,14 +23,19 @@ def test_break_lock_confirm_page(): @pytest.mark.django_db +@patch( + "judgments.views.unlock.Judgment", + autospec=Judgment, +) @patch("judgments.views.unlock.api_client.break_checkout") @patch("judgments.views.unlock.messages") -def test_break_lock_post(messages, break_checkout): +def test_break_lock_post(messages, break_checkout, mock_judgment): + mock_judgment.return_value.uri = "ewca/civ/2023/1" client = Client() client.force_login(User.objects.get_or_create(username="testuser")[0]) response = client.post("/unlock", data={"judgment_uri": "/ewca/civ/2023/1"}) - break_checkout.assert_called_with("/ewca/civ/2023/1") + break_checkout.assert_called_with("ewca/civ/2023/1") messages.success.assert_called_with(ANY, "Judgment unlocked.") assert response.status_code == 302 - assert response.url == "/edit?judgment_uri=/ewca/civ/2023/1" # type: ignore + assert response.url == reverse("edit-judgment", kwargs={"judgment_uri": "ewca/civ/2023/1"}) # type: ignore diff --git a/judgments/tests/tests.py b/judgments/tests/tests.py index 84c95f02c..06d1c4f6b 100644 --- a/judgments/tests/tests.py +++ b/judgments/tests/tests.py @@ -2,6 +2,7 @@ from django.contrib.auth.models import User from django.test import TestCase +from django.urls import reverse from lxml import etree from judgments.models import Judgment, SearchResult, SearchResultMeta @@ -142,11 +143,14 @@ class TestJudgmentEditor(TestCase): autospec=Judgment, ) def test_assigned(self, mock_judgment): + mock_judgment.return_value.uri = "ewhc/ch/1999/1" mock_judgment.return_value.assigned_to = "otheruser" mock_judgment.return_value.versions = [] User.objects.get_or_create(username="otheruser")[0] self.client.force_login(User.objects.get_or_create(username="testuser")[0]) - response = self.client.get("/edit?judgment_uri=ewhc/ch/1999/1") + response = self.client.get( + reverse("edit-judgment", kwargs={"judgment_uri": "ewhc/ch/1999/1"}) + ) assert b"selected>otheruser" in response.content diff --git a/judgments/urls.py b/judgments/urls.py index 118cc6d71..dc6ee2b54 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -6,7 +6,7 @@ prioritise_judgment_button, ) from .views.delete import delete -from .views.edit_judgment import EditJudgmentView +from .views.edit_judgment import EditJudgmentView, edit_view_redirect from .views.full_text import html_view, html_view_redirect, xml_view, xml_view_redirect from .views.index import index from .views.results import results @@ -14,7 +14,6 @@ from .views.unlock import unlock urlpatterns = [ - path("edit", EditJudgmentView.as_view(), name="edit"), path("results", results, name="results"), # buttons that do a thing and redirect path("delete", delete, name="delete"), @@ -26,9 +25,11 @@ path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), path("", index, name="home"), # Redirects for legacy judgment URIs + path("edit", edit_view_redirect), path("detail", html_view_redirect), path("xml", xml_view_redirect), # Different views on judgments + path("/edit", EditJudgmentView.as_view(), name="edit-judgment"), path("/xml", xml_view, name="full-text-xml"), # This 'bare judgment' URL must always go last path("", html_view, name="full-text-html"), diff --git a/judgments/views/edit_judgment.py b/judgments/views/edit_judgment.py index ee107d7e2..a7b2d3127 100644 --- a/judgments/views/edit_judgment.py +++ b/judgments/views/edit_judgment.py @@ -109,8 +109,7 @@ def render(self, request, context): return HttpResponse(template.render({"context": context}, request)) def get(self, request, *args, **kwargs): - params = request.GET - judgment_uri = params.get("judgment_uri") + judgment_uri = kwargs["judgment_uri"] judgment = Judgment(judgment_uri) context = {"judgment_uri": judgment_uri} @@ -170,7 +169,9 @@ def post(self, request, *args, **kwargs): # If judgment_uri is a `failure` URI, amend it to match new neutral citation and redirect if "failures" in judgment_uri and new_citation is not None: new_judgment_uri = update_judgment_uri(judgment_uri, new_citation) - return redirect(reverse("edit") + f"?judgment_uri={new_judgment_uri}") + return redirect( + reverse("edit-judgment", kwargs={"judgment_uri": new_judgment_uri}) + ) if published: notify_status = "published" @@ -195,4 +196,14 @@ def post(self, request, *args, **kwargs): invalidate_caches(judgment_uri) - return HttpResponseRedirect(reverse("edit") + "?judgment_uri=" + judgment_uri) + return HttpResponseRedirect( + reverse("edit-judgment", kwargs={"judgment_uri": judgment_uri}) + ) + + +def edit_view_redirect(request): + params = request.GET + judgment_uri = params.get("judgment_uri", None) + return HttpResponseRedirect( + reverse("edit-judgment", kwargs={"judgment_uri": judgment_uri}) + ) diff --git a/judgments/views/unlock.py b/judgments/views/unlock.py index 2b3fa84e7..bad8faa26 100644 --- a/judgments/views/unlock.py +++ b/judgments/views/unlock.py @@ -11,6 +11,8 @@ from django.utils.translation import gettext from django.views.decorators.http import require_http_methods +from judgments.models import Judgment + @require_http_methods(["POST", "GET", "HEAD"]) def unlock(request): @@ -34,10 +36,11 @@ def unlock_get(request): def unlock_post(request): """Unlock the judgment in Marklogic and return to edit judgment""" - judgment_uri = request.POST.get("judgment_uri") + judgment_uri = request.POST.get("judgment_uri") try: - api_client.break_checkout(judgment_uri) + judgment = Judgment(judgment_uri) + api_client.break_checkout(judgment.uri) except MarklogicResourceUnmanagedError as exc: raise Http404( f"Resource Unmanaged: Judgment '{judgment_uri}' might not exist." @@ -48,4 +51,4 @@ def unlock_post(request): ) from exc else: messages.success(request, "Judgment unlocked.") - return redirect(reverse("edit") + f"?judgment_uri={judgment_uri}") + return redirect(reverse("edit-judgment", kwargs={"judgment_uri": judgment.uri})) From 8c58ff1aa688f26999658220ace6be83954187db Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 28 Feb 2023 15:10:14 +0000 Subject: [PATCH 7/7] Rearrange URLs list and add some more comments --- judgments/urls.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/judgments/urls.py b/judgments/urls.py index dc6ee2b54..541c35da5 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -14,16 +14,18 @@ from .views.unlock import unlock urlpatterns = [ + # Home + path("", index, name="home"), + # Search path("results", results, name="results"), - # buttons that do a thing and redirect + # redirect to signed asset URLs + path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), + # Legacy judgment verbs path("delete", delete, name="delete"), path("unlock", unlock, name="unlock"), path("assign", assign_judgment_button, name="assign"), path("prioritise", prioritise_judgment_button, name="prioritise"), path("hold", hold_judgment_button, name="hold"), - # redirect to signed asset URLs - path("signed-asset/", redirect_to_signed_asset, name="signed-asset"), - path("", index, name="home"), # Redirects for legacy judgment URIs path("edit", edit_view_redirect), path("detail", html_view_redirect),