Add member-only visibility for project updates (#1988)

---------

Co-authored-by: Amogh M Aradhya <[email protected]>
Co-authored-by: Zainab Bawa <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Makefile

@@ -136,7 +136,7 @@ initpy-loginproviders:
 initpy-transports:
 	# Do not auto-gen funnel/transports/__init__.py, only sub-packages
 	mkinit --inplace --relative --black funnel/transports/email
-	mkinit --inplace --relative --black  funnel/transports/sms
+	mkinit --inplace --relative --black funnel/transports/sms
 	isort funnel/transports/*/__init__.py
 	black funnel/transports/*/__init__.py
 

funnel/forms/update.py

@@ -4,7 +4,7 @@
 
 from baseframe import __, forms
 
-from ..models import Update
+from ..models import VISIBILITY_STATE, Update
 
 __all__ = ['UpdateForm', 'UpdatePinForm']
 
@@ -23,11 +23,24 @@ class UpdateForm(forms.Form):
         validators=[forms.validators.DataRequired()],
         description=__("Markdown formatting is supported"),
     )
-    is_pinned = forms.BooleanField(
-        __("Pin this update above other updates"), default=False
-    )
-    is_restricted = forms.BooleanField(
-        __("Limit access to current participants only"), default=False
+    visibility = forms.RadioField(
+        __("Who gets this update?"),
+        description=__("This can’t be changed after publishing the update"),
+        default=VISIBILITY_STATE[VISIBILITY_STATE.PUBLIC].name,
+        choices=[
+            (
+                VISIBILITY_STATE[VISIBILITY_STATE.PUBLIC].name,
+                __("Public; account followers will be notified"),
+            ),
+            (
+                VISIBILITY_STATE[VISIBILITY_STATE.MEMBERS].name,
+                __("Only account members"),
+            ),
+            (
+                VISIBILITY_STATE[VISIBILITY_STATE.PARTICIPANTS].name,
+                __("Only project participants"),
+            ),
+        ],
     )
 
 

funnel/models/__init__.py

@@ -98,7 +98,6 @@
     "ModelUrlProtocol",
     "ModelUuidProtocol",
     "NewCommentNotification",
-    "NewUpdateNotification",
     "NoIdMixin",
     "Notification",
     "NotificationFor",
@@ -131,6 +130,7 @@
     "ProjectSponsorMembership",
     "ProjectStartingNotification",
     "ProjectTomorrowNotification",
+    "ProjectUpdateNotification",
     "Proposal",
     "ProposalLabelProxy",
     "ProposalLabelProxyWrapper",
@@ -171,6 +171,7 @@
     "UrlType",
     "User",
     "UuidMixin",
+    "VISIBILITY_STATE",
     "Venue",
     "VenueRoom",
     "VideoError",

funnel/models/__init__.pyi

@@ -172,13 +172,13 @@ from .notification_types import (
     CommentReplyNotification,
     CommentReportReceivedNotification,
     NewCommentNotification,
-    NewUpdateNotification,
     OrganizationAdminMembershipNotification,
     OrganizationAdminMembershipRevokedNotification,
     ProjectCrewMembershipNotification,
     ProjectCrewMembershipRevokedNotification,
     ProjectStartingNotification,
     ProjectTomorrowNotification,
+    ProjectUpdateNotification,
     ProposalReceivedNotification,
     ProposalSubmittedNotification,
     RegistrationCancellationNotification,
@@ -229,7 +229,7 @@ from .typing import (
     ModelUrlProtocol,
     ModelUuidProtocol,
 )
-from .update import Update
+from .update import VISIBILITY_STATE, Update
 from .utils import (
     AccountAndAnchor,
     IncompleteUserMigrationError,
@@ -323,7 +323,6 @@ __all__ = [
     "ModelUrlProtocol",
     "ModelUuidProtocol",
     "NewCommentNotification",
-    "NewUpdateNotification",
     "NoIdMixin",
     "Notification",
     "NotificationFor",
@@ -356,6 +355,7 @@ __all__ = [
     "ProjectSponsorMembership",
     "ProjectStartingNotification",
     "ProjectTomorrowNotification",
+    "ProjectUpdateNotification",
     "Proposal",
     "ProposalLabelProxy",
     "ProposalLabelProxyWrapper",
@@ -396,6 +396,7 @@ __all__ = [
     "UrlType",
     "User",
     "UuidMixin",
+    "VISIBILITY_STATE",
     "Venue",
     "VenueRoom",
     "VideoError",

funnel/models/account.py

@@ -35,13 +35,13 @@
 from baseframe import __
 from coaster.sqlalchemy import (
     DynamicAssociationProxy,
-    LazyRoleSet,
     RoleMixin,
     StateManager,
     add_primary_relationship,
     auto_init_default,
     failsafe_add,
     immutable,
+    role_check,
     with_roles,
 )
 from coaster.utils import LabeledEnum, newsecret, require_one_of, utcnow
@@ -281,9 +281,6 @@ class Account(UuidMixin, BaseMixin[int, 'Account'], Model):
         ImgeeType, sa.CheckConstraint("banner_image_url <> ''"), nullable=True
     )
 
-    # These two flags are read-only. There is no provision for writing to them within
-    # the app:
-
     #: Protected accounts cannot be deleted
     is_protected: Mapped[bool] = with_roles(
         immutable(sa_orm.mapped_column(default=False)),
@@ -380,17 +377,20 @@ class Account(UuidMixin, BaseMixin[int, 'Account'], Model):
             order_by=lambda: AccountMembership.granted_at.asc(),
             viewonly=True,
         ),
-        grants_via={'member': {'admin', 'owner'}},
+        grants_via={'member': {'admin', 'member'}},
     )
 
-    active_owner_memberships: DynamicMapped[AccountMembership] = relationship(
-        lazy='dynamic',
-        primaryjoin=lambda: sa.and_(
-            sa_orm.remote(AccountMembership.account_id) == Account.id,
-            AccountMembership.is_active,
-            AccountMembership.is_owner.is_(True),
+    active_owner_memberships: DynamicMapped[AccountMembership] = with_roles(
+        relationship(
+            lazy='dynamic',
+            primaryjoin=lambda: sa.and_(
+                sa_orm.remote(AccountMembership.account_id) == Account.id,
+                AccountMembership.is_active,
+                AccountMembership.is_owner.is_(True),
+            ),
+            viewonly=True,
         ),
-        viewonly=True,
+        grants_via={'member': {'owner', 'admin', 'member'}},
     )
 
     active_invitations: DynamicMapped[AccountMembership] = relationship(
@@ -925,6 +925,7 @@ def ticket_followers(self) -> Query[Account]:
         'polymorphic_on': type_,
         # When querying the Account model, cast automatically to all subclasses
         'with_polymorphic': '*',
+        # Store a version id in this column to prevent edits to obsolete data
         'version_id_col': revisionid,
     }
 
@@ -1043,14 +1044,12 @@ def pickername(self) -> str:
 
     with_roles(pickername, read={'all'})
 
-    def roles_for(
-        self, actor: Account | None = None, anchors: Sequence = ()
-    ) -> LazyRoleSet:
-        """Identify roles for the given actor."""
-        roles = super().roles_for(actor, anchors)
-        if self.profile_state.ACTIVE_AND_PUBLIC:
-            roles.add('reader')
-        return roles
+    @role_check('reader')
+    def has_reader_role(
+        self, _actor: Account | None, _anchors: Sequence[Any] = ()
+    ) -> bool:
+        """Grant 'reader' role to all if the profile state is active and public."""
+        return bool(self.profile_state.ACTIVE_AND_PUBLIC)
 
     @cached_property
     def verified_contact_count(self) -> int:
@@ -1394,16 +1393,19 @@ def default_email(
         return None
 
     @property
-    def _self_is_owner_and_admin_of_self(self) -> Account:
+    def _self_is_owner_of_self(self) -> Account | None:
         """
-        Return self.
+        Return self in a user account.
 
         Helper method for :meth:`roles_for` and :meth:`actors_with` to assert that the
         user is owner and admin of their own account.
         """
-        return self
+        return self if self.is_user_profile else None
 
-    with_roles(_self_is_owner_and_admin_of_self, grants={'owner', 'admin'})
+    with_roles(
+        _self_is_owner_of_self,
+        grants={'follower', 'member', 'admin', 'owner'},
+    )
 
     def organizations_as_owner_ids(self) -> list[int]:
         """
@@ -2678,10 +2680,8 @@ class AccountExternalId(BaseMixin[int, Account], Model):
     # FIXME: change to sa.Unicode
     service: Mapped[str] = sa_orm.mapped_column(sa.UnicodeText, nullable=False)
     #: Unique user id as per external service, used for identifying related accounts
-    # FIXME: change to sa.Unicode
-    userid: Mapped[str] = sa_orm.mapped_column(
-        sa.UnicodeText, nullable=False
-    )  # Unique id (or obsolete OpenID)
+    # FIXME: change to sa.Unicode (uses UnicodeText for obsolete OpenID support)
+    userid: Mapped[str] = sa_orm.mapped_column(sa.UnicodeText, nullable=False)
     #: Optional public-facing username on the external service
     # FIXME: change to sa.Unicode. LinkedIn once used full URLs
     username: Mapped[str | None] = sa_orm.mapped_column(sa.UnicodeText, nullable=True)

funnel/models/account_membership.py

@@ -15,13 +15,11 @@
 
 class AccountMembership(ImmutableMembershipMixin, Model):
     """
-    An account can be a member of another account as an owner, admin or follower.
+    An account can be an owner, admin, member or follower of another account.
 
-    Owners can manage other administrators.
+    Owners can manage other owners, admins and members, but not followers.
 
-    TODO: This model may introduce non-admin memberships in a future iteration by
-    replacing :attr:`is_owner` with :attr:`member_level` or distinct role flags as in
-    :class:`ProjectMembership`.
+    TODO: Distinct flags for is_member, is_follower and is_admin.
     """
 
     __tablename__ = 'account_membership'
@@ -76,7 +74,7 @@ class AccountMembership(ImmutableMembershipMixin, Model):
         'related': {'urls', 'uuid_b58', 'offered_roles', 'is_owner'},
     }
 
-    #: Organization that this membership is being granted on
+    #: Account that this membership is being granted on
     account_id: Mapped[int] = sa_orm.mapped_column(
         sa.ForeignKey('account.id', ondelete='CASCADE'),
         default=None,
@@ -96,7 +94,10 @@ class AccountMembership(ImmutableMembershipMixin, Model):
     @cached_property
     def offered_roles(self) -> set[str]:
         """Roles offered by this membership record."""
-        roles = {'admin'}
+        # TODO: is_member and is_admin will be distinct flags in the future, with the
+        # base role set to `follower` only. is_owner will remain, but if it's set, then
+        # is_admin must also be set (enforced with a check constraint)
+        roles = {'follower', 'member', 'admin'}
         if self.is_owner:
             roles.add('owner')
         return roles

funnel/models/comment.py

@@ -9,7 +9,13 @@
 from werkzeug.utils import cached_property
 
 from baseframe import _, __
-from coaster.sqlalchemy import LazyRoleSet, RoleAccessProxy, StateManager, with_roles
+from coaster.sqlalchemy import (
+    LazyRoleSet,
+    RoleAccessProxy,
+    StateManager,
+    role_check,
+    with_roles,
+)
 from coaster.utils import LabeledEnum
 
 from .account import (
@@ -204,14 +210,14 @@ def last_comment(self) -> Comment | None:
 
     with_roles(last_comment, read={'all'}, datasets={'primary'})
 
-    def roles_for(
-        self, actor: Account | None = None, anchors: Sequence = ()
-    ) -> LazyRoleSet:
-        roles = super().roles_for(actor, anchors)
-        parent_roles = self.parent.roles_for(actor, anchors)
-        if 'participant' in parent_roles or 'commenter' in parent_roles:
-            roles.add('parent_participant')
-        return roles
+    @role_check('parent_participant')
+    def has_parent_participant_role(
+        self, actor: Account | None, _anchors: Sequence[Any] = ()
+    ) -> bool:
+        """Confirm if the actor is a participant in the parent object."""
+        return (parent := self.parent) is not None and parent.roles_for(actor).has_any(
+            {'participant', 'commenter'}
+        )
 
     @with_roles(call={'all'})
     @state.requires(state.NOT_DISABLED)
@@ -521,12 +527,12 @@ def was_reviewed_by(self, account: Account) -> bool:
             CommentModeratorReport.reported_by == account,
         ).notempty()
 
-    def roles_for(
-        self, actor: Account | None = None, anchors: Sequence = ()
-    ) -> LazyRoleSet:
-        roles = super().roles_for(actor, anchors)
-        roles.add('reader')
-        return roles
+    @role_check('reader')
+    def has_reader_role(
+        self, _actor: Account | None, _anchors: Sequence[Any] = ()
+    ) -> bool:
+        """Everyone is always a reader (for now)."""
+        return True
 
 
 add_search_trigger(Comment, 'search_vector')

funnel/models/contact_exchange.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from collections.abc import Collection, Sequence
+from collections.abc import Collection
 from dataclasses import dataclass
 from datetime import date as date_type, datetime
 from itertools import groupby
@@ -12,7 +12,7 @@
 from pytz import BaseTzInfo, timezone
 from sqlalchemy.ext.associationproxy import association_proxy
 
-from coaster.sqlalchemy import LazyRoleSet
+from coaster.sqlalchemy import with_roles
 from coaster.utils import uuid_to_base58
 
 from .account import Account
@@ -62,16 +62,19 @@ class ContactExchange(TimestampMixin, RoleMixin, Model):
     account_id: Mapped[int] = sa_orm.mapped_column(
         sa.ForeignKey('account.id', ondelete='CASCADE'), primary_key=True, default=None
     )
-    account: Mapped[Account] = relationship(back_populates='scanned_contacts')
+    account: Mapped[Account] = with_roles(
+        relationship(back_populates='scanned_contacts'), grants={'owner'}
+    )
     #: Participant whose contact was scanned
     ticket_participant_id: Mapped[int] = sa_orm.mapped_column(
         sa.ForeignKey('ticket_participant.id', ondelete='CASCADE'),
         primary_key=True,
         default=None,
         index=True,
     )
-    ticket_participant: Mapped[TicketParticipant] = relationship(
-        back_populates='scanned_contacts'
+    ticket_participant: Mapped[TicketParticipant] = with_roles(
+        relationship(back_populates='scanned_contacts'),
+        grants_via={'participant': {'subject'}},
     )
     #: Datetime at which the scan happened
     scanned_at: Mapped[datetime] = sa_orm.mapped_column(
@@ -101,17 +104,6 @@ class ContactExchange(TimestampMixin, RoleMixin, Model):
         'subject': {'read': {'account', 'ticket_participant', 'scanned_at'}},
     }
 
-    def roles_for(
-        self, actor: Account | None = None, anchors: Sequence = ()
-    ) -> LazyRoleSet:
-        roles = super().roles_for(actor, anchors)
-        if actor is not None:
-            if actor == self.account:
-                roles.add('owner')
-            if actor == self.ticket_participant.participant:
-                roles.add('subject')
-        return roles
-
     @classmethod
     def migrate_account(cls, old_account: Account, new_account: Account) -> None:
         """Migrate one account's data to another when merging accounts."""