Skip to content

Commit

Permalink
Use django-stubs, pass mypy with it
Browse files Browse the repository at this point in the history
  • Loading branch information
inducer committed Sep 11, 2024
1 parent d5e5bef commit 8c4631d
Show file tree
Hide file tree
Showing 25 changed files with 297 additions and 178 deletions.
2 changes: 1 addition & 1 deletion .ci/run-safety.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# 67599: pip issue, utter nonsense
# 70612: Jinja2 SSTI, as of https://github.com/inducer/relate/pull/1053
# there is no longer a direct Jinja dependency, and no known path to SSTI.
poetry run safety check \
safety check \
-i 38678 \
-i 39253 \
-i 39535 \
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,14 @@ jobs:
run: |
poetry run ruff check
- uses: crate-ci/typos@master
- name: "Set up local settings"
run: cp local_settings_example.py local_settings.py
- name: "Mypy"
run: poetry run mypy relate course
run: poetry run ./.ci/run-mypy.sh
- name: "Safety"
run: bash ./.ci/run-safety.sh
run: poetry run ./.ci/run-safety.sh
- name: "Sphinx"
run: |
cp local_settings_example.py local_settings.py
(cd doc; poetry run make html SPHINXOPTS="-W --keep-going -n")
frontend:
Expand Down
4 changes: 3 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ ruff:
mypy:
<<: *quality
script: poetry run mypy relate course
variables:
RELATE_LOCAL_TEST_SETTINGS: './local_settings_example.py'
script: poetry run ./.ci/run-mypy.sh

safety:
<<: *quality
Expand Down
16 changes: 12 additions & 4 deletions accounts/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ def queryset(self, request, queryset):
class UserAdmin(UserAdminBase):
save_on_top = True

list_display = (*tuple(UserAdminBase.list_display),
# Fixing this type-ignore would require type.__getitem__ on Django types,
# which is only available via monkeypatching, ugh.
list_display = (
*UserAdminBase.list_display, # type: ignore[misc]
"name_verified", "status", "institutional_id", "institutional_id_verified")
list_editable = ("first_name", "last_name",
"name_verified",
Expand All @@ -77,8 +80,12 @@ class UserAdmin(UserAdminBase):
"status", CourseListFilter) # type: ignore
search_fields = (*tuple(UserAdminBase.search_fields), "institutional_id")

fieldsets = UserAdminBase.fieldsets[:1] + (
(UserAdminBase.fieldsets[1][0], {"fields": (
_fsets = UserAdminBase.fieldsets
assert _fsets is not None

fieldsets = (
*_fsets[:1],
(_fsets[1][0], {"fields": (
"status",
"first_name",
"last_name",
Expand All @@ -88,7 +95,8 @@ class UserAdmin(UserAdminBase):
"institutional_id_verified",
"editor_mode",)
}),
) + UserAdminBase.fieldsets[2:]
*_fsets[2:],
)
ordering = ["-date_joined"]

def get_fieldsets(self, request, obj=None):
Expand Down
22 changes: 12 additions & 10 deletions accounts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from django.contrib.auth.validators import ASCIIUsernameValidator
from django.db import models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _, pgettext_lazy
from django.utils.translation import gettext, gettext_lazy as _, pgettext_lazy

from course.constants import USER_STATUS_CHOICES

Expand Down Expand Up @@ -130,20 +130,22 @@ class Meta:
verbose_name = _("user")
verbose_name_plural = _("users")

def get_full_name(self, allow_blank=True, force_verbose_blank=False):
def get_full_name(
self, allow_blank=True, force_verbose_blank=False
) -> str | None:
if (not allow_blank
and (not self.first_name or not self.last_name)):
return None

def verbose_blank(s):
def verbose_blank(s: str) -> str:
if force_verbose_blank:
if not s:
return _("(blank)")
return gettext("(blank)")
else:
return s
return s

def default_fullname(first_name, last_name):
def default_fullname(first_name: str, last_name: str) -> str:
"""
Returns the first_name plus the last_name, with a space in
between.
Expand All @@ -164,7 +166,7 @@ def default_fullname(first_name, last_name):

return full_name.strip()

def get_masked_profile(self):
def get_masked_profile(self) -> str:
"""
Returns the masked user profile.
"""
Expand All @@ -188,11 +190,11 @@ def default_mask_method(user):
"an empty string.")
return result

def get_short_name(self):
def get_short_name(self) -> str:
"""Returns the short name for the user."""
return self.first_name

def get_email_appellation(self):
def get_email_appellation(self) -> str:
"""Return the appellation of the receiver in email."""

from accounts.utils import relate_user_method_settings
Expand All @@ -210,9 +212,9 @@ def get_email_appellation(self):

return appellation

return _("user")
return gettext("user")

def clean(self):
def clean(self) -> None:
super().clean()

# email can be None in Django admin when create new user
Expand Down
23 changes: 16 additions & 7 deletions course/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
Participation,
ParticipationRole,
)
from course.utils import course_view, render_course_page
from course.utils import CoursePageContext, course_view, render_course_page
from relate.utils import (
HTML5DateTimeInput,
StyledForm,
Expand Down Expand Up @@ -1191,21 +1191,30 @@ def find_matching_token(
token_hash_str: str | None = None,
now_datetime: datetime.datetime | None = None
) -> AuthenticationToken | None:
if token_id is None:
return None

try:
token = AuthenticationToken.objects.get(
id=token_id,
participation__course__identifier=course_identifier)
except AuthenticationToken.DoesNotExist:
return None

if token.token_hash is None:
return None

from django.contrib.auth.hashers import check_password
if not check_password(token_hash_str, token.token_hash):
return None

if token.revocation_time is not None:
return None
if token.valid_until is not None and now_datetime > token.valid_until:
return None
if token.valid_until is not None:
if now_datetime is None:
return None
if now_datetime > token.valid_until:
return None

return token

Expand Down Expand Up @@ -1400,7 +1409,7 @@ def __init__(
pperm.impersonate_role, prole.identifier)}
)

self.fields["restrict_to_participation_role"].queryset = (
self.fields["restrict_to_participation_role"].queryset = ( # type:ignore[attr-defined]
ParticipationRole.objects.filter(
id__in=list(allowable_role_ids)
))
Expand All @@ -1409,15 +1418,15 @@ def __init__(


@course_view
def manage_authentication_tokens(pctx: http.HttpRequest) -> http.HttpResponse:

def manage_authentication_tokens(pctx: CoursePageContext) -> http.HttpResponse:
request = pctx.request

if not request.user.is_authenticated:
raise PermissionDenied()

if not pctx.has_permission(pperm.view_analytics):
raise PermissionDenied()
assert pctx.participation is not None

from course.views import get_now_or_fake_time
now_datetime = get_now_or_fake_time(request)
Expand Down Expand Up @@ -1446,7 +1455,7 @@ def manage_authentication_tokens(pctx: http.HttpRequest) -> http.HttpResponse:

from django.contrib.auth.hashers import make_password
auth_token = AuthenticationToken(
user=pctx.request.user,
user=request.user,
participation=pctx.participation,
restrict_to_participation_role=form.cleaned_data[
"restrict_to_participation_role"],
Expand Down
15 changes: 6 additions & 9 deletions course/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

# {{{ mypy

from collections.abc import Callable
from collections.abc import Callable, Collection
from typing import (
TYPE_CHECKING,
Any,
Expand Down Expand Up @@ -1736,7 +1736,7 @@ def compute_chunk_weight_and_shown(
chunk: ChunkDesc,
roles: list[str],
now_datetime: datetime.datetime,
facilities: frozenset[str],
facilities: Collection[str],
) -> tuple[float, bool]:
if not hasattr(chunk, "rules"):
return 0, True
Expand Down Expand Up @@ -1794,7 +1794,7 @@ def get_processed_page_chunks(
page_desc: StaticPageDesc,
roles: list[str],
now_datetime: datetime.datetime,
facilities: frozenset[str],
facilities: Collection[str],
) -> list[ChunkDesc]:
for chunk in page_desc.chunks:
chunk.weight, chunk.shown = \
Expand Down Expand Up @@ -2006,15 +2006,12 @@ def is_commit_sha_valid(repo: Repo_ish, commit_sha: str) -> bool:
preview_sha = participation.preview_git_commit_sha

if repo is not None:
commit_sha_valid = is_commit_sha_valid(repo, preview_sha)
preview_sha_valid = is_commit_sha_valid(repo, preview_sha)
else:
with get_course_repo(course) as repo:
commit_sha_valid = is_commit_sha_valid(repo, preview_sha)
preview_sha_valid = is_commit_sha_valid(repo, preview_sha)

if not commit_sha_valid:
preview_sha = None

if preview_sha is not None:
if preview_sha_valid:
sha = preview_sha

return sha.encode()
Expand Down
Loading

0 comments on commit 8c4631d

Please sign in to comment.