Merge pull request #697 from nationalarchives/FCL-340-add-sim-rules-t…

…o-ruff-in-api-client-and-make-necessary-adjustments

[FCL-340] Add code simplification rules to ruff in API Client and make necessary adjustments

pyproject.toml

@@ -64,9 +64,9 @@ line-length = 120
 
 [tool.ruff.lint]
 ignore = ["E501", "G004", "PLR2004", "RUF005", "RUF012", "UP040"] # longlines, fstrings in logs, magic values, consider not concat, mutable classbits, type instead of TypeAlias
-extend-select = ["W", "I", "SLF"]
+extend-select = ["W", "I", "SLF", "SIM"]
 # extend-select = [ "B", "Q", "C90", "I", "UP", "YTT", "ASYNC", "S", "BLE", "A", "COM", "C4", "DTZ", "T10", "DJ", "EM", "EXE", "FA",
-#                  "ISC", "ICN", "G", "INP", "PIE", "T20", "PYI", "PT", "Q", "RSE", "RET", "SLOT", "SIM", "TID", "TCH", "INT", "PTH",
+#                  "ISC", "ICN", "G", "INP", "PIE", "T20", "PYI", "PT", "Q", "RSE", "RET", "SLOT", "TID", "TCH", "INT", "PTH",
 #                  "FIX", "PGH", "PL", "TRY", "FLY", "PERF", "RUF"]
 unfixable = ["ERA"]
 

src/caselawclient/Client.py

@@ -800,10 +800,7 @@ def eval_xslt(
             else None
         )
 
-        if os.getenv("XSLT_IMAGE_LOCATION"):
-            image_location = os.getenv("XSLT_IMAGE_LOCATION")
-        else:
-            image_location = ""
+        image_location = os.getenv("XSLT_IMAGE_LOCATION", "")
 
         show_unpublished = self.verify_show_unpublished(show_unpublished)
 

src/caselawclient/models/documents/__init__.py

@@ -281,22 +281,15 @@ def is_failure(self) -> bool:
 
         :return: `True` if this document is in a 'failure' state, otherwise `False`
         """
-        if self.body.failed_to_parse:
-            return True
-        return False
+        return self.body.failed_to_parse
 
     @cached_property
     def is_parked(self) -> bool:
-        if "parked" in self.uri:
-            return True
-        return False
+        return "parked" in self.uri
 
     @cached_property
     def has_name(self) -> bool:
-        if not self.body.name:
-            return False
-
-        return True
+        return bool(self.body.name)
 
     @cached_property
     def has_valid_court(self) -> bool:
@@ -373,9 +366,7 @@ def can_enrich(self) -> bool:
         """
         Is it sensible to enrich this document?
         """
-        if (self.enriched_recently is False) and self.validates_against_schema:
-            return True
-        return False
+        return (self.enriched_recently is False) and self.validates_against_schema
 
     @cached_property
     def enriched_recently(self) -> bool:
@@ -388,9 +379,8 @@ def enriched_recently(self) -> bool:
             return False
 
         now = datetime.datetime.now(tz=datetime.timezone.utc)
-        if now - last_enrichment < MINIMUM_ENRICHMENT_TIME:
-            return True
-        return False
+
+        return now - last_enrichment < MINIMUM_ENRICHMENT_TIME
 
     @cached_property
     def validates_against_schema(self) -> bool:
@@ -506,6 +496,4 @@ def can_reparse(self) -> bool:
         """
         Is it sensible to reparse this document?
         """
-        if self.docx_exists():
-            return True
-        return False
+        return self.docx_exists()

src/caselawclient/models/documents/body.py

@@ -137,6 +137,4 @@ def failed_to_parse(self) -> bool:
 
         :return: `True` if there was a complete parser failure, otherwise `False`
         """
-        if "error" in self._xml.root_element:
-            return True
-        return False
+        return "error" in self._xml.root_element

src/caselawclient/models/neutral_citation_mixin.py

@@ -40,14 +40,8 @@ def neutral_citation(self) -> str:
 
     @cached_property
     def has_ncn(self) -> bool:
-        if not self.neutral_citation:
-            return False
-
-        return True
+        return bool(self.neutral_citation)
 
     @cached_property
     def has_valid_ncn(self) -> bool:
-        if not self.has_ncn or not neutral_url(self.neutral_citation):
-            return False
-
-        return True
+        return self.has_ncn and neutral_url(self.neutral_citation) is not None

src/caselawclient/models/utilities/dates.py

@@ -9,11 +9,7 @@ def parse_string_date_as_utc(iso_string: str, timezone: tzinfo.BaseTzInfo) -> da
     ensure that it is converted to a UTC-aware datetime"""
 
     mixed_date = isoparse(iso_string)
-    if not mixed_date.tzinfo:
-        # it is an unaware time
-        aware_date = timezone.localize(mixed_date)
-    else:
-        aware_date = mixed_date
+    aware_date = mixed_date if mixed_date.tzinfo else timezone.localize(mixed_date)
 
     # make UTC
     utc_date = aware_date.astimezone(UTC)

tests/client/test_advanced_search.py

@@ -128,46 +128,45 @@ def test_user_can_view_unpublished_but_show_unpublished_is_false(
         When the advanced_search method is called with the show_unpublished parameter set to False
         Then it should call the MarkLogic module with the expected query parameters
         """
-        with patch.object(self.client, "invoke") as mock_invoke:
-            with patch.object(
-                self.client,
-                "user_can_view_unpublished_judgments",
-                return_value=True,
-            ):
-                self.client.advanced_search(
-                    SearchParameters(
-                        query="my-query",
-                        court="ewhc",
-                        judge="a. judge",
-                        party="a party",
-                        page=2,
-                        page_size=20,
-                        show_unpublished=False,
-                    ),
-                )
-
-                expected_vars = {
-                    "court": ["ewhc"],
-                    "judge": "a. judge",
-                    "page": 2,
-                    "page-size": 20,
-                    "q": "my-query",
-                    "party": "a party",
-                    "neutral_citation": "",
-                    "specific_keyword": "",
-                    "order": "",
-                    "from": "",
-                    "to": "",
-                    "show_unpublished": "false",
-                    "only_unpublished": "false",
-                    "collections": "",
-                    "quoted_phrases": [],
-                }
-
-                mock_invoke.assert_called_with(
-                    "/judgments/search/search-v2.xqy",
-                    json.dumps(expected_vars),
-                )
+        with patch.object(self.client, "invoke") as mock_invoke, patch.object(
+            self.client,
+            "user_can_view_unpublished_judgments",
+            return_value=True,
+        ):
+            self.client.advanced_search(
+                SearchParameters(
+                    query="my-query",
+                    court="ewhc",
+                    judge="a. judge",
+                    party="a party",
+                    page=2,
+                    page_size=20,
+                    show_unpublished=False,
+                ),
+            )
+
+            expected_vars = {
+                "court": ["ewhc"],
+                "judge": "a. judge",
+                "page": 2,
+                "page-size": 20,
+                "q": "my-query",
+                "party": "a party",
+                "neutral_citation": "",
+                "specific_keyword": "",
+                "order": "",
+                "from": "",
+                "to": "",
+                "show_unpublished": "false",
+                "only_unpublished": "false",
+                "collections": "",
+                "quoted_phrases": [],
+            }
+
+            mock_invoke.assert_called_with(
+                "/judgments/search/search-v2.xqy",
+                json.dumps(expected_vars),
+            )
 
     def test_user_can_view_unpublished_and_show_unpublished_is_true(
         self,
@@ -178,24 +177,23 @@ def test_user_can_view_unpublished_and_show_unpublished_is_true(
         When the advanced_search method is called with the show_unpublished parameter set to True
         Then it should call the MarkLogic module with the expected query parameters
         """
-        with patch.object(self.client, "invoke"):
-            with patch.object(
-                self.client,
-                "user_can_view_unpublished_judgments",
-                return_value=True,
-            ):
-                self.client.advanced_search(
-                    SearchParameters(
-                        query="my-query",
-                        court="ewhc",
-                        judge="a. judge",
-                        party="a party",
-                        page=2,
-                        page_size=20,
-                        show_unpublished=True,
-                    ),
-                )
-                assert '"show_unpublished": "true"' in self.client.invoke.call_args.args[1]
+        with patch.object(self.client, "invoke"), patch.object(
+            self.client,
+            "user_can_view_unpublished_judgments",
+            return_value=True,
+        ):
+            self.client.advanced_search(
+                SearchParameters(
+                    query="my-query",
+                    court="ewhc",
+                    judge="a. judge",
+                    party="a party",
+                    page=2,
+                    page_size=20,
+                    show_unpublished=True,
+                ),
+            )
+            assert '"show_unpublished": "true"' in self.client.invoke.call_args.args[1]
 
     def test_user_cannot_view_unpublished_but_show_unpublished_is_true(
         self,
@@ -206,27 +204,25 @@ def test_user_cannot_view_unpublished_but_show_unpublished_is_true(
         When the advanced_search method is called with the show_unpublished parameter set to True
         Then it should call the MarkLogic module with the show_unpublished parameter set to False and log a warning
         """
-        with patch.object(self.client, "invoke"):
-            with patch.object(
-                self.client,
-                "user_can_view_unpublished_judgments",
-                return_value=False,
-            ):
-                with patch.object(logging, "warning") as mock_logging:
-                    self.client.advanced_search(
-                        SearchParameters(
-                            query="my-query",
-                            court="ewhc",
-                            judge="a. judge",
-                            party="a party",
-                            page=2,
-                            page_size=20,
-                            show_unpublished=True,
-                        ),
-                    )
-
-                    assert '"show_unpublished": "false"' in self.client.invoke.call_args.args[1]
-                    mock_logging.assert_called()
+        with patch.object(self.client, "invoke"), patch.object(
+            self.client,
+            "user_can_view_unpublished_judgments",
+            return_value=False,
+        ), patch.object(logging, "warning") as mock_logging:
+            self.client.advanced_search(
+                SearchParameters(
+                    query="my-query",
+                    court="ewhc",
+                    judge="a. judge",
+                    party="a party",
+                    page=2,
+                    page_size=20,
+                    show_unpublished=True,
+                ),
+            )
+
+            assert '"show_unpublished": "false"' in self.client.invoke.call_args.args[1]
+            mock_logging.assert_called()
 
     def test_no_page_0(self):
         """

tests/client/test_checkout_checkin_judgment.py

@@ -29,24 +29,23 @@ def test_checkout_judgment(self):
             )
 
     def test_checkout_judgment_with_timeout(self):
-        with patch.object(self.client, "eval") as mock_eval:
-            with patch.object(
-                self.client,
-                "calculate_seconds_until_midnight",
-                return_value=3600,
-            ):
-                uri = "/ewca/civ/2004/632"
-                annotation = "locked by A KITTEN"
-                expires_at_midnight = True
-                expected_vars = {
-                    "uri": "/ewca/civ/2004/632.xml",
-                    "annotation": "locked by A KITTEN",
-                    "timeout": 3600,
-                }
-                self.client.checkout_judgment(uri, annotation, expires_at_midnight)
-
-                assert mock_eval.call_args.args[0] == (os.path.join(ROOT_DIR, "xquery", "checkout_judgment.xqy"))
-                assert mock_eval.call_args.kwargs["vars"] == json.dumps(expected_vars)
+        with patch.object(self.client, "eval") as mock_eval, patch.object(
+            self.client,
+            "calculate_seconds_until_midnight",
+            return_value=3600,
+        ):
+            uri = "/ewca/civ/2004/632"
+            annotation = "locked by A KITTEN"
+            expires_at_midnight = True
+            expected_vars = {
+                "uri": "/ewca/civ/2004/632.xml",
+                "annotation": "locked by A KITTEN",
+                "timeout": 3600,
+            }
+            self.client.checkout_judgment(uri, annotation, expires_at_midnight)
+
+            assert mock_eval.call_args.args[0] == (os.path.join(ROOT_DIR, "xquery", "checkout_judgment.xqy"))
+            assert mock_eval.call_args.kwargs["vars"] == json.dumps(expected_vars)
 
     def test_checkin_judgment(self):
         with patch.object(self.client, "eval") as mock_eval: