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 2a28ef35e..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,10 +11,10 @@
{% if context.judgment.is_editable %} - + {% 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..470bb37b9 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 %} @@ -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/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html b/ds_caselaw_editor_ui/templates/judgment/confirm-unlock.html index 280e04dc2..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 020030add..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..97d664938 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) @@ -241,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 new file mode 100644 index 000000000..cd134e530 --- /dev/null +++ b/judgments/tests/test_judgments.py @@ -0,0 +1,63 @@ +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_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") + 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", + } + ) + ) + + @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/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 6f175a578..06d1c4f6b 100644 --- a/judgments/tests/tests.py +++ b/judgments/tests/tests.py @@ -2,19 +2,12 @@ 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): - def test_404_response(self): - response = self.client.get("/judgments/ewca/civ/2004/63X") - decoded_response = response.content.decode("utf-8") - self.assertIn("Page not found", decoded_response) - self.assertEqual(response.status_code, 404) - - class TestSearchResults(TestCase): @patch("judgments.utils.view_helpers.perform_advanced_search") def test_oldest(self, advanced_search): @@ -150,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 6b53918f2..541c35da5 100644 --- a/judgments/urls.py +++ b/judgments/urls.py @@ -6,26 +6,33 @@ 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.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 from .views.signed_asset import redirect_to_signed_asset from .views.unlock import unlock urlpatterns = [ - path("edit", EditJudgmentView.as_view(), name="edit"), - path("detail", detail, name="detail"), - path("xml", detail_xml, name="detail_xml"), + # 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), + 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/detail.py b/judgments/views/detail.py deleted file mode 100644 index db7dca5c3..000000000 --- a/judgments/views/detail.py +++ /dev/null @@ -1,32 +0,0 @@ -from caselawclient.Client import MarklogicResourceNotFoundError -from django.http import Http404, HttpResponse -from django.template import loader - -from judgments.models import Judgment -from judgments.utils import extract_version - - -def detail(request): - 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: - if not judgment.is_editable: - judgment_content = judgment.content_as_xml() - metadata_name = judgment_uri - else: - judgment_content = judgment.content_as_html(version_uri=version_uri) - metadata_name = judgment.name - - context["judgment_content"] = judgment_content - context["page_title"] = metadata_name - - if version_uri: - context["version"] = extract_version(version_uri) - except MarklogicResourceNotFoundError as e: - 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)) 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/edit_judgment.py b/judgments/views/edit_judgment.py index 8454fe5cf..a7b2d3127 100644 --- a/judgments/views/edit_judgment.py +++ b/judgments/views/edit_judgment.py @@ -73,24 +73,17 @@ def build_jira_create_link(self, request, context): tdr=context["judgment"].consignment_reference, ) - editor_details_url = request.build_absolute_uri( - "{base_url}?{params}".format( - base_url=reverse("detail"), - params=urlencode( - { - "judgment_uri": context["judgment_uri"], - } - ), - ) + editor_html_url = request.build_absolute_uri( + reverse("full-text-html", kwargs={"judgment_uri": context["judgment_uri"]}) ) - 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"), @@ -116,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} @@ -177,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" @@ -202,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/full_text.py b/judgments/views/full_text.py new file mode 100644 index 000000000..d9020e0c2 --- /dev/null +++ b/judgments/views/full_text.py @@ -0,0 +1,76 @@ +from urllib.parse import urlencode + +from caselawclient.Client import MarklogicResourceNotFoundError +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 + + +def html_view(request, judgment_uri): + params = request.GET + version_uri = params.get("version_uri", None) + + 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 + else: + judgment_content = judgment.content_as_html(version_uri=version_uri) + metadata_name = judgment.name + + context["judgment_content"] = judgment_content + context["page_title"] = metadata_name + + if version_uri: + context["version"] = extract_version(version_uri) + except MarklogicResourceNotFoundError as e: + 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 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) + 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) + + +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}) + ) 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}))