diff --git a/app/accounts/admin.py b/app/accounts/admin.py
index 8c38f3f3..846f6b40 100644
--- a/app/accounts/admin.py
+++ b/app/accounts/admin.py
@@ -1,3 +1 @@
-from django.contrib import admin
-
# Register your models here.
diff --git a/app/accounts/models.py b/app/accounts/models.py
index 71a83623..6b202199 100644
--- a/app/accounts/models.py
+++ b/app/accounts/models.py
@@ -1,3 +1 @@
-from django.db import models
-
# Create your models here.
diff --git a/app/accounts/views.py b/app/accounts/views.py
index 375e0135..15e2e5b1 100644
--- a/app/accounts/views.py
+++ b/app/accounts/views.py
@@ -1,4 +1,3 @@
-from django.contrib.auth import authenticate
from django.contrib.auth import login as auth_login
from django.contrib.auth.views import LoginView as _LoginView
from django.contrib.auth.views import PasswordChangeView as _PasswordChangeView
diff --git a/app/app/urls.py b/app/app/urls.py
index afd11b25..6cc6f633 100644
--- a/app/app/urls.py
+++ b/app/app/urls.py
@@ -18,7 +18,6 @@
from django.conf import settings
from django.conf.urls.static import static
from django.contrib import admin
-from django.contrib.auth import views as auth_views
from django.urls import include, path
from django.utils.translation import gettext_lazy as _
diff --git a/app/general/filters.py b/app/general/filters.py
index 0eaa4882..fbd38c32 100644
--- a/app/general/filters.py
+++ b/app/general/filters.py
@@ -7,10 +7,10 @@
SearchVector,
)
from django.db.models import F, Value
-from django.db.models.functions import Greatest, Left
+from django.db.models.functions import Left
from django.db.models.query import EmptyQuerySet
from django.utils.translation import gettext_lazy as _
-from django_filters import ModelMultipleChoiceFilter, MultipleChoiceFilter
+from django_filters import ModelMultipleChoiceFilter
from general.models import Document, Institution, Language, Project, Subject
diff --git a/app/general/management/commands/import_documents.py b/app/general/management/commands/import_documents.py
index b4e2042d..bc9bad4b 100644
--- a/app/general/management/commands/import_documents.py
+++ b/app/general/management/commands/import_documents.py
@@ -47,7 +47,7 @@ def save_data(self, file_path, file_name):
document_data=pdf_to_text(file_path),
uploaded_file=content_file,
document_type="Glossary",
- institution_id=random.randint(1, 20),
+ institution_id=random.randint(1, 20), # noqa: S311 - this is a fixture; security isn't important
)
instance.save()
instance.languages.set(Language.objects.all())
diff --git a/app/general/templatetags/bs_icons.py b/app/general/templatetags/bs_icons.py
index fe459d98..407a580a 100644
--- a/app/general/templatetags/bs_icons.py
+++ b/app/general/templatetags/bs_icons.py
@@ -1,5 +1,3 @@
-import re
-
from django import template
from django.utils.safestring import mark_safe
@@ -12,20 +10,6 @@
register = template.Library()
-icon_name_re = re.compile(r"[a-z0-9\-]+")
-
-
-def _bs_icon(name):
- assert icon_name_re.fullmatch(name)
- return mark_safe(f' ')
- # The trailing space is intentional: Since this is an inline element
- # usually followed by text, the absence/presence of a space is significant,
- # and usually wanted for layout. That's too hard to remember, so we always
- # add it. Multiple spaces are equal to one. That way the exact layout of
- # code in the templates doesn't matter. Beware of using {% spaceless %}
- # which will negate this. A pure CSS solution escaped me thus far, since a
- # space will take additional space in addition to a margin.
-
# a mapping from project types to Bootstrap icon names:
_icons = {
@@ -44,4 +28,14 @@ def icon(name):
if not (bs_name := _icons.get(name)):
raise template.TemplateSyntaxError(f"'icon' requires a registered icon name (got {name!r})")
- return _bs_icon(bs_name)
+ # This `mark_safe` is okay because we only allow certain, whitelisted strings. This is enforced above by fetching it
+ # from the `_icons` dictionary
+ return mark_safe(f' ') # noqa: S308 - see above
+
+ # The trailing space is intentional: Since this is an inline element
+ # usually followed by text, the absence/presence of a space is significant,
+ # and usually wanted for layout. That's too hard to remember, so we always
+ # add it. Multiple spaces are equal to one. That way the exact layout of
+ # code in the templates doesn't matter. Beware of using {% spaceless %}
+ # which will negate this. A pure CSS solution escaped me thus far, since a
+ # space will take additional space in addition to a margin.
diff --git a/app/general/tests/test_document_detail.py b/app/general/tests/test_document_detail.py
index d6ea0c1b..b0e5c363 100644
--- a/app/general/tests/test_document_detail.py
+++ b/app/general/tests/test_document_detail.py
@@ -1,7 +1,7 @@
from django.test import TestCase
from django.urls import reverse
-from general.models import Document, Institution, Language, Project, Subject
+from general.models import Document, Institution, Language, Subject
class DocumentDetailViewTest(TestCase):
diff --git a/app/general/tests/test_filter.py b/app/general/tests/test_filter.py
index 36f36190..49b84e01 100644
--- a/app/general/tests/test_filter.py
+++ b/app/general/tests/test_filter.py
@@ -48,23 +48,23 @@ def setUp(self):
def test_institution_filter(self):
data = {"institution": [self.institution1.id]}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 2)
# TODO: ordering between documents and projects are not yet defined
self.assertEqual(qs[0]["id"], self.project1.id)
def test_subjects_filter(self):
data = {"subjects": [self.subject1.id]}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 1)
self.assertEqual(qs[0]["id"], self.doc1.id)
def test_languages_filter(self):
data = {"languages": [self.language1.id]}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 1)
self.assertEqual(qs[0]["id"], self.doc1.id)
@@ -74,35 +74,35 @@ def test_combined_filters(self):
"subjects": [self.subject1.id],
"languages": [self.language1.id],
}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 1)
self.assertEqual(qs[0]["id"], self.doc1.id)
def test_search_filter_documents(self):
data = {"search": "Document"}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 2)
self.assertCountEqual([qs[0]["id"], qs[1]["id"]], [self.doc1.id, self.doc2.id])
data = {"search": "Document 1"}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 1)
self.assertEqual(qs[0]["id"], self.doc1.id)
def test_search_filter_projects(self):
data = {"search": "Project 1"}
- filter = DocumentFilter(data=data)
- qs = filter.qs
+ doc_filter = DocumentFilter(data=data)
+ qs = doc_filter.qs
self.assertEqual(len(qs), 1)
self.assertEqual(qs[0]["id"], self.project1.id)
def test_search_filter_combined(self):
data = {"search": "1"}
- filter = DocumentFilter(data=data)
- qs_ids = [x["id"] for x in filter.qs]
+ doc_filter = DocumentFilter(data=data)
+ qs_ids = [x["id"] for x in doc_filter.qs]
expected_ids = [self.doc1.id, self.project1.id, self.institution1.id]
self.assertCountEqual(qs_ids, expected_ids)
# TODO: test properly instead of relying on randomly agreeing IDs
diff --git a/app/general/tests/test_frontend.py b/app/general/tests/test_frontend.py
index 3b231907..7a807b17 100644
--- a/app/general/tests/test_frontend.py
+++ b/app/general/tests/test_frontend.py
@@ -78,9 +78,10 @@ def test_no_404s(self):
# Sanity check in case we ever change the 404 title
self.driver.get(f"{self.live_server_url}/blabla404")
print(self.driver.title)
- assert self.driver.title.startswith(
- "Error"
- ), f"Actual title was {self.driver.title}. Page: {self.driver.page_source}"
+ self.assertTrue(
+ self.driver.title.startswith("Error"),
+ f"Actual title was {self.driver.title}. Page: {self.driver.page_source}",
+ )
# Check main page does not 404
self.driver.get(self.live_server_url)
@@ -98,15 +99,22 @@ def check_nav_item(self, link_text):
wait = WebDriverWait(self.driver, timeout=WAIT_TIMEOUT)
wait.until(lambda d: link_text in self.driver.title)
except TimeoutException:
- assert link_text in self.driver.title, (
- f"Expected title for page {link_text} to have {link_text};"
- f" was {self.driver.title}"
+ self.assertIn(
+ link_text,
+ self.driver.title,
+ (
+ f"Expected title for page {link_text} to have {link_text};"
+ f" was {self.driver.title}"
+ ),
)
self.assert_current_page_not_error()
def assert_current_page_not_error(self):
- assert not self.driver.title.startswith("Error"), f"Actual title was {self.driver.title}"
- assert not self.driver.title.startswith(
- "ProgrammingError"
- ), f"Actual title was {self.driver.title}"
- assert not self.driver.find_element(By.ID, "error-block").is_displayed()
+ self.assertFalse(
+ self.driver.title.startswith("Error"), f"Actual title was {self.driver.title}"
+ )
+ self.assertFalse(
+ self.driver.title.startswith("ProgrammingError"),
+ f"Actual title was {self.driver.title}",
+ )
+ self.assertFalse(self.driver.find_element(By.ID, "error-block").is_displayed())
diff --git a/app/general/tests/test_import_documents.py b/app/general/tests/test_import_documents.py
index a18de80b..1925f5ce 100644
--- a/app/general/tests/test_import_documents.py
+++ b/app/general/tests/test_import_documents.py
@@ -41,13 +41,13 @@ def test_save_data(self):
command = Command()
# Create some Institutions instances for testing
for i in range(1, 21):
- id = random.randint(1, 1000)
+ institution_id = random.randint(1, 1000) # noqa: S311 - this is only for testing
Institution.objects.create(
id=i,
- name=f"{id}_{self.fake.company()}",
- abbreviation=f"{id}_{self.fake.company_suffix()}",
- url=f"{id}_{self.fake.url()}",
- email=f"{id}_{self.fake.company_email()}",
+ name=f"{institution_id}_{self.fake.company()}",
+ abbreviation=f"{institution_id}_{self.fake.company_suffix()}",
+ url=f"{institution_id}_{self.fake.url()}",
+ email=f"{institution_id}_{self.fake.company_email()}",
logo="",
)
diff --git a/app/general/tests/test_project_detail.py b/app/general/tests/test_project_detail.py
index 5991b237..4ba4f539 100644
--- a/app/general/tests/test_project_detail.py
+++ b/app/general/tests/test_project_detail.py
@@ -49,7 +49,7 @@ def test_project_detail_view_context(self):
def test_project_detail_view_num_queries(self):
with self.assertNumQueries(3):
- response = self.client.get(reverse("project_detail", args=[self.project.id]))
+ self.client.get(reverse("project_detail", args=[self.project.id]))
def test_project_detail_view_invalid_id(self):
invalid_project_id = self.project.id + 1
diff --git a/app/general/tests/test_view_search.py b/app/general/tests/test_view_search.py
index 2a3b7af8..56a45835 100644
--- a/app/general/tests/test_view_search.py
+++ b/app/general/tests/test_view_search.py
@@ -1,6 +1,6 @@
import unittest
-from django.test import Client, TestCase
+from django.test import TestCase
from django.urls import reverse
from general.models import Document, Institution, Language, Subject
diff --git a/app/general/tests/test_views_institution.py b/app/general/tests/test_views_institution.py
index 9690c56f..a39e30c0 100644
--- a/app/general/tests/test_views_institution.py
+++ b/app/general/tests/test_views_institution.py
@@ -1,4 +1,4 @@
-from django.test import Client, TestCase
+from django.test import TestCase
from django.urls import reverse
from general.models import Document, Institution, Project
diff --git a/app/pyproject.toml b/app/pyproject.toml
index 5be1384c..063daac0 100644
--- a/app/pyproject.toml
+++ b/app/pyproject.toml
@@ -3,8 +3,11 @@ line-length = 100
exclude = ["migrations", ".venv"]
[tool.ruff.lint]
-select = [
- "DJ",
- "I",
- "W"
+extend-select = [
+ "DJ", # Django - flake-8-django - https://docs.astral.sh/ruff/rules/#flake8-django-dj
+ "I", # Imports - isort - https://docs.astral.sh/ruff/rules/#isort-i
+ "W", # General - pycodestyle - https://docs.astral.sh/ruff/rules/#pycodestyle-e-w
+ "S", # Security - flake-8-bandit https://docs.astral.sh/ruff/rules/#flake8-bandit-s
+ "F", # General - Pyflakes - https://docs.astral.sh/ruff/rules/#pyflakes-f
+ "A", # Shadowing of builtins - flake-8-builtins - https://docs.astral.sh/ruff/rules/#flake8-builtins-a
]