From d8346491300664aa8fe3e12b347ad7e21bc7bf6e Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Fri, 15 Dec 2023 01:00:20 +0530 Subject: [PATCH] Make OptionalEmailAddressMixin and EmailAddressMixin distinct; remove more backrefs --- funnel/models/account.py | 11 ++++-- funnel/models/contact_exchange.py | 22 ++++-------- funnel/models/email_address.py | 46 +++++++++++++++++++++---- funnel/models/sync_ticket.py | 12 ++++--- tests/unit/models/email_address_test.py | 9 +++-- 5 files changed, 67 insertions(+), 33 deletions(-) diff --git a/funnel/models/account.py b/funnel/models/account.py index 1c17d4c55..075ff0e5c 100644 --- a/funnel/models/account.py +++ b/funnel/models/account.py @@ -381,6 +381,14 @@ class Account(UuidMixin, BaseMixin, Model): 'active_commentset_memberships', 'commentset' ) + # contact_exchange.py + scanned_contacts: DynamicMapped[ContactExchange] = relationship( + lazy='dynamic', + order_by='ContactExchange.scanned_at.desc()', + passive_deletes=True, + back_populates='account', + ) + __table_args__ = ( sa.Index( 'ix_account_name_lower', @@ -1541,7 +1549,6 @@ class AccountEmail(EmailAddressMixin, BaseMixin, Model): """An email address linked to an account.""" __tablename__ = 'account_email' - __email_optional__ = False __email_unique__ = True __email_is_exclusive__ = True __email_for__ = 'account' @@ -1720,7 +1727,6 @@ class AccountEmailClaim(EmailAddressMixin, BaseMixin, Model): """Claimed but unverified email address for a user.""" __tablename__ = 'account_email_claim' - __email_optional__ = False __email_unique__ = False __email_for__ = 'account' __email_is_exclusive__ = False @@ -2234,3 +2240,4 @@ def get( ) from .comment import Comment, Commentset # noqa: F401 from .commentset_membership import CommentsetMembership + from .contact_exchange import ContactExchange diff --git a/funnel/models/contact_exchange.py b/funnel/models/contact_exchange.py index 64fd50858..d9398bc4a 100644 --- a/funnel/models/contact_exchange.py +++ b/funnel/models/contact_exchange.py @@ -21,7 +21,6 @@ Query, RoleMixin, TimestampMixin, - backref, db, relationship, sa, @@ -63,15 +62,7 @@ class ContactExchange(TimestampMixin, RoleMixin, Model): account_id: Mapped[int] = sa_orm.mapped_column( sa.ForeignKey('account.id', ondelete='CASCADE'), primary_key=True ) - account: Mapped[Account] = relationship( - Account, - backref=backref( - 'scanned_contacts', - lazy='dynamic', - order_by='ContactExchange.scanned_at.desc()', - passive_deletes=True, - ), - ) + account: Mapped[Account] = relationship(back_populates='scanned_contacts') #: Participant whose contact was scanned ticket_participant_id: Mapped[int] = sa_orm.mapped_column( sa.Integer, @@ -80,8 +71,7 @@ class ContactExchange(TimestampMixin, RoleMixin, Model): index=True, ) ticket_participant: Mapped[TicketParticipant] = relationship( - TicketParticipant, - backref=backref('scanned_contacts', passive_deletes=True), + back_populates='scanned_contacts' ) #: Datetime at which the scan happened scanned_at: Mapped[datetime] = sa_orm.mapped_column( @@ -158,10 +148,10 @@ def grouped_counts_for( query = ( db.session.query( - sa.column('project_id'), - sa.column('project_uuid'), - sa.column('project_title'), - sa.column('project_timezone'), + sa.column('project_id', sa.Integer()), + sa.column('project_uuid', sa.Uuid()), + sa.column('project_title', sa.String()), + sa.column('project_timezone', sa.String()), sa.cast( sa.func.date_trunc( 'day', diff --git a/funnel/models/email_address.py b/funnel/models/email_address.py index 7cf120e3c..67cbd13fd 100644 --- a/funnel/models/email_address.py +++ b/funnel/models/email_address.py @@ -39,6 +39,7 @@ 'EmailAddressBlockedError', 'EmailAddressInUseError', 'EmailAddress', + 'OptionalEmailAddressMixin', 'EmailAddressMixin', ] @@ -701,7 +702,7 @@ def is_valid_email_address( @declarative_mixin -class EmailAddressMixin: +class OptionalEmailAddressMixin: """ Mixin class for models that refer to :class:`EmailAddress`. @@ -737,7 +738,7 @@ def email_address_id(cls) -> Mapped[int | None]: @declared_attr @classmethod - def email_address(cls) -> Mapped[EmailAddress]: + def email_address(cls) -> Mapped[EmailAddress | None]: """Instance of :class:`EmailAddress` as a relationship.""" backref_name = 'used_in_' + cls.__tablename__ EmailAddress.__backrefs__.add(backref_name) @@ -763,19 +764,19 @@ def email(self) -> str | None: return None @email.setter - def email(self, value: str | None) -> None: + def email(self, __value: str | None) -> None: """Set an email address.""" if self.__email_for__: - if value is not None: + if __value is not None: self.email_address = EmailAddress.add_for( - getattr(self, self.__email_for__), value + getattr(self, self.__email_for__), __value ) else: self.email_address = None else: - if value is not None: - self.email_address = EmailAddress.add(value) + if __value is not None: + self.email_address = EmailAddress.add(__value) else: self.email_address = None @@ -799,6 +800,37 @@ def transport_hash(self) -> str | None: ) +@declarative_mixin +class EmailAddressMixin(OptionalEmailAddressMixin): + """Non-optional version of :class:`OptionalEmailAddressMixin`.""" + + __email_optional__: ClassVar[bool] = False + + if TYPE_CHECKING: + + @declared_attr + @classmethod + def email_address_id(cls) -> Mapped[int]: # type: ignore[override] + ... + + @declared_attr + @classmethod + def email_address(cls) -> Mapped[EmailAddress]: # type: ignore[override] + ... + + @property # type: ignore[override] + def email(self) -> str: + ... + + @email.setter + def email(self, __value: str) -> None: + ... + + @property + def transport_hash(self) -> str: + ... + + auto_init_default(EmailAddress._delivery_state) # pylint: disable=protected-access auto_init_default(EmailAddress.delivery_state_at) auto_init_default(EmailAddress._is_blocked) # pylint: disable=protected-access diff --git a/funnel/models/sync_ticket.py b/funnel/models/sync_ticket.py index 1f7eb4bd3..2ad59769e 100644 --- a/funnel/models/sync_ticket.py +++ b/funnel/models/sync_ticket.py @@ -24,7 +24,7 @@ sa_orm, ) from .account import Account, AccountEmail -from .email_address import EmailAddress, EmailAddressMixin +from .email_address import EmailAddress, OptionalEmailAddressMixin from .helpers import reopen from .project import Project from .project_membership import project_child_role_map @@ -206,11 +206,10 @@ class TicketType(GetTitleMixin, Model): } -class TicketParticipant(EmailAddressMixin, UuidMixin, BaseMixin, Model): +class TicketParticipant(OptionalEmailAddressMixin, UuidMixin, BaseMixin, Model): """A participant in one or more events, synced from an external ticket source.""" __tablename__ = 'ticket_participant' - __email_optional__ = True __email_for__ = 'participant' fullname: Mapped[str] = with_roles( @@ -267,6 +266,10 @@ class TicketParticipant(EmailAddressMixin, UuidMixin, BaseMixin, Model): grants_via={None: project_child_role_map}, ) + scanned_contacts: Mapped[ContactExchange] = relationship( + passive_deletes=True, back_populates='ticket_participant' + ) + __table_args__ = (sa.UniqueConstraint('project_id', 'email_address_id'),) # Since 'email' comes from the mixin, it's not available to be annotated using @@ -617,5 +620,4 @@ def ticket_followers(self) -> Query[Account]: # Tail imports to avoid cyclic dependency errors, for symbols used only in methods -# pylint: disable=wrong-import-position -from .contact_exchange import ContactExchange # isort:skip +from .contact_exchange import ContactExchange diff --git a/tests/unit/models/email_address_test.py b/tests/unit/models/email_address_test.py index c56d2ab07..0f49e7e51 100644 --- a/tests/unit/models/email_address_test.py +++ b/tests/unit/models/email_address_test.py @@ -420,7 +420,6 @@ class EmailLink(models.EmailAddressMixin, models.BaseMixin, models.Model): """Test model connecting EmailUser to EmailAddress.""" __tablename__ = 'test_email_link' - __email_optional__ = False __email_unique__ = True __email_for__ = 'emailuser' __email_is_exclusive__ = True @@ -430,12 +429,16 @@ class EmailLink(models.EmailAddressMixin, models.BaseMixin, models.Model): ) emailuser = relationship(EmailUser) - class EmailDocument(models.EmailAddressMixin, models.BaseMixin, models.Model): + class EmailDocument( + models.OptionalEmailAddressMixin, models.BaseMixin, models.Model + ): """Test model unaffiliated to a user that has an email address attached.""" __tablename__ = 'test_email_document' - class EmailLinkedDocument(models.EmailAddressMixin, models.BaseMixin, models.Model): + class EmailLinkedDocument( + models.OptionalEmailAddressMixin, models.BaseMixin, models.Model + ): """Test model that accepts an optional user and an optional email.""" __tablename__ = 'test_email_linked_document'