Remove all uses of reopen for Account model

hasgeek · Dec 15, 2023 · 22b10fc · 22b10fc
1 parent 609be70
commit 22b10fc
Show file tree

Hide file tree

Showing 12 changed files with 378 additions and 427 deletions.
diff --git a/funnel/models/account.py b/funnel/models/account.py
diff --git a/funnel/models/account_membership.py b/funnel/models/account_membership.py
@@ -4,11 +4,10 @@
 
 from werkzeug.utils import cached_property
 
-from coaster.sqlalchemy import DynamicAssociationProxy, immutable, with_roles
+from coaster.sqlalchemy import immutable, with_roles
 
-from . import DynamicMapped, Mapped, Model, relationship, sa, sa_orm
+from . import Mapped, Model, relationship, sa, sa_orm
 from .account import Account
-from .helpers import reopen
 from .membership_mixin import ImmutableUserMembershipMixin
 
 __all__ = ['AccountMembership']
@@ -103,135 +102,3 @@ def offered_roles(self) -> set[str]:
         if self.is_owner:
             roles.add('owner')
         return roles
-
-
-# Add active membership relationships to Account
-@reopen(Account)
-class __Account:
-    memberships: DynamicMapped[AccountMembership] = relationship(
-        AccountMembership,
-        foreign_keys=[AccountMembership.account_id],
-        lazy='dynamic',
-        passive_deletes=True,
-        back_populates='account',
-    )
-    active_admin_memberships: DynamicMapped[AccountMembership] = with_roles(
-        relationship(
-            AccountMembership,
-            lazy='dynamic',
-            primaryjoin=sa.and_(
-                sa_orm.remote(AccountMembership.account_id) == Account.id,
-                AccountMembership.is_active,
-            ),
-            order_by=AccountMembership.granted_at.asc(),
-            viewonly=True,
-        ),
-        grants_via={'member': {'admin', 'owner'}},
-    )
-
-    active_owner_memberships: DynamicMapped[AccountMembership] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.account_id) == Account.id,
-            AccountMembership.is_active,
-            AccountMembership.is_owner.is_(True),
-        ),
-        viewonly=True,
-    )
-
-    active_invitations: DynamicMapped[AccountMembership] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.account_id) == Account.id,
-            AccountMembership.is_invite,
-            AccountMembership.revoked_at.is_(None),
-        ),
-        viewonly=True,
-    )
-
-    owner_users = with_roles(
-        DynamicAssociationProxy('active_owner_memberships', 'member'),
-        read={'all'},
-    )
-    admin_users = with_roles(
-        DynamicAssociationProxy[Account]('active_admin_memberships', 'member'),
-        read={'all'},
-    )
-
-    # pylint: disable=invalid-unary-operand-type
-    organization_admin_memberships: DynamicMapped[AccountMembership] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        foreign_keys=[AccountMembership.member_id],  # type: ignore[has-type]
-        viewonly=True,
-    )
-
-    noninvite_organization_admin_memberships: DynamicMapped[
-        AccountMembership
-    ] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        foreign_keys=[AccountMembership.member_id],
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.member_id)  # type: ignore[has-type]
-            == Account.id,
-            ~AccountMembership.is_invite,
-        ),
-        viewonly=True,
-    )
-
-    active_organization_admin_memberships: DynamicMapped[
-        AccountMembership
-    ] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        foreign_keys=[AccountMembership.member_id],
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.member_id)  # type: ignore[has-type]
-            == Account.id,
-            AccountMembership.is_active,
-        ),
-        viewonly=True,
-    )
-
-    active_organization_owner_memberships: DynamicMapped[
-        AccountMembership
-    ] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        foreign_keys=[AccountMembership.member_id],
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.member_id)  # type: ignore[has-type]
-            == Account.id,
-            AccountMembership.is_active,
-            AccountMembership.is_owner.is_(True),
-        ),
-        viewonly=True,
-    )
-
-    active_organization_invitations: DynamicMapped[AccountMembership] = relationship(
-        AccountMembership,
-        lazy='dynamic',
-        foreign_keys=[AccountMembership.member_id],
-        primaryjoin=sa.and_(
-            sa_orm.remote(AccountMembership.member_id)  # type: ignore[has-type]
-            == Account.id,
-            AccountMembership.is_invite,
-            AccountMembership.revoked_at.is_(None),
-        ),
-        viewonly=True,
-    )
-
-    organizations_as_owner = DynamicAssociationProxy[Account](
-        'active_organization_owner_memberships', 'account'
-    )
-
-    organizations_as_admin = DynamicAssociationProxy[Account](
-        'active_organization_admin_memberships', 'account'
-    )
-
-
-Account.__active_membership_attrs__.add('active_organization_admin_memberships')
-Account.__noninvite_membership_attrs__.add('noninvite_organization_admin_memberships')
diff --git a/funnel/models/helpers.py b/funnel/models/helpers.py
@@ -319,7 +319,7 @@ def decorator(temp_cls: type) -> ReopenedType:
                     # requires code rewrite. This is however an implicit assumption when
                     # using @reopen -- it should not be within a try block.
                     raise AttributeError(
-                        f"{cls.__qualname__} already has attribute {attr}",
+                        f"{cls.__qualname__} already has attribute {attr!r}",
                         name=attr,
                         obj=cls,
                     )

diff --git a/funnel/models/mailer.py b/funnel/models/mailer.py
@@ -31,7 +31,6 @@
     sa_orm,
 )
 from .account import Account
-from .helpers import reopen
 from .types import jsonb
 
 __all__ = [
@@ -449,10 +448,3 @@ def custom_draft_in(cls, mailer: Mailer) -> list[Self]:
             )
             .all()
         )
-
-
-@reopen(Account)
-class __Account:
-    mailers: Mapped[list[Mailer]] = relationship(
-        Mailer, back_populates='user', order_by=Mailer.updated_at.desc()
-    )
diff --git a/funnel/models/membership_mixin.py b/funnel/models/membership_mixin.py
@@ -5,7 +5,7 @@
 from collections.abc import Callable, Iterable
 from datetime import datetime as datetime_type
 from types import SimpleNamespace
-from typing import TYPE_CHECKING, Any, ClassVar, Generic, TypeVar
+from typing import TYPE_CHECKING, Any, ClassVar, Generic, Self, TypeVar
 from uuid import UUID
 
 from sqlalchemy import event
@@ -101,7 +101,7 @@ class ImmutableMembershipMixin(UuidMixin, BaseMixin[UUID]):
         __tablename__: str
         #: Parent column (declare as synonym of 'profile_id' or 'project_id' in
         #: subclasses)
-        parent_id: Mapped[int]
+        parent_id: Mapped[int] | None
         #: Parent object
         parent: Mapped[Model | None]
         #: Subject of this membership (subclasses must define)
@@ -453,7 +453,7 @@ def copy_template(self: MembershipType, **kwargs) -> MembershipType:
     @classmethod
     def migrate_account(cls, old_account: Account, new_account: Account) -> None:
         """
-        Migrate memberhip records from one account to another.
+        Migrate membership records from one account to another.
 
         If both accounts have active records, they are merged into a new record in the
         new account's favour. All revoked records for the old account are transferred to
@@ -582,7 +582,7 @@ class FrozenAttributionProtoMixin:
 
     if TYPE_CHECKING:
         member: Mapped[Account]
-        replace: Callable[..., FrozenAttributionType]
+        replace: Callable[..., Self]
         _local_data_only: bool
 
     @declared_attr
@@ -599,7 +599,7 @@ def _title(cls) -> Mapped[str | None]:
     def title(self) -> str:
         """Attribution title for this record."""
         if self._local_data_only:
-            # self._title may be None
+            # self._title may be None when returning local data
             return self._title  # type: ignore[return-value]
         return self._title or self.member.title
 
@@ -616,14 +616,10 @@ def pickername(self) -> str:
         return self._title if self._title else self.member.pickername
 
     @with_roles(call={'owner', 'member'})
-    def freeze_member_attribution(
-        self: FrozenAttributionType, actor: Account
-    ) -> FrozenAttributionType:
+    def freeze_member_attribution(self, actor: Account) -> Self:
         """Freeze member attribution and return a replacement record."""
         if self._title is None:
-            membership: FrozenAttributionType = self.replace(
-                actor=actor, title=self.member.title
-            )
+            membership = self.replace(actor=actor, title=self.member.title)
         else:
             membership = self
         return membership

diff --git a/funnel/models/proposal_membership.py b/funnel/models/proposal_membership.py
@@ -4,17 +4,16 @@
 
 from werkzeug.utils import cached_property
 
-from coaster.sqlalchemy import DynamicAssociationProxy, immutable, with_roles
+from coaster.sqlalchemy import immutable, with_roles
 
-from . import DynamicMapped, Mapped, Model, backref, relationship, sa, sa_orm
+from . import Mapped, Model, backref, relationship, sa, sa_orm
 from .account import Account
 from .helpers import reopen
 from .membership_mixin import (
     FrozenAttributionProtoMixin,
     ImmutableUserMembershipMixin,
     ReorderMembershipProtoMixin,
 )
-from .project import Project
 from .proposal import Proposal
 
 __all__ = ['ProposalMembership']
@@ -154,57 +153,3 @@ def first_user(self) -> Account:
             if not membership.is_uncredited:
                 return membership.member
         return self.created_by
-
-
-@reopen(Account)
-class __Account:
-    # pylint: disable=invalid-unary-operand-type
-
-    all_proposal_memberships: DynamicMapped[ProposalMembership] = relationship(
-        ProposalMembership,
-        lazy='dynamic',
-        foreign_keys=[ProposalMembership.member_id],
-        viewonly=True,
-    )
-
-    noninvite_proposal_memberships: DynamicMapped[ProposalMembership] = relationship(
-        ProposalMembership,
-        lazy='dynamic',
-        primaryjoin=sa.and_(
-            ProposalMembership.member_id == Account.id,
-            ~ProposalMembership.is_invite,
-        ),
-        viewonly=True,
-    )
-
-    proposal_memberships: DynamicMapped[ProposalMembership] = relationship(
-        ProposalMembership,
-        lazy='dynamic',
-        primaryjoin=sa.and_(
-            ProposalMembership.member_id == Account.id,
-            ProposalMembership.is_active,
-        ),
-        viewonly=True,
-    )
-
-    proposals = DynamicAssociationProxy[Proposal]('proposal_memberships', 'proposal')
-
-    @property
-    def public_proposal_memberships(self):
-        """Query for all proposal memberships to proposals that are public."""
-        return (
-            self.proposal_memberships.join(Proposal, ProposalMembership.proposal)
-            .join(Project, Proposal.project)
-            .filter(
-                ProposalMembership.is_uncredited.is_(False),
-                # TODO: Include proposal state filter (pending proposal workflow fix)
-            )
-        )
-
-    public_proposals = DynamicAssociationProxy[Proposal](
-        'public_proposal_memberships', 'proposal'
-    )
-
-
-Account.__active_membership_attrs__.add('proposal_memberships')
-Account.__noninvite_membership_attrs__.add('noninvite_proposal_memberships')
diff --git a/funnel/models/reorder_mixin.py b/funnel/models/reorder_mixin.py
@@ -107,7 +107,7 @@ def reorder_item(self: Reorderable, other: Reorderable, before: bool) -> None:
             .where(self.parent_scoped_reorder_query_filter)
             .scalar_subquery()
         )
-        # Flush it so the db doesn't complain when there's a unique constraint
+        # Flush it so the db does not complain when there's a unique constraint
         db.session.flush()
         # Reassign all remaining sequence numbers
         for reorderable_item in items_to_reorder[1:]:  # Skip 0, which is self

diff --git a/funnel/models/rsvp.py b/funnel/models/rsvp.py
@@ -15,7 +15,6 @@
     Mapped,
     Model,
     NoIdMixin,
-    Query,
     UuidMixin,
     backref,
     db,
@@ -253,18 +252,3 @@ def rsvp_count_going(self) -> int:
             .filter(Account.state.ACTIVE, Rsvp.state.YES)
             .count()
         )
-
-
-@reopen(Account)
-class __Account:
-    @property
-    def rsvp_followers(self) -> Query[Account]:
-        """All users with an active RSVP in a project."""
-        return (
-            Account.query.filter(Account.state.ACTIVE)
-            .join(Rsvp, Rsvp.participant_id == Account.id)
-            .join(Project, Rsvp.project_id == Project.id)
-            .filter(Rsvp.state.YES, Project.state.PUBLISHED, Project.account == self)
-        )
-
-    with_roles(rsvp_followers, grants={'follower'})