Fixes from flake8-simplify and flake8-annotations; upgrade deps (#2033)

.pre-commit-config.yaml

@@ -56,7 +56,7 @@ repos:
       - id: pyupgrade
         args: ['--keep-runtime-typing', '--py311-plus']
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.4.1
+    rev: v0.4.2
     hooks:
       - id: ruff
         args: ['--fix', '--exit-non-zero-on-fix']
@@ -79,7 +79,7 @@ repos:
       - id: yesqa
         additional_dependencies: &flake8deps
           - bandit
-          # - flake8-annotations
+          - flake8-annotations
           - flake8-assertive
           - flake8-blind-except
           - flake8-bugbear
@@ -92,6 +92,7 @@ repos:
           - flake8-plugin-utils
           - flake8-print
           - flake8-pytest-style
+          - flake8-simplify
           - pep8-naming
           - toml
           - tomli
@@ -102,7 +103,7 @@ repos:
         additional_dependencies:
           - tomli
   - repo: https://github.com/psf/black
-    rev: 24.4.1
+    rev: 24.4.2
     hooks:
       - id: black
   - repo: https://github.com/PyCQA/flake8

funnel/extapi/boxoffice.py

@@ -23,7 +23,7 @@ def __init__(self, access_token: str, base_url: str | None = None) -> None:
         else:
             self.base_url = base_url
 
-    def get_orders(self, ic: str):  # TODO: Return type annotation
+    def get_orders(self, ic: str) -> list[dict]:  # TODO: Return type annotation
         url = urljoin(
             self.base_url,
             f'ic/{ic}/orders?access_token={self.access_token}',
@@ -33,25 +33,25 @@ def get_orders(self, ic: str):  # TODO: Return type annotation
     def get_tickets(self, ic: str) -> list[ExtTicketsDict]:
         tickets: list[ExtTicketsDict] = []
         for order in self.get_orders(ic):
-            for line_item in order.get('line_items'):
-                if line_item.get('assignee'):
+            for line_item in order.get('line_items', []):
+                if assignee := line_item.get('assignee', {}):
                     status = line_item.get('line_item_status')
                     tickets.append(
                         {
-                            'fullname': line_item.get('assignee').get('fullname', ''),
-                            'email': line_item.get('assignee').get('email'),
-                            'phone': line_item.get('assignee').get('phone', ''),
+                            'fullname': assignee.get('fullname', ''),
+                            'email': assignee.get('email'),
+                            'phone': assignee.get('phone', ''),
                             'twitter': extract_twitter_handle(
-                                line_item.get('assignee').get('twitter', '')
+                                assignee.get('twitter', '')
                             ),
-                            'company': line_item.get('assignee').get('company'),
-                            'city': line_item.get('assignee').get('city', ''),
-                            'job_title': line_item.get('assignee').get('jobtitle', ''),
-                            'ticket_no': str(line_item.get('line_item_seq')),
+                            'company': assignee.get('company'),
+                            'city': assignee.get('city', ''),
+                            'job_title': assignee.get('jobtitle', ''),
+                            'ticket_no': str(line_item.get('line_item_seq', '')),
                             'ticket_type': line_item.get('ticket', {}).get('title', '')[
                                 :80
                             ],
-                            'order_no': str(order.get('invoice_no')),
+                            'order_no': str(order.get('invoice_no', '')),
                             'status': status,
                         }
                     )

funnel/forms/account.py

@@ -151,7 +151,7 @@ def pwned_password_validator(_form: Any, field: forms.PasswordField) -> None:
         return
 
     # If we have data, check for our hash suffix in the returned range of matches
-    count = matches.get(suffix, None)
+    count = matches.get(suffix)
     if count:  # not 0 and not None
         raise forms.validators.StopValidation(
             ngettext(

funnel/forms/helpers.py

@@ -145,17 +145,20 @@ def __call__(self, form: forms.Form, field: forms.Field) -> None:
         if has_error is not None:
             app.logger.error("Unknown email address validation code: %r", has_error)
 
-        if has_error is None and self.purpose == 'register':
-            # One last check: is there an existing claim? If so, stop the user from
-            # making a dupe account
-            if AccountEmailClaim.all(email=field.data).notempty():
-                raise forms.validators.StopValidation(
-                    _(
-                        "You or someone else has made an account with this email"
-                        " address but has not confirmed it. Do you need to reset your"
-                        " password?"
-                    )
+        # One last check: is there an existing claim? If so, stop the user from making a
+        # dupe account
+        if (
+            has_error is None
+            and self.purpose == 'register'
+            and AccountEmailClaim.all(email=field.data).notempty()
+        ):
+            raise forms.validators.StopValidation(
+                _(
+                    "You or someone else has made an account with this email"
+                    " address but has not confirmed it. Do you need to reset your"
+                    " password?"
                 )
+            )
 
 
 class PhoneNumberAvailable:

funnel/loginproviders/github.py

@@ -45,7 +45,7 @@ def callback(self) -> LoginProviderData:
                     _("This server’s callback URL is misconfigured")
                 )
             raise LoginCallbackError(_("Unknown failure"))
-        code = request.args.get('code', None)
+        code = request.args.get('code')
         try:
             response = requests.post(
                 self.token_url,

funnel/loginproviders/google.py

@@ -40,7 +40,7 @@ def callback(self) -> LoginProviderData:
             if request.args['error'] == 'access_denied':
                 raise LoginCallbackError(_("You denied the Google login request"))
             raise LoginCallbackError(_("Unknown failure"))
-        code = request.args.get('code', None)
+        code = request.args.get('code')
         try:
             credentials = self.flow(callback_url).step2_exchange(code)
             response = requests.get(

funnel/loginproviders/linkedin.py

@@ -61,7 +61,7 @@ def callback(self) -> LoginProviderData:
                     _("This server’s callback URL is misconfigured")
                 )
             raise LoginCallbackError(_("Unknown failure"))
-        code = request.args.get('code', None)
+        code = request.args.get('code')
         try:
             response = requests.post(
                 self.token_url,

funnel/loginproviders/zoom.py

@@ -49,7 +49,7 @@ def callback(self) -> LoginProviderData:
                     _("This server’s callback URL is misconfigured")
                 )
             raise LoginCallbackError(_("Unknown failure"))
-        code = request.args.get('code', None)
+        code = request.args.get('code')
         try:
             response = requests.post(
                 self.token_url,

funnel/models/__init__.py

@@ -40,6 +40,7 @@
     "BaseScopedIdMixin",
     "BaseScopedIdNameMixin",
     "BaseScopedNameMixin",
+    "CheckinParticipantProtocol",
     "Comment",
     "CommentModeratorReport",
     "CommentReplyNotification",

funnel/models/__init__.pyi

@@ -215,6 +215,7 @@ from .shortlink import Shortlink
 from .site_membership import SiteMembership
 from .sponsor_membership import ProjectSponsorMembership, ProposalSponsorMembership
 from .sync_ticket import (
+    CheckinParticipantProtocol,
     SyncTicket,
     TicketClient,
     TicketEvent,
@@ -267,6 +268,7 @@ __all__ = [
     "BaseScopedIdMixin",
     "BaseScopedIdNameMixin",
     "BaseScopedNameMixin",
+    "CheckinParticipantProtocol",
     "Comment",
     "CommentModeratorReport",
     "CommentReplyNotification",

funnel/models/email_address.py

@@ -672,12 +672,11 @@ def validate_for(
             return 'taken'
 
         # There is an existing but it's available for this owner. Any other concerns?
-        if new:
-            # Caller is asking to confirm this is not already belonging to this owner
-            if existing.is_exclusive():
-                # It's in an exclusive relationship, and we're already determined it's
-                # available to this owner, so it must be exclusive to them
-                return 'not_new'
+        if new and existing.is_exclusive():
+            # Caller is asking to confirm this is not already belonging to this owner:
+            # It's in an exclusive relationship, and we're already determined it's
+            # available to this owner, so it must be exclusive to them
+            return 'not_new'
         if existing.delivery_state.SOFT_FAIL:
             return 'soft_fail'
         if existing.delivery_state.HARD_FAIL:
@@ -850,13 +849,13 @@ def _validate_email(
     if old_value == value:
         # Old value is new value. Do nothing. Return without validating
         return
-    if old_value is NO_VALUE and inspect(target).has_identity is False:
+    if old_value is NO_VALUE and inspect(target).has_identity is False:  # noqa: SIM114
         # Old value is unknown and target is a transient object. Continue
         pass
-    elif value is None:
+    elif value is None:  # noqa: SIM114
         # Caller is trying to unset email. Allow this
         pass
-    elif old_value is None:
+    elif old_value is None:  # noqa: SIM114
         # Caller is trying to restore email. Allow but validate match for existing hash
         pass
     elif (
@@ -916,12 +915,11 @@ def _email_address_mixin_set_validator(
     old_value: EmailAddress | None,
     _initiator: Any,
 ) -> None:
-    if value != old_value and target.__email_for__:
-        if value is not None:
-            if value.is_blocked:
-                raise EmailAddressBlockedError("This email address has been blocked")
-            if not value.is_available_for(getattr(target, target.__email_for__)):
-                raise EmailAddressInUseError("This email address it not available")
+    if value != old_value and target.__email_for__ and value is not None:
+        if value.is_blocked:
+            raise EmailAddressBlockedError("This email address has been blocked")
+        if not value.is_available_for(getattr(target, target.__email_for__)):
+            raise EmailAddressInUseError("This email address it not available")
 
 
 @event.listens_for(OptionalEmailAddressMixin, 'mapper_configured', propagate=True)

funnel/models/notification.py

@@ -83,6 +83,7 @@
 from __future__ import annotations
 
 from collections.abc import Callable, Generator, Sequence
+from contextlib import suppress
 from dataclasses import dataclass
 from datetime import datetime
 from enum import ReprEnum
@@ -794,10 +795,8 @@ def is_not_deleted(self, revoke: bool = False) -> bool:
         :param bool revoke: Mark the notification as revoked if document or fragment
             is missing
         """
-        try:
+        with suppress(NoResultFound):
             return bool(self.fragment and self.document or self.document)
-        except NoResultFound:
-            pass
         if revoke:
             self.is_revoked = True
             # Do not set `self.rollupid` because this is not a rollup

funnel/models/phone_number.py

@@ -4,6 +4,7 @@
 
 import hashlib
 import warnings
+from contextlib import suppress
 from datetime import datetime
 from typing import TYPE_CHECKING, Any, ClassVar, Literal, Self, overload
 
@@ -159,7 +160,7 @@ def parse_phone_number(
     # with the _last_ valid candidate (as it's coupled with a
     # :class:`~funnel.models.account.AccountPhone` lookup)
     sms_invalid = False
-    try:
+    with suppress(phonenumbers.NumberParseException):
         for region in PHONE_LOOKUP_REGIONS:
             parsed_number = phonenumbers.parse(candidate, region)
             if phonenumbers.is_valid_number(parsed_number):
@@ -174,8 +175,6 @@ def parse_phone_number(
                 return phonenumbers.format_number(
                     parsed_number, phonenumbers.PhoneNumberFormat.E164
                 )
-    except phonenumbers.NumberParseException:
-        pass
     # We found a number that is valid, but the caller wanted it to be valid for SMS and
     # it isn't, so return a special flag
     if sms_invalid:
@@ -855,11 +854,9 @@ def transport_hash(self) -> str: ...
 def _clear_cached_properties(target: PhoneNumber) -> None:
     """Clear cached properties in :class:`PhoneNumber`."""
     for attr in ('parsed', 'formatted'):
-        try:
-            delattr(target, attr)
-        except KeyError:
+        with suppress(KeyError):
             # cached_property raises KeyError when there's no existing cached value
-            pass
+            delattr(target, attr)
 
 
 @event.listens_for(PhoneNumber.number, 'set', retval=True)
@@ -873,10 +870,10 @@ def _validate_number(
     if old_value == value:
         # Old value is new value. Do nothing. Return without validating
         return value
-    if old_value is NO_VALUE and inspect(target).has_identity is False:
+    if old_value is NO_VALUE and inspect(target).has_identity is False:  # noqa: SIM114
         # Old value is unknown and target is a transient object. Continue
         pass
-    elif value is None:
+    elif value is None:  # noqa: SIM114
         # Caller is trying to unset phone. Allow this
         pass
     elif old_value is None:

funnel/models/project.py

@@ -798,10 +798,13 @@ def withdraw(self) -> None:
     @property
     def title_inline(self) -> str:
         """Suffix a colon if the title does not end in ASCII sentence punctuation."""
-        if self.title and self.tagline:
-            # pylint: disable=unsubscriptable-object
-            if self.title[-1] not in ('?', '!', ':', ';', '.', ','):
-                return self.title + ':'
+        # pylint: disable=unsubscriptable-object
+        if (
+            self.title
+            and self.tagline
+            and self.title[-1] not in ('?', '!', ':', ';', '.', ',')
+        ):
+            return self.title + ':'
         return self.title
 
     with_roles(title_inline, read={'all'}, datasets={'primary', 'without_parent'})
@@ -1400,14 +1403,14 @@ def calendar_weeks(self, leading_weeks: bool = True) -> dict[str, Any]:
                         session_dates_dict[date]['day_start_at']
                         .astimezone(self.timezone)
                         .strftime('%I:%M %p')
-                        if date in session_dates_dict.keys()
+                        if date in session_dates_dict
                         else None
                     ),
                     'day_end_at': (
                         session_dates_dict[date]['day_end_at']
                         .astimezone(self.timezone)
                         .strftime('%I:%M %p %Z')
-                        if date in session_dates_dict.keys()
+                        if date in session_dates_dict
                         else None
                     ),
                 }

funnel/models/sync_ticket.py

@@ -5,7 +5,8 @@
 import base64
 import os
 from collections.abc import Iterable
-from typing import TYPE_CHECKING, Any, Self
+from typing import TYPE_CHECKING, Any, Protocol, Self
+from uuid import UUID
 
 from coaster.sqlalchemy import with_roles
 
@@ -27,6 +28,7 @@
 from .project_membership import project_child_role_map
 
 __all__ = [
+    'CheckinParticipantProtocol',
     'SyncTicket',
     'TicketClient',
     'TicketEvent',
@@ -69,6 +71,17 @@ def make_private_key() -> str:
 )
 
 
+class CheckinParticipantProtocol(Protocol):
+    uuid: UUID
+    fullname: str
+    company: str
+    email: str | None
+    badge_printed: bool
+    checked_in: bool
+    ticket_type_titles: str  # Comma separated string
+    has_user: bool
+
+
 class GetTitleMixin(BaseScopedNameMixin):
     @classmethod
     def get(
@@ -361,7 +374,9 @@ def remove_events(self, ticket_events: Iterable[TicketEvent]) -> None:
                 self.ticket_events.remove(ticket_event)
 
     @classmethod
-    def checkin_list(cls, ticket_event: TicketEvent) -> list:  # TODO: List type?
+    def checkin_list(
+        cls, ticket_event: TicketEvent
+    ) -> list[CheckinParticipantProtocol]:
         """
         Return ticket participant details as a comma separated string.