More typing and mypy warnings, non-null user email

.github/workflows/test.yml

@@ -41,3 +41,5 @@ jobs:
         run: docker compose -f ./docker-compose.yml -f ./docker-compose.ci.yml build --parallel
       - name: Test
         run: docker compose -f ./docker-compose.yml -f ./docker-compose.ci.yml run --quiet-pull -e GITHUB_ACTIONS=true -T app uv run make pytest
+      - name: Adding report
+        run: cat cov.md >> $GITHUB_STEP_SUMMARY

Makefile

@@ -11,7 +11,7 @@ endif
 test: check-syntax pytest
 
 pytest:
-	SETTINGS_FILE=$(TEST_SETTINGS) pytest --random-order --cov=apps --cov=models ./tests/ ./models/
+	SETTINGS_FILE=$(TEST_SETTINGS) pytest --random-order --cov-report markdown:cov.md --cov=apps --cov=models ./tests/ ./models/
 	SETTINGS_FILE=$(TEST_SETTINGS) flask db upgrade
 	SETTINGS_FILE=$(TEST_SETTINGS) flask db check
 

apps/admin/admin_message.py

@@ -7,8 +7,8 @@
 from main import db
 from models.admin_message import AdminMessage
 
+from ..common.forms import Form
 from . import admin
-from .forms import Form
 
 
 class AdminMessageForm(Form):

apps/admin/arrivals.py

@@ -134,7 +134,7 @@ def arrivals_view_add(view_id, group_id=None, product_id=None):
 
         elif form.add_all_products_recursive.data:
 
-            def _fetch_all_products(group) -> list[dict[str, int]]:
+            def _fetch_all_products(group: ProductGroup) -> list[dict[str, int]]:
                 out = []
                 for product in group.products:
                     out.append({"view_id": view.id, "product_id": product.id})

apps/admin/email.py

@@ -1,11 +1,17 @@
+from collections.abc import Sequence
+from typing import Literal
+
 from flask import flash, redirect, render_template, url_for
+from sqlalchemy import select
 from wtforms import SelectField, StringField, SubmitField
 from wtforms.validators import DataRequired
 from wtforms.widgets import TextArea
 
+from main import db
 from models import event_year
 from models.cfp import Proposal
 from models.payment import Payment
+from models.purchase import Purchase
 from models.user import User
 from models.village import VillageMember
 
@@ -37,24 +43,24 @@ class EmailComposeForm(Form):
     send = SubmitField("Send Email")
 
 
-def get_users(dest: str) -> list[User]:
-    query = User.query
+def get_users(dest: Literal["ticket", "cfp", "purchasers", "villages"]) -> Sequence[User]:
+    query = select(User)
     if dest == "ticket":
         query = (
             query.join(User.owned_purchases)
-            .filter_by(type="admission_ticket", is_paid_for=True)
+            .where(Purchase.type == "admission_ticket", Purchase.is_paid_for == True)
             .group_by(User.id)
         )
     elif dest == "purchasers":
-        query = query.join(User.payments).filter(Payment.state == "paid")
+        query = query.join(User.payments).where(Payment.state == "paid")
     elif dest == "cfp":
-        query = query.join(User.proposals).filter(Proposal.is_accepted)
+        query = query.join(User.proposals).where(Proposal.is_accepted)
     elif dest == "villages":
-        query = query.join(User.village_membership).filter(VillageMember.admin)
+        query = query.join(User.village_membership).where(VillageMember.admin == True)
     else:
         raise ValueError(f"Invalid email destination set: {dest}")
 
-    return query.distinct().all()
+    return db.session.execute(query.distinct()).scalars().all()
 
 
 def get_email_reason(dest: str) -> str:

apps/admin/payments.py

@@ -84,7 +84,7 @@ class ResetExpiryForm(Form):
 
 
 @admin.route("/payment/<int:payment_id>/reset-expiry", methods=["GET", "POST"])
-def reset_expiry(payment_id) -> ResponseReturnValue:
+def reset_expiry(payment_id: int) -> ResponseReturnValue:
     payment = get_or_404(db, BankPayment, payment_id)
 
     form = ResetExpiryForm()

apps/api/installations.py

@@ -1,3 +1,5 @@
+from typing import Any, TypedDict
+
 from flask import url_for
 from flask_restful import Resource
 from geoalchemy2.shape import to_shape
@@ -8,7 +10,15 @@
 from . import api
 
 
-def render_installation(installation: InstallationProposal):
+class InstallationResponse(TypedDict):
+    id: int
+    name: str
+    url: str
+    description: str | None
+    location: dict[str, Any] | None
+
+
+def render_installation(installation: InstallationProposal) -> InstallationResponse:
     return {
         "id": installation.id,
         "name": installation.display_title,

apps/api/villages.py

@@ -1,3 +1,5 @@
+from typing import Any, TypedDict
+
 from flask import abort, request
 from flask_login import current_user
 from flask_restful import Resource
@@ -12,13 +14,21 @@
 from . import api
 
 
-def render_village(village: Village):
+class VillageResponse(TypedDict):
+    id: int
+    name: str
+    url: str | None
+    description: str | None
+    location: dict[str, Any] | None
+
+
+def render_village(village: Village) -> VillageResponse:
     return {
         "id": village.id,
         "name": village.name,
         "url": village.url,
         "description": village.description,
-        "location": (to_shape(village.location).__geo_interface__ if village.location else None),
+        "location": to_shape(village.location).__geo_interface__ if village.location else None,
     }
 
 

apps/base/about.py

@@ -12,24 +12,25 @@
     render_template,
     url_for,
 )
+from flask.typing import ResponseValue
 
 from apps.common import render_markdown
 
 from . import base
 
 
 @base.route("/about/branding")
-def branding():
+def branding() -> ResponseValue:
     return render_template("about/branding.html")
 
 
 @base.route("/about/<page_name>")
-def page(page_name: str):
+def page(page_name: str) -> ResponseValue:
     return render_markdown(f"about/{page_name}", page_name=page_name)
 
 
 @base.route("/about/diversity/<int:year>")
-def yearly_diversity_stats(year: int):
+def yearly_diversity_stats(year: int) -> ResponseValue:
     if year in (2018, 2022):
         with open(f"exports/{year}/public/UserDiversity.json") as raw_data:
             data = json.load(raw_data)
@@ -45,20 +46,20 @@ def yearly_diversity_stats(year: int):
 # About and Contact have actual logic in them, so remain as HTML rather than
 # markdown
 @base.route("/about")
-def about():
+def about() -> ResponseValue:
     return render_template("about/index.html")
 
 
 @base.route("/company")
-def company():
+def company() -> ResponseValue:
     return render_markdown("about/company")
 
 
 @base.route("/about/contact")
-def contact():
+def contact() -> ResponseValue:
     return render_template("about/contact.html")
 
 
 @base.route("/about/covid")
-def covid():
+def covid() -> ResponseValue:
     return redirect(url_for(".page", page_name="health"), code=301)

apps/base/arcade.py

@@ -3,6 +3,7 @@
 """
 
 from flask import redirect, render_template, url_for
+from flask.typing import ResponseValue
 
 from apps.common import render_markdown
 from models import event_year
@@ -11,17 +12,17 @@
 
 
 @base.route("/arcade")
-def yearly_arcade_redirection():
+def yearly_arcade_redirection() -> ResponseValue:
     return redirect(url_for(".arcade", year=event_year()))
 
 
 @base.route("/arcade/<int:year>/<page_name>")
-def arcade_page(year: int, page_name: str):
+def arcade_page(year: int, page_name: str) -> ResponseValue:
     return render_markdown(
         f"arcade/{year}/{page_name}", template=f"arcade/{year}/template.html", page_name=page_name
     )
 
 
 @base.route("/arcade/<int:year>")
-def arcade(year: int):
+def arcade(year: int) -> ResponseValue:
     return render_template(f"arcade/{year}/main.html")

apps/base/dev/fake.py

@@ -5,13 +5,6 @@
 from sqlalchemy import select
 
 from apps.cfp.scheduler import Scheduler
-from apps.common.forms import (
-    AGE_CHOICES,
-    DISABILITY_CHOICES,
-    ETHNICITY_CHOICES,
-    GENDER_CHOICES,
-    SEXUALITY_CHOICES,
-)
 from main import db
 from models import Currency
 from models.basket import Basket
@@ -21,11 +14,19 @@
     InstallationProposal,
     LightningTalkProposal,
     PerformanceProposal,
+    Proposal,
     TalkProposal,
     WorkshopProposal,
     YouthWorkshopProposal,
 )
-from models.diversity import UserDiversity
+from models.diversity import (
+    AGE_CHOICES,
+    DISABILITY_CHOICES,
+    ETHNICITY_CHOICES,
+    GENDER_CHOICES,
+    SEXUALITY_CHOICES,
+    UserDiversity,
+)
 from models.event_tickets import EventTicket, get_max_rank_for_user
 from models.payment import BankPayment, StripePayment
 from models.product import PriceTier
@@ -232,7 +233,7 @@ def create_volunteer_data(self, user):
         user.grant_permission("volunteer:user")
         db.session.add(vol)
 
-    def create_fake_tickets(self, user) -> None:
+    def create_fake_tickets(self, user: User) -> None:
         currency = Currency.EUR if random.random() < 0.2 else Currency.GBP
 
         # In this case we're going to use the same payment method if we make multiple payments.
@@ -274,9 +275,9 @@ def create_fake_tickets(self, user) -> None:
 
         db.session.commit()
 
-    def create_fake_lottery_tickets(self, users_list: list[User], proposal):
+    def create_fake_lottery_tickets(self, users_list: list[User], proposal: Proposal) -> None:
         n_lottery_tickets = random.randint(0, len(users_list))
-        max_ticket_count = 2 if proposal is WorkshopProposal else 5
+        max_ticket_count = 2 if isinstance(proposal, WorkshopProposal) else 5
 
         for user in random.sample(users_list, k=n_lottery_tickets):
             rank = get_max_rank_for_user(user, proposal.type)

apps/base/installations.py

@@ -3,6 +3,7 @@
 """
 
 from flask import redirect, render_template, url_for
+from flask.typing import ResponseValue
 
 from apps.common import render_markdown
 from models import event_year
@@ -11,12 +12,12 @@
 
 
 @base.route("/installations")
-def yearly_installation_redirection():
+def yearly_installation_redirection() -> ResponseValue:
     return redirect(url_for(".installations", year=event_year()))
 
 
 @base.route("/installations/<int:year>/<page_name>")
-def installations_page(year: int, page_name: str):
+def installations_page(year: int, page_name: str) -> ResponseValue:
     return render_markdown(
         f"installations/{year}/{page_name}",
         template=f"installations/{year}/template.html",
@@ -25,5 +26,5 @@ def installations_page(year: int, page_name: str):
 
 
 @base.route("/installations/<int:year>")
-def installations(year: int):
+def installations(year: int) -> ResponseValue:
     return render_template(f"installations/{year}/main.html")

apps/base/tasks_export.py

@@ -1,6 +1,7 @@
 import json
 import os
-from typing import cast
+from collections.abc import Iterable
+from typing import Any, cast
 
 import click
 from flask import current_app as app
@@ -14,7 +15,7 @@
 from . import base
 
 
-def get_export_data(table_filter: str | None = None):
+def get_export_data(table_filter: str | None = None) -> Iterable[tuple[str, Any]]:
     """Export data to archive using the `get_export_data` method in the model class."""
     # As we go, we check against the list of all tables, in case we forget about some
     # new object type (e.g. association table).

apps/cfp/schedule_tasks.py

@@ -26,10 +26,7 @@ class VenueDefinition:
 
     @property
     def location(self) -> str:
-        if self.latlon:
-            return f"POINT({self.latlon[1]} {self.latlon[0]})"
-        else:
-            return None
+        return f"POINT({self.latlon[1]} {self.latlon[0]})"
 
     def as_venue(self) -> Venue:
         return Venue(

apps/cfp/views.py

@@ -584,7 +584,7 @@ class FinaliseForm(Form):
             ("mon am", "Monday am"),
         ],
     )
-    _available_slots: tuple = tuple()
+    _available_slots: list[str]
 
     def get_availability_json(self):
         res = []

apps/cfp_review/base.py

@@ -71,7 +71,7 @@
     copy_request_args,
 )
 from ..common.email import from_email
-from ..common.forms import guess_age, guess_gender, guess_ethnicity
+from models.diversity import guess_age, guess_gender, guess_ethnicity
 
 
 @cfp_review.route("/")