diff --git a/src/caselawclient/models/documents/__init__.py b/src/caselawclient/models/documents/__init__.py index 6af19c3e..a85f1763 100644 --- a/src/caselawclient/models/documents/__init__.py +++ b/src/caselawclient/models/documents/__init__.py @@ -15,6 +15,7 @@ NotSupportedOnVersion, OnlySupportedOnVersion, ) +from caselawclient.models.identifiers import Identifier from caselawclient.models.identifiers.fclid import FindCaseLawIdentifier, FindCaseLawIdentifierSchema from caselawclient.models.identifiers.unpacker import unpack_all_identifiers_from_etree from caselawclient.models.utilities import VersionsDict, extract_version, render_versions @@ -171,13 +172,11 @@ def _initialise_identifiers(self) -> None: self.identifiers = unpack_all_identifiers_from_etree(identifiers_element_as_etree) @property - def best_human_identifier(self) -> Optional[str]: - """ - Some identifier that is understood by legal professionals to refer to this legal event - that is not the name of the document. - Typically, this will be the neutral citation number, should it exist. - Should typically be overridden in subclasses. - """ + def best_human_identifier(self) -> Optional[Identifier]: + """Return the preferred identifier for the document, providing that it is considered human readable.""" + preferred_identifier = self.identifiers.preferred + if preferred_identifier and preferred_identifier.schema.human_readable: + return preferred_identifier return None @property @@ -507,13 +506,17 @@ def force_reparse(self) -> None: "documentType": parser_type_noun, "metadata": { "name": self.body.name or None, - "cite": self.best_human_identifier or None, + "cite": None, "court": self.body.court or None, "date": checked_date, "uri": self.uri, }, } + ## TODO: Remove this hack around the fact that NCNs are assumed to be present for all documents' metadata, but actually different document classes may have different metadata + if hasattr(self, "neutral_citation"): + parser_instructions["metadata"]["cite"] = self.neutral_citation + request_parse( uri=self.uri, reference=self.consignment_reference, diff --git a/src/caselawclient/models/judgments.py b/src/caselawclient/models/judgments.py index 730c3c0c..a2d0fffe 100644 --- a/src/caselawclient/models/judgments.py +++ b/src/caselawclient/models/judgments.py @@ -25,20 +25,17 @@ def __init__(self, uri: DocumentURIString, *args: Any, **kwargs: Any) -> None: super().__init__(self.document_noun, uri, *args, **kwargs) @cached_property - def neutral_citation(self) -> NeutralCitationString: - return NeutralCitationString( - self.body.get_xpath_match_string( - "/akn:akomaNtoso/akn:*/akn:meta/akn:proprietary/uk:cite/text()", - { - "uk": "https://caselaw.nationalarchives.gov.uk/akn", - "akn": "http://docs.oasis-open.org/legaldocml/ns/akn/3.0", - }, - ) + def neutral_citation(self) -> Optional[NeutralCitationString]: + value_in_xml = self.body.get_xpath_match_string( + "/akn:akomaNtoso/akn:*/akn:meta/akn:proprietary/uk:cite/text()", + { + "uk": "https://caselaw.nationalarchives.gov.uk/akn", + "akn": "http://docs.oasis-open.org/legaldocml/ns/akn/3.0", + }, ) - - @property - def best_human_identifier(self) -> str: - return self.neutral_citation + if value_in_xml: + return NeutralCitationString(value_in_xml) + return None @cached_property def linked_document(self) -> Optional["PressSummary"]: diff --git a/src/caselawclient/models/neutral_citation_mixin.py b/src/caselawclient/models/neutral_citation_mixin.py index 6b55a125..7f5c38df 100644 --- a/src/caselawclient/models/neutral_citation_mixin.py +++ b/src/caselawclient/models/neutral_citation_mixin.py @@ -1,9 +1,10 @@ from abc import ABC, abstractmethod from functools import cached_property -from typing import Any +from typing import Any, Optional from ds_caselaw_utils import neutral_url from ds_caselaw_utils.types import NeutralCitationString +from typing_extensions import deprecated class NeutralCitationMixin(ABC): @@ -38,12 +39,15 @@ def __init__(self, document_noun: str, *args: Any, **kwargs: Any) -> None: @cached_property @abstractmethod - def neutral_citation(self) -> NeutralCitationString: ... + @deprecated("Legacy usage of NCNs is deprecated; you should be moving to the Identifiers framework") + def neutral_citation(self) -> Optional[NeutralCitationString]: ... @cached_property + @deprecated("Legacy usage of NCNs is deprecated; you should be moving to the Identifiers framework") def has_ncn(self) -> bool: - return bool(self.neutral_citation) + return self.neutral_citation is not None and self.neutral_citation != "" @cached_property + @deprecated("Legacy usage of NCNs is deprecated; you should be moving to the Identifiers framework") def has_valid_ncn(self) -> bool: - return self.has_ncn and neutral_url(self.neutral_citation) is not None + return self.neutral_citation is not None and neutral_url(self.neutral_citation) is not None diff --git a/src/caselawclient/models/press_summaries.py b/src/caselawclient/models/press_summaries.py index 2996c132..587ba051 100644 --- a/src/caselawclient/models/press_summaries.py +++ b/src/caselawclient/models/press_summaries.py @@ -27,19 +27,16 @@ def __init__(self, uri: DocumentURIString, *args: Any, **kwargs: Any) -> None: super().__init__(self.document_noun, uri, *args, **kwargs) @cached_property - def neutral_citation(self) -> NeutralCitationString: - return NeutralCitationString( - self.body.get_xpath_match_string( - "/akn:akomaNtoso/akn:doc/akn:preface/akn:p/akn:neutralCitation/text()", - { - "akn": "http://docs.oasis-open.org/legaldocml/ns/akn/3.0", - }, - ) + def neutral_citation(self) -> Optional[NeutralCitationString]: + value_in_xml = self.body.get_xpath_match_string( + "/akn:akomaNtoso/akn:doc/akn:preface/akn:p/akn:neutralCitation/text()", + { + "akn": "http://docs.oasis-open.org/legaldocml/ns/akn/3.0", + }, ) - - @property - def best_human_identifier(self) -> str: - return self.neutral_citation + if value_in_xml: + return NeutralCitationString(value_in_xml) + return None @cached_property def linked_document(self) -> Optional[Judgment]: diff --git a/tests/models/test_judgments.py b/tests/models/test_judgments.py index 814ea087..af43bb2f 100644 --- a/tests/models/test_judgments.py +++ b/tests/models/test_judgments.py @@ -5,6 +5,7 @@ from caselawclient.errors import DocumentNotFoundError from caselawclient.factories import PressSummaryFactory from caselawclient.models.documents import DocumentURIString +from caselawclient.models.identifiers.neutral_citation import NeutralCitationNumber from caselawclient.models.judgments import Judgment from caselawclient.models.neutral_citation_mixin import NeutralCitationString @@ -12,8 +13,9 @@ class TestJudgment: def test_best_identifier(self, mock_api_client): judgment = Judgment(DocumentURIString("test/1234"), mock_api_client) - judgment.neutral_citation = NeutralCitationString("[2023] TEST 1234") - assert judgment.best_human_identifier == judgment.neutral_citation + document_ncn = NeutralCitationNumber(value="[2023] TEST 1234") + judgment.identifiers.add(document_ncn) + assert judgment.best_human_identifier == document_ncn class TestJudgmentValidation: diff --git a/tests/models/test_press_summaries.py b/tests/models/test_press_summaries.py index cd4e461b..d5bf3251 100644 --- a/tests/models/test_press_summaries.py +++ b/tests/models/test_press_summaries.py @@ -5,6 +5,7 @@ from caselawclient.errors import DocumentNotFoundError from caselawclient.factories import JudgmentFactory from caselawclient.models.documents import DocumentURIString +from caselawclient.models.identifiers.neutral_citation import NeutralCitationNumber from caselawclient.models.neutral_citation_mixin import NeutralCitationString from caselawclient.models.press_summaries import PressSummary @@ -12,8 +13,9 @@ class TestPressSummary: def test_best_identifier(self, mock_api_client): summary = PressSummary(DocumentURIString("test/1234"), mock_api_client) - summary.neutral_citation = NeutralCitationString("[2023] TEST 1234") - assert summary.best_human_identifier == summary.neutral_citation + document_ncn = NeutralCitationNumber(value="[2023] TEST 1234") + summary.identifiers.add(document_ncn) + assert summary.best_human_identifier == document_ncn class TestPressSummaryValidation: