From f2dfb7dfca39c8d0a5bb7e64460b165beb9100e4 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:09:52 +0100 Subject: [PATCH 01/10] Add a helper for compiling dependencies --- script/compile-requirements.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100755 script/compile-requirements.sh diff --git a/script/compile-requirements.sh b/script/compile-requirements.sh new file mode 100755 index 0000000..2bb2f6f --- /dev/null +++ b/script/compile-requirements.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +set -euo pipefail + +cd $(dirname $0)/.. + +echo "Compiling main requirements" +pip-compile --quiet requirements.in "$@" + +echo "Compiling development requirements" +pip-compile --quiet requirements-dev.in "$@" From 3a0b71103360915facb4809541552d8053fbff2f Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:10:54 +0100 Subject: [PATCH 02/10] Bump Django --- requirements-dev.txt | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index c6b3080..2ab17af 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -40,7 +40,7 @@ defusedxml==0.7.1 # via # -r requirements.txt # python3-openid -django==4.2.3 +django==4.2.5 # via # -r requirements.txt # crispy-bulma diff --git a/requirements.txt b/requirements.txt index d9b0097..beeb07a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,7 +20,7 @@ cryptography==41.0.3 # via pyjwt defusedxml==0.7.1 # via python3-openid -django==4.2.3 +django==4.2.5 # via # -r requirements.in # crispy-bulma From e0160a2adc945dcd580f66abb643a403b7fab9fd Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:11:15 +0100 Subject: [PATCH 03/10] Trailing newlines --- requirements-dev.in | 2 +- requirements.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-dev.in b/requirements-dev.in index 4908a9e..bc028f5 100644 --- a/requirements-dev.in +++ b/requirements-dev.in @@ -15,4 +15,4 @@ types-dj-database-url types-setuptools faker -freezegun \ No newline at end of file +freezegun diff --git a/requirements.in b/requirements.in index 8b06011..15a5619 100644 --- a/requirements.in +++ b/requirements.in @@ -7,4 +7,4 @@ django-filter django-stubs-ext django-tables2 django-tables2-bulma-template -requests \ No newline at end of file +requests From 82d03d1457bea8c0956bacba864f8f264f0b5560 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:12:54 +0100 Subject: [PATCH 04/10] Bump django-stubs-ext --- requirements-dev.txt | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 2ab17af..07581e7 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -65,7 +65,7 @@ django-stubs[compatible-mypy]==1.14.0 # -r requirements-dev.in # django-filter-stubs # djangorestframework-stubs -django-stubs-ext==0.7.0 +django-stubs-ext==4.2.2 # via # -r requirements.txt # django-stubs diff --git a/requirements.txt b/requirements.txt index beeb07a..d8f1267 100644 --- a/requirements.txt +++ b/requirements.txt @@ -37,7 +37,7 @@ django-crispy-forms==1.14.0 # crispy-bulma django-filter==22.1 # via -r requirements.in -django-stubs-ext==0.7.0 +django-stubs-ext==4.2.2 # via -r requirements.in django-tables2==2.5.2 # via From d0ab122316c73da825474879b95ade4f44daf80b Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:14:08 +0100 Subject: [PATCH 05/10] Bump development dependencies --- requirements-dev.txt | 61 ++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 07581e7..9997e9a 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -8,9 +8,7 @@ asgiref==3.6.0 # via # -r requirements.txt # django -attrs==22.2.0 - # via pytest -build==0.10.0 +build==1.0.3 # via pip-tools cachetools==5.3.0 # via -r requirements.txt @@ -26,9 +24,9 @@ charset-normalizer==3.1.0 # via # -r requirements.txt # requests -click==8.1.3 +click==8.1.7 # via pip-tools -coverage[toml]==7.1.0 +coverage[toml]==7.3.1 # via pytest-cov crispy-bulma==0.8.3 # via -r requirements.txt @@ -58,9 +56,9 @@ django-crispy-forms==1.14.0 # crispy-bulma django-filter==22.1 # via -r requirements.txt -django-filter-stubs==0.1.2 +django-filter-stubs==0.1.3 # via -r requirements-dev.in -django-stubs[compatible-mypy]==1.14.0 +django-stubs[compatible-mypy]==4.2.4 # via # -r requirements-dev.in # django-filter-stubs @@ -75,9 +73,11 @@ django-tables2==2.5.2 # django-tables2-bulma-template django-tables2-bulma-template==0.2.0 # via -r requirements.txt -djangorestframework-stubs==1.8.0 +djangorestframework-stubs==3.14.2 # via django-filter-stubs -faker==16.7.0 +exceptiongroup==1.1.3 + # via pytest +faker==19.6.1 # via -r requirements-dev.in freezegun==1.2.2 # via -r requirements-dev.in @@ -89,25 +89,25 @@ iniconfig==2.0.0 # via pytest isort==5.12.0 # via -r requirements-dev.in -mypy==0.991 +mypy==1.5.1 # via # -r requirements-dev.in # django-filter-stubs # django-stubs # djangorestframework-stubs -mypy-extensions==0.4.3 +mypy-extensions==1.0.0 # via mypy oauthlib==3.2.2 # via # -r requirements.txt # requests-oauthlib -packaging==23.0 +packaging==23.1 # via # build # pytest -pip-tools==6.13.0 +pip-tools==7.3.0 # via -r requirements-dev.in -pluggy==1.0.0 +pluggy==1.3.0 # via pytest pycparser==2.21 # via @@ -119,12 +119,12 @@ pyjwt[crypto]==2.6.0 # django-allauth pyproject-hooks==1.0.0 # via build -pytest==7.2.1 +pytest==7.4.2 # via # -r requirements-dev.in # pytest-cov # pytest-django -pytest-cov==4.0.0 +pytest-cov==4.1.0 # via -r requirements-dev.in pytest-django==4.5.2 # via -r requirements-dev.in @@ -146,7 +146,7 @@ requests-oauthlib==1.3.1 # via # -r requirements.txt # django-allauth -ruff==0.0.247 +ruff==0.0.290 # via -r requirements-dev.in six==1.16.0 # via python-dateutil @@ -155,24 +155,29 @@ sqlparse==0.4.4 # -r requirements.txt # django tomli==2.0.1 - # via django-stubs -types-cachetools==5.3.0.4 + # via + # build + # coverage + # django-stubs + # mypy + # pip-tools + # pyproject-hooks + # pytest +types-cachetools==5.3.0.6 # via -r requirements-dev.in -types-dj-database-url==1.2.0.1 +types-dj-database-url==1.3.0.4 # via -r requirements-dev.in -types-docutils==0.19.1.2 - # via types-setuptools -types-pytz==2022.7.1.0 +types-pytz==2023.3.0.1 # via django-stubs -types-pyyaml==6.0.12.3 +types-pyyaml==6.0.12.11 # via # django-stubs # djangorestframework-stubs -types-requests==2.28.11.12 +types-requests==2.31.0.2 # via djangorestframework-stubs -types-setuptools==65.7.0.3 +types-setuptools==68.2.0.0 # via -r requirements-dev.in -types-urllib3==1.26.25.5 +types-urllib3==1.26.25.14 # via types-requests typing-extensions==4.4.0 # via @@ -186,7 +191,7 @@ urllib3==1.26.15 # via # -r requirements.txt # requests -wheel==0.38.4 +wheel==0.41.2 # via pip-tools # The following packages are considered to be unsafe in a requirements file: From 78b0329848f96390dc2fbe3a783527155cb6dab4 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:15:52 +0100 Subject: [PATCH 06/10] Trim trailing whitespace (via ruff) Also add an editorconfig to simplify meeting this. --- .editorconfig | 15 +++++++++++ helpdesk/accounts/forms.py | 2 +- helpdesk/accounts/views.py | 2 +- helpdesk/helpdesk/views.py | 4 +-- .../management/commands/import_from_srcomp.py | 2 +- helpdesk/teams/models.py | 2 +- helpdesk/teams/srcomp.py | 4 +-- helpdesk/teams/views.py | 6 ++--- helpdesk/tickets/models.py | 26 +++++++++---------- helpdesk/tickets/views.py | 8 +++--- 10 files changed, 43 insertions(+), 28 deletions(-) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..a895e9d --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +[*] +indent_size = 4 +indent_style = space + +insert_final_newline = true +trim_trailing_whitespace = true + +[*.html] +indent_size = 2 + +[*.yml] +indent_size = 2 + +[Makefile] +indent_style = tab diff --git a/helpdesk/accounts/forms.py b/helpdesk/accounts/forms.py index 70cba94..7eaa3aa 100644 --- a/helpdesk/accounts/forms.py +++ b/helpdesk/accounts/forms.py @@ -16,7 +16,7 @@ class SignupForm(UserCreationForm): widget=forms.PasswordInput(), required=True, ) - + class Meta: model = User fields = ("username",) diff --git a/helpdesk/accounts/views.py b/helpdesk/accounts/views.py index 8748e9f..1c12d36 100644 --- a/helpdesk/accounts/views.py +++ b/helpdesk/accounts/views.py @@ -60,7 +60,7 @@ def get_form(self, form_class: type[BaseModelForm] | None = None) -> BaseModelFo form.fields['first_name'].required = True form.fields['first_name'].label = "Given name" return form - + def form_valid(self, form: BaseModelForm) -> HttpResponse: messages.info(self.request, f"Welcome to {settings.SYSTEM_TITLE}!") self.object.onboarded_at = timezone.now() diff --git a/helpdesk/helpdesk/views.py b/helpdesk/helpdesk/views.py index 71d51f5..e9f7197 100644 --- a/helpdesk/helpdesk/views.py +++ b/helpdesk/helpdesk/views.py @@ -56,7 +56,7 @@ class SearchResult(TypedDict): class SearchView(LoginRequiredMixin, SingleTableMixin, TemplateView): - + template_name = "search.html" table_class = SearchTable @@ -73,7 +73,7 @@ def _get_filters(self, q: str) -> dict[type[Ticket] | type[Team], Q]: Ticket: Q(title__icontains=q) | Q(events__comment__icontains=q) | Q(id=ticket_id), Team: Q(tla__icontains=q) | Q(name__icontains=q), } - + def get_result_count(self, q: str) -> int: filters = self._get_filters(q) return sum( diff --git a/helpdesk/teams/management/commands/import_from_srcomp.py b/helpdesk/teams/management/commands/import_from_srcomp.py index 0e57e22..ea8230f 100644 --- a/helpdesk/teams/management/commands/import_from_srcomp.py +++ b/helpdesk/teams/management/commands/import_from_srcomp.py @@ -36,7 +36,7 @@ def _import_pit_locations(self, srcomp_url: str) -> None: slug=slug, defaults={"name": location_data["display_name"]}, ) - + def _import_teams(self, srcomp_url: str) -> None: resp = requests.get(f"{srcomp_url}/teams", timeout=5) resp.raise_for_status() diff --git a/helpdesk/teams/models.py b/helpdesk/teams/models.py index 8322aae..62e1e72 100644 --- a/helpdesk/teams/models.py +++ b/helpdesk/teams/models.py @@ -32,7 +32,7 @@ def __str__(self) -> str: def get_absolute_url(self) -> str: return reverse_lazy('teams:team_detail', args=[self.tla]) - + class Meta: ordering = ["tla"] diff --git a/helpdesk/teams/srcomp.py b/helpdesk/teams/srcomp.py index b3ce012..b9e7553 100644 --- a/helpdesk/teams/srcomp.py +++ b/helpdesk/teams/srcomp.py @@ -29,7 +29,7 @@ def _get(self, endpoint: str) -> dict[str, Any] | None: return None except ConnectionError: return None - + try: return resp.json() except JSONDecodeError: @@ -39,7 +39,7 @@ def _get(self, endpoint: str) -> dict[str, Any] | None: def get_score_info_for_team(self, tla: str) -> ScoreInfo | None: if not self._base_url: return None - + if data := self._get(f"teams/{tla}"): return ScoreInfo( league_pos=data.get("league_pos"), # type: ignore[arg-type] diff --git a/helpdesk/teams/views.py b/helpdesk/teams/views.py index 6f8e86f..3faf326 100644 --- a/helpdesk/teams/views.py +++ b/helpdesk/teams/views.py @@ -54,7 +54,7 @@ def get_context_data(self, **kwargs: dict[str, Any]) -> dict[str, Any]: comment_form=CommentSubmitForm(), **kwargs, ) - + class TeamSubmitCommentFormView(LoginRequiredMixin, FormMixin, SingleObjectMixin, ProcessFormView): http_method_names = ['post', 'put'] @@ -86,10 +86,10 @@ class TeamDetailTicketsView(LoginRequiredMixin, SingleTableMixin, DetailView): def get_ticket_queryset(self) -> QuerySet[Ticket]: return self.get_object().tickets.with_event_fields() - + def get_ticket_filter(self) -> TicketFilter: queryset = self.get_ticket_queryset() - + return TicketFilter( data=self.request.GET or None, request=self.request, diff --git a/helpdesk/tickets/models.py b/helpdesk/tickets/models.py index aba2b8f..82b30b4 100644 --- a/helpdesk/tickets/models.py +++ b/helpdesk/tickets/models.py @@ -32,13 +32,13 @@ def __str__(self) -> str: if self.description: return f"{self.name} - {self.description}" return self.name - + def attention_count(self) -> int: tickets = self.tickets.with_event_fields().exclude( models.Q(status=TicketStatus.RESOLVED) | models.Q(assignee_id__isnull=False), ) return tickets.count() - + def in_progress_count(self) -> int: tickets = self.tickets.with_event_fields().filter( status=TicketStatus.OPEN, @@ -53,7 +53,7 @@ def with_status(self) -> TicketQuerySet: ticket_id=models.OuterRef("pk"), ).order_by("-created_at") return self.annotate(status=models.Subquery(events.values("new_status")[:1])) - + def with_assignee(self) -> TicketQuerySet: events = TicketEvent.objects.annotate( assignee=models.F("assignee_change__user"), @@ -62,13 +62,13 @@ def with_assignee(self) -> TicketQuerySet: assignee_change__isnull=False, ).order_by("-created_at") return self.annotate(assignee_id=models.Subquery(events.values("assignee")[:1])) - + def with_event_fields(self) -> TicketQuerySet: return self.with_assignee().with_status() class TicketManager(models.Manager): - + def get_queryset(self) -> TicketQuerySet: # type: ignore[override] return TicketQuerySet(self.model, using=self._db) @@ -104,7 +104,7 @@ def __str__(self) -> str: def get_absolute_url(self) -> str: return reverse_lazy('tickets:ticket_detail', kwargs={'pk': self.id}) - + @property def assignee(self) -> User | None: """ @@ -113,22 +113,22 @@ def assignee(self) -> User | None: Must be called only when the Ticket has with_event_fields() """ return get_object_or_none(User, pk=self.assignee_id) # type: ignore[attr-defined] - + @property def status_name(self) -> str: """ Get the name of the status. - + Must be called only when the Ticket has with_event_fields() """ lookups = {val: name for val, name in TicketStatus.choices} return lookups.get(self.status, "Unknown") # type: ignore[attr-defined] - + @property def status_css_tag(self) -> str: """ Get the CSS class of the status. - + Must be called only when the Ticket has with_event_fields() """ lookup = { @@ -140,21 +140,21 @@ def status_css_tag(self) -> str: @property def is_escalatable(self) -> bool: return self.queue.escalation_queue is not None - + @property def last_updated(self) -> datetime: return max(self.updated_at, self.created_at, *self.events.values_list("created_at", flat=True)) class TicketStatus(models.TextChoices): - OPEN = "OP", "Open" + OPEN = "OP", "Open" RESOLVED = "RS", "Resolved" class TicketEventAssigneeChange(models.Model): """ For a given ticket event, this model exists iff the assignee changed. - + We need a separate table for this so that we can differentiate between changing of an assignee and removal. diff --git a/helpdesk/tickets/views.py b/helpdesk/tickets/views.py index 27333a3..96da928 100644 --- a/helpdesk/tickets/views.py +++ b/helpdesk/tickets/views.py @@ -37,10 +37,10 @@ def get_ticket_queryset(self) -> QuerySet[Ticket]: if "status" not in self.request.GET: queryset = queryset.filter(status=TicketStatus.OPEN) return queryset - + def get_ticket_filter(self) -> TicketFilter: queryset = self.get_ticket_queryset() - + return TicketFilter( data=self.request.GET or None, request=self.request, @@ -76,7 +76,7 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context = super().get_context_data(**kwargs) context["ticket_queues"] = TicketQueue.objects.all() return context - + class TicketListView(LoginRequiredMixin, SingleTableMixin, FilterView): filterset_class = TicketFilter @@ -106,7 +106,7 @@ class TicketCreateView(LoginRequiredMixin, CreateView): def get_context_data(self, **kwargs: Any) -> dict[str, Any]: """The same template is used for updating tickets.""" return super().get_context_data(create=True, **kwargs) - + def get_initial(self) -> dict[str, Any]: return { "team": get_object_or_none(Team, tla=self.request.GET.get("team")), From 7af02ad30eb0eb8792c167216341841214ec0fd7 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:18:41 +0100 Subject: [PATCH 07/10] Code simplifications via ruff --- helpdesk/tickets/models.py | 2 +- helpdesk/tickets/tables.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helpdesk/tickets/models.py b/helpdesk/tickets/models.py index 82b30b4..c0736c4 100644 --- a/helpdesk/tickets/models.py +++ b/helpdesk/tickets/models.py @@ -121,7 +121,7 @@ def status_name(self) -> str: Must be called only when the Ticket has with_event_fields() """ - lookups = {val: name for val, name in TicketStatus.choices} + lookups = dict(TicketStatus.choices) return lookups.get(self.status, "Unknown") # type: ignore[attr-defined] @property diff --git a/helpdesk/tickets/tables.py b/helpdesk/tickets/tables.py index b22de1e..c34e6d7 100644 --- a/helpdesk/tickets/tables.py +++ b/helpdesk/tickets/tables.py @@ -26,5 +26,5 @@ class Meta: order_by = 'created_at' def render_status(self, value: str) -> str: - lookups = {val: name for val, name in TicketStatus.choices} + lookups = dict(TicketStatus.choices) return lookups.get(value, "Unknown") From 60483b64e1a275fcf4fedf4769d49066c58be642 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:19:27 +0100 Subject: [PATCH 08/10] Fix model member ordering according to ruff --- helpdesk/teams/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helpdesk/teams/models.py b/helpdesk/teams/models.py index 62e1e72..669fbdb 100644 --- a/helpdesk/teams/models.py +++ b/helpdesk/teams/models.py @@ -27,15 +27,15 @@ class Team(models.Model): is_rookie = models.BooleanField("Is Rookie") pit_location = models.ForeignKey(TeamPitLocation, on_delete=models.PROTECT) + class Meta: + ordering = ["tla"] + def __str__(self) -> str: return f"{self.tla} - {self.name}" def get_absolute_url(self) -> str: return reverse_lazy('teams:team_detail', args=[self.tla]) - class Meta: - ordering = ["tla"] - class TeamComment(models.Model): team = models.ForeignKey( From 68e467ebfb140248652845b401dd89c60c21f9cc Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:26:31 +0100 Subject: [PATCH 09/10] Ingore invalid module names ruff complains about It doesn't seem to be possible to ignore these on a per-file basis, or at least I couldn't find a spelling which worked. --- helpdesk/pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helpdesk/pyproject.toml b/helpdesk/pyproject.toml index 9e83300..85b3ead 100644 --- a/helpdesk/pyproject.toml +++ b/helpdesk/pyproject.toml @@ -46,6 +46,7 @@ ignore = [ "ANN401", # Dynamically typed expressions (typing.Any) are disallowed "B009", # Do not call `getattr` with a constant attribute value. "S101", # S101 Use of `assert` detected + "N999", # N999 Invalid module name ] -line-length = 120 \ No newline at end of file +line-length = 120 From 7c9db62be4afff9bb266027773c89039f192b916 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 16 Sep 2023 17:32:51 +0100 Subject: [PATCH 10/10] Placate mypy Not sure why it can't see these attributes; they're definitely present. --- helpdesk/helpdesk/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helpdesk/helpdesk/utils.py b/helpdesk/helpdesk/utils.py index 1c3dce8..75c8c21 100644 --- a/helpdesk/helpdesk/utils.py +++ b/helpdesk/helpdesk/utils.py @@ -8,6 +8,6 @@ def get_object_or_none(model: type[ModelT], **kwargs: Any) -> ModelT | None: try: - return model.objects.get(**kwargs) - except model.DoesNotExist: + return model.objects.get(**kwargs) # type: ignore[attr-defined] + except model.DoesNotExist: # type: ignore[attr-defined] return None