Merge pull request #657 from nationalarchives/fix-all-the-mypy-issues

Fix all the mypy issues

.github/workflows/ci.yml

@@ -64,6 +64,9 @@ jobs:
       - name: Generate coverage XML
         run: docker-compose run django coverage xml
 
+      - name: Run mypy
+        run: docker-compose run django mypy ds_judgements_public_ui judgments
+
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
 

config/settings/base.py

@@ -2,6 +2,7 @@
 Base settings to build other settings files upon.
 """
 from pathlib import Path
+from typing import Any, Dict
 
 import django
 import environ
@@ -49,7 +50,7 @@
 # https://docs.djangoproject.com/en/stable/ref/settings/#std:setting-DEFAULT_AUTO_FIELD
 DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"
 
-FLAGS = {}
+FLAGS: Dict[str, Any] = {}
 
 # URLS
 # ------------------------------------------------------------------------------

fabfile.py

@@ -122,6 +122,7 @@ def test(c):
             "django",
             "mypy",
             "ds_judgements_public_ui",
+            "judgments",
         ]
     )
     # Pytest

judgments/feeds.py

@@ -1,4 +1,5 @@
 import datetime
+from typing import List, Optional
 
 from django.contrib.syndication.views import Feed
 from django.http import Http404
@@ -96,24 +97,24 @@ def link(self, obj):
         page = obj.get("page", 1)
         return f"/{obj['slug']}/atom.xml/?page={page}&order={obj.get('order')}"
 
-    def items(self, obj):
+    def items(self, obj) -> List[SearchResult]:
         return [
             SearchResult.create_from_node(result) for result in obj["model"].results
         ]
 
-    def item_description(self, item: SearchResult) -> str:
+    def item_description(self, item) -> str:
         return ""
 
-    def item_title(self, item: SearchResult):
+    def item_title(self, item):
         return item.name
 
-    def item_link(self, item: SearchResult) -> str:
+    def item_link(self, item) -> str:
         return reverse("detail", kwargs={"judgment_uri": item.uri})
 
-    def item_author_name(self, item: SearchResult) -> str:
+    def item_author_name(self, item) -> Optional[str]:
         return item.author
 
-    def item_extra_kwargs(self, item: SearchResult):
+    def item_extra_kwargs(self, item):
         extra_kwargs = super().item_extra_kwargs(item)
         extra_kwargs["uri"] = item.uri
         extra_kwargs["content_hash"] = item.content_hash
@@ -123,7 +124,7 @@ def item_updateddate(self, item: SearchResult) -> datetime.datetime:
         date_string = item.transformation_date or "1970-01-01T00:00:00.000"
         return datetime.datetime.fromisoformat(date_string)
 
-    def item_pubdate(self, item: SearchResult) -> datetime.datetime:
+    def item_pubdate(self, item: SearchResult) -> Optional[datetime.datetime]:
         return item.date
 
     def feed_extra_kwargs(self, obj):

judgments/models.py

@@ -1,5 +1,7 @@
 import logging
+from datetime import datetime
 from os.path import dirname, join
+from typing import Optional
 
 from caselawclient.Client import api_client
 from dateutil import parser as dateparser
@@ -24,9 +26,11 @@ def __init__(
         content_hash="",
         transformation_date="",
     ) -> None:
-        self.uri = uri
-        self.neutral_citation = neutral_citation
-        self.name = name
+        self.uri: str = uri
+        self.neutral_citation: str = neutral_citation
+        self.name: str = name
+        self.date: Optional[datetime]
+        self.court: Optional[str]
 
         try:
             self.date = dateparser.parse(date)
@@ -41,9 +45,9 @@ def __init__(
         except CourtNotFoundException:
             self.court = None
         self.matches = matches
-        self.author = author
+        self.author: Optional[str] = author
         self.last_modified = last_modified
-        self.content_hash = content_hash
+        self.content_hash: str = content_hash
         self.transformation_date = transformation_date
 
     @staticmethod

judgments/tests/test_search.py

@@ -42,6 +42,21 @@ def fake_search_result(
     )
 
 
+class TestBrowseResults(TestCase):
+    @patch("judgments.views.browse.perform_advanced_search")
+    @patch("judgments.models.SearchResult.create_from_node")
+    @patch("judgments.utils.perform_advanced_search")
+    def test_browse_results(self, f3, fake_result, fake_advanced_search):
+        fake_advanced_search.return_value = fake_search_results()
+        f3.r = fake_search_results()
+        fake_result.return_value = fake_search_result()
+        response = self.client.get("/ewhc/ch/2022")
+        self.assertContains(
+            response,
+            "A SearchResult name!",
+        )
+
+
 class TestSearchResults(TestCase):
     @patch("judgments.views.results.perform_advanced_search")
     @patch("judgments.models.SearchResult.create_from_node")

judgments/tests/tests.py

@@ -16,7 +16,7 @@ class TestJudgment(TestCase):
     @patch("judgments.views.detail.api_client")
     def test_valid_content(self, client, decoder, head):
         if "ASSETS_CDN_BASE_URL" not in environ:
-            raise "ensure ASSETS_CDN_BASE_URL is set in .env!"
+            raise RuntimeError("ensure ASSETS_CDN_BASE_URL is set in .env!")
         head.return_value.headers = {"Content-Length": "1234567890"}
         head.return_value.status_code = 200
         client.eval_xslt.return_value = "eval_xslt"
@@ -37,7 +37,7 @@ def test_valid_content(self, client, decoder, head):
     @patch("judgments.views.detail.api_client")
     def test_valid_content_no_filesize(self, client, decoder, head):
         if "ASSETS_CDN_BASE_URL" not in environ:
-            raise "ensure ASSETS_CDN_BASE_URL is set in .env!"
+            raise RuntimeError("ensure ASSETS_CDN_BASE_URL is set in .env!")
         head.return_value.headers = {}
         head.return_value.status_code = 200
         client.eval_xslt.return_value = "eval_xslt"
@@ -70,14 +70,14 @@ def test_no_valid_pdf(self, client, decoder, head):
         # We don't use the Download as PDF text because there's an issue with localisated strings on CI
         self.assertEqual(response.status_code, 200)
 
-    @skip
+    @skip("requires network")
     def test_good_response(self):
         response = self.client.get("/ewca/civ/2004/637")
         decoded_response = response.content.decode("utf-8")
         self.assertIn("[2004] EWCA Civ 637", decoded_response)
         self.assertEqual(response.status_code, 200)
 
-    @skip
+    @skip("requires network")
     def test_404_response(self):
         response = self.client.get("/ewca/civ/2004/63X")
         decoded_response = response.content.decode("utf-8")
@@ -143,8 +143,7 @@ class TestConverters(TestCase):
     def test_year_converter_parses_year(self):
         converter = converters.YearConverter()
         match = re.match(converter.regex, "1994")
-
-        self.assertEqual(match.group(0), "1994")
+        self.assertEqual(match.group(0), "1994")  # type: ignore
 
     def test_year_converter_converts_to_python(self):
         converter = converters.YearConverter()
@@ -157,7 +156,7 @@ def test_year_converter_converts_to_url(self):
     def test_date_converter_parses_date(self):
         converter = converters.DateConverter()
         match = re.match(converter.regex, "2022-02-28")
-        self.assertEqual(match.group(0), "2022-02-28")
+        self.assertEqual(match.group(0), "2022-02-28")  # type: ignore
 
     def test_date_converter_fails_to_parse_string(self):
         converter = converters.DateConverter()
@@ -167,7 +166,7 @@ def test_date_converter_fails_to_parse_string(self):
     def test_court_converter_parses_court(self):
         converter = converters.CourtConverter()
         match = re.match(converter.regex, "ewhc")
-        self.assertEqual(match.group(0), "ewhc")
+        self.assertEqual(match.group(0), "ewhc")  # type: ignore
 
     def test_court_converter_fails_to_parse(self):
         converter = converters.CourtConverter()
@@ -176,7 +175,7 @@ def test_court_converter_fails_to_parse(self):
     def test_subdivision_converter_parses_court(self):
         converter = converters.SubdivisionConverter()
         match = re.match(converter.regex, "comm")
-        self.assertEqual(match.group(0), "comm")
+        self.assertEqual(match.group(0), "comm")  # type:ignore
 
     def test_subdivision_converter_fails_to_parse(self):
         converter = converters.SubdivisionConverter()

judgments/utils.py

@@ -51,7 +51,7 @@ def perform_advanced_search(
     return SearchResults.create_from_string(multipart_data.parts[0].text)
 
 
-def preprocess_query(query):
+def preprocess_query(query: str) -> str:
     query = normalise_quotes(query)
     query = remove_unquoted_stop_words(query)
     return query
@@ -137,7 +137,7 @@ def paginator(current_page, total, size_per_page=RESULTS_PER_PAGE):
     }
 
 
-def get_pdf_uri(judgment_uri):
+def get_pdf_uri(judgment_uri: str) -> str:
     env = environ.Env()
     """Create a string saying where the S3 PDF will be for a judgment uri"""
     pdf_path = f'{judgment_uri}/{judgment_uri.replace("/", "_")}.pdf'

judgments/views/advanced_search.py

@@ -2,7 +2,6 @@
 
 from caselawclient.Client import RESULTS_PER_PAGE, MarklogicResourceNotFoundError
 from django.http import Http404
-from django.template import loader
 from django.template.response import TemplateResponse
 from ds_caselaw_utils import courts as all_courts
 
@@ -84,10 +83,9 @@ def advanced_search(request):
         context["courts"] = all_courts.get_selectable()
     except MarklogicResourceNotFoundError:
         raise Http404("Search failed")  # TODO: This should be something else!
-    template = loader.get_template("judgment/results.html")
     return TemplateResponse(
         request,
-        template,
+        "judgment/results.html",
         context={
             "context": context,
             "feedback_survey_type": "structured_search",

judgments/views/browse.py

@@ -1,8 +1,8 @@
 import datetime
+from typing import Any, Dict
 
 from caselawclient.Client import RESULTS_PER_PAGE, MarklogicResourceNotFoundError
 from django.http import Http404
-from django.template import loader
 from django.template.response import TemplateResponse
 from ds_caselaw_utils import courts as all_courts
 
@@ -27,7 +27,7 @@ def browse(request, court=None, subdivision=None, year=None):
         )
     )
 
-    context = {}
+    context: Dict[str, Any] = {}
 
     try:
         model = perform_advanced_search(
@@ -51,10 +51,9 @@ def browse(request, court=None, subdivision=None, year=None):
         context["courts"] = all_courts.get_selectable()
     except MarklogicResourceNotFoundError:
         raise Http404("Search failed")  # TODO: This should be something else!
-    template = loader.get_template("judgment/results.html")
     return TemplateResponse(
         request,
-        template,
+        "judgment/results.html",
         context={
             "context": context,
             "feedback_survey_type": "browse",

judgments/views/detail.py

@@ -10,7 +10,6 @@
 from django.conf import settings
 from django.http import Http404, HttpResponse
 from django.shortcuts import redirect
-from django.template import loader
 from django.template.defaultfilters import filesizeformat
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -87,11 +86,9 @@ def detail(request, judgment_uri):
         context["back_link"] = get_back_link(request)
     except MarklogicResourceNotFoundError:
         raise Http404("Judgment was not found")
-
-    template = loader.get_template("judgment/detail.html")
     return TemplateResponse(
         request,
-        template,
+        "judgment/detail.html",
         context={
             "context": context,
             "feedback_survey_type": "judgment",
@@ -113,15 +110,17 @@ def detail_xml(_request, judgment_uri):
 def get_pdf_size(judgment_uri):
     """Return the size of the S3 PDF for a judgment as a string in brackets, or an empty string if unavailable"""
     response = requests.head(
-        get_pdf_uri(judgment_uri), headers={"Accept-Encoding": None}
+        # it is possible that "" is a better value than None, but that is untested
+        get_pdf_uri(judgment_uri),
+        headers={"Accept-Encoding": None},  # type: ignore
     )
     content_length = response.headers.get("Content-Length", None)
     if response.status_code >= 400:
         return ""
     if content_length:
         filesize = filesizeformat(int(content_length))
         return f" ({filesize})"
-    logging.warn(f"Unable to determine PDF size for {judgment_uri}")
+    logging.warning(f"Unable to determine PDF size for {judgment_uri}")
     return " (unknown size)"
 
 

judgments/views/index.py

@@ -1,6 +1,5 @@
 from caselawclient.Client import MarklogicResourceNotFoundError
 from django.http import Http404
-from django.template import loader
 from django.template.response import TemplateResponse
 from ds_caselaw_utils import courts as all_courts
 
@@ -21,10 +20,9 @@ def index(request):
         raise Http404(
             "Search results not found"
         )  # TODO: This should be something else!
-    template = loader.get_template("pages/home.html")
     return TemplateResponse(
         request,
-        template,
+        "pages/home.html",
         context={
             "context": context,
             "courts": all_courts.get_listable_courts(),

judgments/views/results.py

@@ -1,8 +1,8 @@
 import urllib
+from typing import Any, Dict
 
 from caselawclient.Client import RESULTS_PER_PAGE, MarklogicAPIError
 from django.http import Http404
-from django.template import loader
 from django.template.response import TemplateResponse
 from django.utils.translation import gettext
 from ds_caselaw_utils import courts as all_courts
@@ -19,7 +19,7 @@
 
 
 def results(request):
-    context = {"page_title": gettext("results.search.title")}
+    context: Dict[str, Any] = {"page_title": gettext("results.search.title")}
     try:
         params = request.GET
         query = preprocess_query(params.get("query", ""))
@@ -73,10 +73,9 @@ def results(request):
         context["courts"] = all_courts.get_selectable()
     except MarklogicAPIError:
         raise Http404("Search error")  # TODO: This should be something else!
-    template = loader.get_template("judgment/results.html")
     return TemplateResponse(
         request,
-        template,
+        "judgment/results.html",
         context={
             "context": context,
             "feedback_survey_type": "search",

requirements/local.txt

@@ -31,3 +31,8 @@ django-coverage-plugin==3.0.0  # https://github.com/nedbat/django_coverage_plugi
 pytest-django==4.5.2  # https://github.com/pytest-dev/pytest-django
 pytest-cov==4.0.0
 pytest-timeout==2.1.0
+
+# type stubs
+types-python-dateutil==2.8.19.12
+types-requests==2.28.11.17
+django-stubs==1.16.0