Make sure everything passes the linters, and enable linting via the m…

…akefile (#681) * feat: add lint command to makefile We have this as a precommit hook already, but it's not enforced anywhere, so at the moment our codebase doesn't pass the lint check * fix: lint * fix: remove djhtml - it's not reliable enough
ministryofjustice · Aug 19, 2024 · 2e1985e · 2e1985e
1 parent 91f9f2d
commit 2e1985e
Show file tree

Hide file tree

Showing 22 changed files with 91 additions and 121 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -22,13 +22,14 @@ repos:
     hooks:
       - id: flake8
         name: flake8 lint
-        args: [
-          '--ignore=E203,E266,W503,F403',
-          '--exclude=".git, .mypy_cache, .pytest_cache, build, dist"',
-          '--max-line-length=120',
-          '--max-complexity=18',
-          '--select="B,C,E,F,W,T4,B9"'
-        ]
+        args:
+          [
+            "--ignore=E203,E266,W503,F403",
+            '--exclude=".git, .mypy_cache, .pytest_cache, build, dist"',
+            "--max-line-length=120",
+            "--max-complexity=18",
+            '--select="B,C,E,F,W,T4,B9"',
+          ]
         additional_dependencies:
           - flake8-broken-line
           - flake8-bugbear
@@ -56,10 +57,3 @@ repos:
     hooks:
       - id: sync_with_poetry
         args: []
-
-  - repo: https://github.com/rtts/djhtml
-    rev: "3.0.6" # replace with the latest tag on GitHub
-    hooks:
-      - id: djhtml
-      - id: djcss
-      - id: djjs
diff --git a/Makefile b/Makefile
@@ -61,7 +61,7 @@ run:
 	poetry run python manage.py runserver
 
 # Run unit tests
-test: unit integration
+test: unit integration lint
 
 # Run Python unit tests
 unit:
@@ -78,4 +78,7 @@ clean:
 	rm -f $(ENV_FILE)
 	find . -name "*.pyc" -exec rm -f {} \;
 
-.PHONY: all build install_deps set_env collect_static migrate setup_waffle_switches messages compilemessages run test unit integration clean
+lint:
+	pre-commit run --all-files
+
+.PHONY: all build install_deps set_env collect_static migrate setup_waffle_switches messages compilemessages run test unit integration clean lint
diff --git a/feedback/admin.py b/feedback/admin.py
diff --git a/feedback/templates/thanks.html b/feedback/templates/thanks.html
@@ -3,14 +3,14 @@
 {% load static %}
 
 {% block content %}
-<div class="govuk-grid-row"> 
+  <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <h1 class="govuk-heading-xl">
         {% translate 'Thank you for your feedback' %}
       </h1>
 
       <p class="govuk-body">{% translate 'Your feedback will help us improve the service.' %}</p>
-  </div>
+    </div>
 
-</div>
-{% endblock content %}
+  </div>
+{% endblock content %}
diff --git a/feedback/tests.py b/feedback/tests.py
diff --git a/home/templatetags/snippets.py b/home/templatetags/snippets.py
@@ -54,7 +54,7 @@ def _align_snippet(snippet, max_chars):
     Align a snippet to a few words before start_mark_idx
     """
     start_mark_idx = snippet.find("<mark>")
-    end_mark_idx = snippet[start_mark_idx + 1:].find("</mark>")
+    end_mark_idx = snippet[start_mark_idx + 1 :].find("</mark>")
 
     # If the mark is at the beginning of the first remaining paragraph,
     # no further alignment is required.

diff --git a/jest.config.js b/jest.config.js
@@ -11,9 +11,6 @@ const config = {
   // Stop running tests after `n` failures
   // bail: 0,
 
-  // The directory where Jest should store its cached dependency information
-  // cacheDirectory: "/private/var/folders/lh/0dtbp3254mvc5pvhcx4ngnf80000gq/T/jest_dz",
-
   // Automatically clear mock calls, instances, contexts and results before every test
   clearMocks: true,
 

diff --git a/lib/datahub-client/data_platform_catalogue/client/search.py b/lib/datahub-client/data_platform_catalogue/client/search.py
@@ -2,9 +2,6 @@
 import logging
 from typing import Any, Sequence, Tuple
 
-from datahub.configuration.common import GraphError  # pylint: disable=E0611
-from datahub.ingestion.graph.client import DataHubGraph  # pylint: disable=E0611
-
 from data_platform_catalogue.client.exceptions import CatalogueError
 from data_platform_catalogue.client.graphql_helpers import (
     get_graphql_query,
@@ -28,6 +25,8 @@
     SearchResult,
     SortOption,
 )
+from datahub.configuration.common import GraphError  # pylint: disable=E0611
+from datahub.ingestion.graph.client import DataHubGraph  # pylint: disable=E0611
 
 logger = logging.getLogger(__name__)
 
@@ -115,24 +114,31 @@ def _parse_search_results(self, response) -> Tuple[list, list]:
 
             try:
                 if entity_type == "DATASET":
-                    parsed_result = self._parse_dataset(entity, matched_fields, ResultType.TABLE)
+                    parsed_result = self._parse_dataset(
+                        entity, matched_fields, ResultType.TABLE
+                    )
                     page_results.append(parsed_result)
                 elif entity_type == "CHART":
-                    parsed_result = self._parse_dataset(entity, matched_fields, ResultType.CHART)
+                    parsed_result = self._parse_dataset(
+                        entity, matched_fields, ResultType.CHART
+                    )
                     page_results.append(parsed_result)
                 elif entity_type == "CONTAINER":
-                    parsed_result = self._parse_container(entity, matched_fields, ResultType.DATABASE)
+                    parsed_result = self._parse_container(
+                        entity, matched_fields, ResultType.DATABASE
+                    )
                     page_results.append(parsed_result)
                 elif entity_type == "DASHBOARD":
-                    parsed_result = self._parse_container(entity, matched_fields, ResultType.DASHBOARD)
+                    parsed_result = self._parse_container(
+                        entity, matched_fields, ResultType.DASHBOARD
+                    )
                     page_results.append(parsed_result)
                 else:
                     raise Exception
             except Exception:
                 logger.warn(f"Parsing for result {entity_urn} failed")
                 malformed_result_urns.append(entity_urn)
 
-
         return page_results, malformed_result_urns
 
     @staticmethod

diff --git a/lib/datahub-client/tests/client/datahub/test_datahub_client.py b/lib/datahub-client/tests/client/datahub/test_datahub_client.py
@@ -3,7 +3,6 @@
 from unittest.mock import MagicMock, patch
 
 import pytest
-
 from data_platform_catalogue.client.datahub_client import (
     DataHubCatalogueClient,
     InvalidDomain,

diff --git a/lib/datahub-client/tests/client/datahub/test_graphql_helpers.py b/lib/datahub-client/tests/client/datahub/test_graphql_helpers.py
@@ -1,7 +1,6 @@
 from datetime import datetime, timezone
 
 import pytest
-
 from data_platform_catalogue.client.graphql_helpers import (
     _make_user_email_from_urn,
     parse_columns,

diff --git a/lib/datahub-client/tests/client/datahub/test_search.py b/lib/datahub-client/tests/client/datahub/test_search.py
@@ -247,6 +247,7 @@ def test_dataset_result(mock_graph, searcher):
     )
     assert response == expected
 
+
 def test_bad_entity_type(mock_graph, searcher):
     datahub_response = {
         "searchAcrossEntities": {
@@ -275,7 +276,9 @@ def test_bad_entity_type(mock_graph, searcher):
     expected = expected = SearchResponse(
         total_results=1,
         page_results=[],
-        malformed_result_urns=["urn:li:dataset:(urn:li:dataPlatform:bigquery,calm-pagoda-323403.jaffle_shop.customers,PROD)"],
+        malformed_result_urns=[
+            "urn:li:dataset:(urn:li:dataPlatform:bigquery,calm-pagoda-323403.jaffle_shop.customers,PROD)"
+        ],
         facets=SearchFacets(facets={}),
     )
     assert response == expected
@@ -344,9 +347,9 @@ def test_2_dataset_results_with_one_malformed_result(mock_graph, searcher):
                                     "value": "moj-reg-prod-hmpps-assess-risks-and-needs-prod-glue-job",
                                 },
                             ],
-                        }
+                        },
                     },
-                }
+                },
             ],
         }
     }

diff --git a/lib/datahub-client/tests/test_helpers/mce_helpers.py b/lib/datahub-client/tests/test_helpers/mce_helpers.py
@@ -24,6 +24,7 @@
     load_json_file,
 )
 from datahub.utilities.urns.urn import Urn
+
 from tests.test_helpers.type_helpers import PytestConfig
 
 logger = logging.getLogger(__name__)
@@ -154,20 +155,16 @@ def get_entity_urns(events_file: str) -> Set[str]:
 def _get_entity_urns(events_list: List[Dict]) -> Set[str]:
     entity_type = "dataset"
     # mce urns
-    mce_urns = set(
-        [
-            _get_element(x, _get_mce_urn_path_spec(entity_type))
-            for x in events_list
-            if _get_filter(mce=True, entity_type=entity_type)(x)
-        ]
-    )
-    mcp_urns = set(
-        [
-            _get_element(x, _get_mcp_urn_path_spec())
-            for x in events_list
-            if _get_filter(mcp=True, entity_type=entity_type)(x)
-        ]
-    )
+    mce_urns = {
+        _get_element(x, _get_mce_urn_path_spec(entity_type))
+        for x in events_list
+        if _get_filter(mce=True, entity_type=entity_type)(x)
+    }
+    mcp_urns = {
+        _get_element(x, _get_mcp_urn_path_spec())
+        for x in events_list
+        if _get_filter(mcp=True, entity_type=entity_type)(x)
+    }
     all_urns = mce_urns.union(mcp_urns)
     return all_urns
 
@@ -250,24 +247,20 @@ def assert_for_each_entity(
     test_output = load_json_file(file)
     assert isinstance(test_output, list)
     # mce urns
-    mce_urns = set(
-        [
-            _get_element(x, _get_mce_urn_path_spec(entity_type))
-            for x in test_output
-            if _get_filter(mce=True, entity_type=entity_type)(x)
-        ]
-    )
-    mcp_urns = set(
-        [
-            _get_element(x, _get_mcp_urn_path_spec())
-            for x in test_output
-            if _get_filter(mcp=True, entity_type=entity_type)(x)
-        ]
-    )
+    mce_urns = {
+        _get_element(x, _get_mce_urn_path_spec(entity_type))
+        for x in test_output
+        if _get_filter(mce=True, entity_type=entity_type)(x)
+    }
+    mcp_urns = {
+        _get_element(x, _get_mcp_urn_path_spec())
+        for x in test_output
+        if _get_filter(mcp=True, entity_type=entity_type)(x)
+    }
     all_urns = mce_urns.union(mcp_urns)
     # there should not be any None urns
     assert None not in all_urns
-    aspect_map = {urn: None for urn in all_urns}
+    aspect_map = dict.fromkeys(all_urns)
     # iterate over all mcps
     for o in [
         mcp
@@ -362,20 +355,16 @@ def assert_entity_urn_not_like(entity_type: str, regex_pattern: str, file: str)
     test_output = load_json_file(file)
     assert isinstance(test_output, list)
     # mce urns
-    mce_urns = set(
-        [
-            _get_element(x, _get_mce_urn_path_spec(entity_type))
-            for x in test_output
-            if _get_filter(mce=True, entity_type=entity_type)(x)
-        ]
-    )
-    mcp_urns = set(
-        [
-            _get_element(x, _get_mcp_urn_path_spec())
-            for x in test_output
-            if _get_filter(mcp=True, entity_type=entity_type)(x)
-        ]
-    )
+    mce_urns = {
+        _get_element(x, _get_mce_urn_path_spec(entity_type))
+        for x in test_output
+        if _get_filter(mce=True, entity_type=entity_type)(x)
+    }
+    mcp_urns = {
+        _get_element(x, _get_mcp_urn_path_spec())
+        for x in test_output
+        if _get_filter(mcp=True, entity_type=entity_type)(x)
+    }
     all_urns = mce_urns.union(mcp_urns)
     print(all_urns)
     matched_urns = [u for u in all_urns if re.match(regex_pattern, u)]
@@ -392,20 +381,16 @@ def assert_entity_urn_like(entity_type: str, regex_pattern: str, file: str) -> i
     test_output = load_json_file(file)
     assert isinstance(test_output, list)
     # mce urns
-    mce_urns = set(
-        [
-            _get_element(x, _get_mce_urn_path_spec(entity_type))
-            for x in test_output
-            if _get_filter(mce=True, entity_type=entity_type)(x)
-        ]
-    )
-    mcp_urns = set(
-        [
-            _get_element(x, _get_mcp_urn_path_spec())
-            for x in test_output
-            if _get_filter(mcp=True, entity_type=entity_type)(x)
-        ]
-    )
+    mce_urns = {
+        _get_element(x, _get_mce_urn_path_spec(entity_type))
+        for x in test_output
+        if _get_filter(mce=True, entity_type=entity_type)(x)
+    }
+    mcp_urns = {
+        _get_element(x, _get_mcp_urn_path_spec())
+        for x in test_output
+        if _get_filter(mcp=True, entity_type=entity_type)(x)
+    }
     all_urns = mce_urns.union(mcp_urns)
     print(all_urns)
     matched_urns = [u for u in all_urns if re.match(regex_pattern, u)]

diff --git a/scss/_details.scss b/scss/_details.scss
@@ -3,4 +3,4 @@
     padding: govuk-spacing(3) govuk-spacing(4);
     margin-bottom: govuk-spacing(6);
     overflow-wrap: break-word;
-}
+}
diff --git a/scss/_glossary.scss b/scss/_glossary.scss
@@ -6,4 +6,4 @@
         overflow-y: auto;
         margin-bottom: govuk-spacing(6);
     }
-}
+}
diff --git a/static/assets/js/enhanced-glossary.js b/static/assets/js/enhanced-glossary.js
@@ -30,7 +30,7 @@ const updateResults = () => {
     const terms = Array.from(el.querySelectorAll(".term"));
     const isEmpty =
       terms.length === 0 ||
-      terms.every((term) => term.classList.contains("govuk-!-display-none"));
+    terms.every((term) => term.classList.contains("govuk-!-display-none"));
 
     if (isEmpty) {
       el.classList.add("govuk-!-display-none");

diff --git a/templates/base/footer.html b/templates/base/footer.html
@@ -50,9 +50,9 @@ <h2 class="govuk-visually-hidden">{% translate "Support links" %}</h2>
       </div>
       <div class="govuk-footer__meta-item">
         {% blocktranslate trimmed with href="https://www.nationalarchives.gov.uk/information-management/re-using-public-sector-information/uk-government-licensing-framework/crown-copyright/" %}
-        <a
-          class="govuk-footer__link govuk-footer__copyright-logo"
-          href="{{ href }}">© Crown copyright</a>
+          <a
+            class="govuk-footer__link govuk-footer__copyright-logo"
+            href="{{ href }}">© Crown copyright</a>
         {% endblocktranslate %}
       </div>
     </div>

diff --git a/tests/home/templatetags/test_snippets.py b/tests/home/templatetags/test_snippets.py
@@ -13,12 +13,12 @@
         (
             "a" * 200 + "\n\n" + "word word word " * 100 + "hello <mark>world</mark>",
             200,
-            "…word word word word word word word word word word word word word word word word word word word word word word word hello <mark>world</mark>",
+            "…word word word word word word word word word word word word word word word word word word word word word word word hello <mark>world</mark>",  # noqa: E501
         ),
         (
-            "A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or remand, recall notices not acted upon, sentence miscalculation or discharging the wrong person on escort.",
+            "A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or remand, recall notices not acted upon, sentence miscalculation or discharging the wrong person on escort.",  # noqa: E501
             300,
-            "A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or…",
+            "A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or…",  # noqa: E501
         ),
     ],
     ids=(

diff --git a/tests/integration/test_primary_nav.py b/tests/integration/test_primary_nav.py
@@ -32,4 +32,3 @@ def start_on_the_home_page(self):
 
         assert heading_text == "Find MoJ data"
         assert self.selenium.title.split("-")[0].strip() == "Home"
-
diff --git a/users/admin.py b/users/admin.py
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,4 +6,4 @@ @@
             overflow-y: auto;
             margin-bottom: govuk-spacing(6);
         }
-    }
+    }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -32,4 +32,3 @@ def start_on_the_home_page(self):

		assert heading_text == "Find MoJ data"
		assert self.selenium.title.split("-")[0].strip() == "Home"