diff --git a/Makefile b/Makefile index 22d701d81..2b6d6e583 100644 --- a/Makefile +++ b/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 diff --git a/funnel/forms/update.py b/funnel/forms/update.py index 9fcb4b2cd..6a299eaba 100644 --- a/funnel/forms/update.py +++ b/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"), + ), + ], ) diff --git a/funnel/models/__init__.py b/funnel/models/__init__.py index cd27c13a9..9ebed3afd 100644 --- a/funnel/models/__init__.py +++ b/funnel/models/__init__.py @@ -98,7 +98,6 @@ "ModelUrlProtocol", "ModelUuidProtocol", "NewCommentNotification", - "NewUpdateNotification", "NoIdMixin", "Notification", "NotificationFor", @@ -130,6 +129,7 @@ "ProjectRsvpStateEnum", "ProjectSponsorMembership", "ProjectStartingNotification", + "ProjectUpdateNotification", "Proposal", "ProposalLabelProxy", "ProposalLabelProxyWrapper", @@ -170,6 +170,7 @@ "UrlType", "User", "UuidMixin", + "VISIBILITY_STATE", "Venue", "VenueRoom", "VideoError", diff --git a/funnel/models/__init__.pyi b/funnel/models/__init__.pyi index ea0833f14..9468a0fe5 100644 --- a/funnel/models/__init__.pyi +++ b/funnel/models/__init__.pyi @@ -172,12 +172,12 @@ from .notification_types import ( CommentReplyNotification, CommentReportReceivedNotification, NewCommentNotification, - NewUpdateNotification, OrganizationAdminMembershipNotification, OrganizationAdminMembershipRevokedNotification, ProjectCrewMembershipNotification, ProjectCrewMembershipRevokedNotification, ProjectStartingNotification, + ProjectUpdateNotification, ProposalReceivedNotification, ProposalSubmittedNotification, RegistrationCancellationNotification, @@ -228,7 +228,7 @@ from .typing import ( ModelUrlProtocol, ModelUuidProtocol, ) -from .update import Update +from .update import VISIBILITY_STATE, Update from .utils import ( AccountAndAnchor, IncompleteUserMigrationError, @@ -322,7 +322,6 @@ __all__ = [ "ModelUrlProtocol", "ModelUuidProtocol", "NewCommentNotification", - "NewUpdateNotification", "NoIdMixin", "Notification", "NotificationFor", @@ -354,6 +353,7 @@ __all__ = [ "ProjectRsvpStateEnum", "ProjectSponsorMembership", "ProjectStartingNotification", + "ProjectUpdateNotification", "Proposal", "ProposalLabelProxy", "ProposalLabelProxyWrapper", @@ -394,6 +394,7 @@ __all__ = [ "UrlType", "User", "UuidMixin", + "VISIBILITY_STATE", "Venue", "VenueRoom", "VideoError", diff --git a/funnel/models/account.py b/funnel/models/account.py index b09e63b4f..ad8cade4a 100644 --- a/funnel/models/account.py +++ b/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) diff --git a/funnel/models/account_membership.py b/funnel/models/account_membership.py index f1aefb8b7..f188f22ac 100644 --- a/funnel/models/account_membership.py +++ b/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 diff --git a/funnel/models/comment.py b/funnel/models/comment.py index 07e188a35..5534cbede 100644 --- a/funnel/models/comment.py +++ b/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') diff --git a/funnel/models/contact_exchange.py b/funnel/models/contact_exchange.py index 92c7cbd28..ccbe9bf0d 100644 --- a/funnel/models/contact_exchange.py +++ b/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,7 +62,9 @@ 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'), @@ -70,8 +72,9 @@ class ContactExchange(TimestampMixin, RoleMixin, Model): 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.""" diff --git a/funnel/models/notification.py b/funnel/models/notification.py index d5128a31e..157bb3314 100644 --- a/funnel/models/notification.py +++ b/funnel/models/notification.py @@ -50,7 +50,7 @@ now queues a third series of background workers, for each of the supported transports if at least one recipient in that batch wants to use that transport. -6. A separate render view class named RenderNewUpdateNotification contains methods named +6. A separate render view class named RenderNotification contains methods named like `web`, `email`, `sms` and others. These are expected to return a rendered message. The `web` render is used for the notification feed page on the website. @@ -283,7 +283,7 @@ class NotificationType(Generic[_D, _F], Protocol): eventid_b58: str document: _D document_uuid: UUID - fragment: _F | None + fragment: _F fragment_uuid: UUID | None created_at: datetime created_by_id: int | None @@ -344,19 +344,22 @@ class Notification(NoIdMixin, Model, Generic[_D, _F]): #: another type (auto-populated from subclass's `shadow=` parameter) pref_type: ClassVar[str] = '' - #: Document model, must be specified in subclasses + #: Document model, auto-populated from generic arg to Notification base class document_model: ClassVar[type[ModelUuidProtocol]] #: SQL table name for document type, auto-populated from the document model document_type: ClassVar[str] - #: Fragment model, optional for subclasses - fragment_model: ClassVar[type[ModelUuidProtocol] | None] = None + #: Fragment model, auto-populated from generic arg to Notification base class + fragment_model: ClassVar[type[ModelUuidProtocol] | None] #: SQL table name for fragment type, auto-populated from the fragment model fragment_type: ClassVar[str | None] #: Roles to send notifications to. Roles must be in order of priority for situations - #: where a user has more than one role on the document. - roles: ClassVar[Sequence[str]] = [] + #: where a user has more than one role on the document. The notification can + #: customize target roles based on the document or fragment's properties + @property + def dispatch_roles(self) -> Sequence[str]: + return [] #: Exclude triggering actor from receiving notifications? Subclasses may override exclude_actor: ClassVar[bool] = False @@ -609,18 +612,19 @@ def document(self) -> _D: ) @cached_property - def fragment(self) -> _F | None: + def fragment(self) -> _F: """ Retrieve the fragment within a document referenced by this Notification, if any. This assumes the underlying object won't disappear, as there is no SQL foreign key constraint enforcing a link. """ - if self.fragment_uuid and self.fragment_model: + if self.fragment_model is not None and self.fragment_uuid is not None: return cast( _F, self.fragment_model.query.filter_by(uuid=self.fragment_uuid).one() ) - return None + # TypeVar _F may be typed `None` in a subclass, but we can't type it so here + return None # type: ignore[return-value] @classmethod def renderer(cls, view: type[T]) -> type[T]: @@ -667,7 +671,7 @@ def dispatch(self) -> Generator[NotificationRecipient, None, None]: should override this method. """ for account, role in self.role_provider_obj.actors_with( - self.roles, with_role=True + self.dispatch_roles, with_role=True ): # If this notification requires that it not be sent to the actor that # triggered the notification, don't notify them. For example, a user who @@ -707,14 +711,6 @@ def dispatch(self) -> Generator[NotificationRecipient, None, None]: yield recipient -# Make :attr:`type_` available under the name `type`, but declare this outside the class -# (a) to avoid conflicts with the Python `type` global that is used for type-hinting, -# and (b) because SQLAlchemy >= 2.0.26 attempts to resolve annotations using the -# class-local namespace, which overrides the global `type` (as of this commit): -# https://github.com/sqlalchemy/sqlalchemy/commit/153f287b9949462ec29d66fc9b329d0144a6ca7c -Notification.type = sa_orm.synonym('type_') - - class PreviewNotification(NotificationType): """ Mimics a Notification subclass without instantiating it, for providing a preview. @@ -1242,7 +1238,7 @@ def role(self) -> str | None: """User's primary matching role for this notification.""" if self.document and self.recipient: roles = self.document.roles_for(self.recipient) - for role in self.notification.roles: + for role in self.notification.dispatch_roles: if role in roles: return role return None @@ -1278,8 +1274,9 @@ class NotificationPreferences(BaseMixin[int, Account], Model): grants={'owner'}, ) - # Notification type, corresponding to Notification.type (a class attribute there) - # notification_type = '' holds the veto switch to disable a transport entirely + # Notification type, corresponding to `Notification.cls_type` (a class attribute + # there). `notification_type = ''` holds the veto switch to disable a transport + # entirely notification_type: Mapped[str] = immutable(sa_orm.mapped_column()) by_email: Mapped[bool] = with_roles(sa_orm.mapped_column(), rw={'owner'}) diff --git a/funnel/models/notification_types.py b/funnel/models/notification_types.py index 3ba50ba9b..93a18a3c0 100644 --- a/funnel/models/notification_types.py +++ b/funnel/models/notification_types.py @@ -1,5 +1,10 @@ """Notification types.""" +# Pyright complains that a property in the base class (for `roles`) has become a +# classvar in the subclass. Mypy does not. Silence Pyright here + +# pyright: reportAssignmentType=false + from __future__ import annotations from baseframe import __ @@ -18,7 +23,7 @@ __all__ = [ 'AccountPasswordNotification', - 'NewUpdateNotification', + 'ProjectUpdateNotification', 'CommentReportReceivedNotification', 'CommentReplyNotification', 'NewCommentNotification', @@ -76,7 +81,7 @@ class AccountPasswordNotification( description = __("For your safety, in case this was not authorized") exclude_actor = False - roles = ['owner'] + dispatch_roles = ['owner'] for_private_recipient = True @@ -92,7 +97,7 @@ class RegistrationConfirmationNotification( title = __("When I register for a project") description = __("This will prompt a calendar entry in Gmail and other apps") - roles = ['owner'] + dispatch_roles = ['owner'] exclude_actor = False # This is a notification to the actor for_private_recipient = True @@ -105,26 +110,41 @@ class RegistrationCancellationNotification( ): """Notification confirming cancelling registration to a project.""" - roles = ['owner'] + dispatch_roles = ['owner'] exclude_actor = False # This is a notification to the actor for_private_recipient = True allow_web = False -class NewUpdateNotification( - DocumentHasProject, Notification[Update, None], type='update_new' +class ProjectUpdateNotification( + DocumentHasAccount, Notification[Project, Update], type='project_update' ): - """Notifications of new updates.""" + """Notification of a new update in a project.""" category = notification_categories.participant - title = __("When a project posts an update") + title = __("When a project has an update") description = __( "Typically contains critical information such as video conference links" ) - roles = ['project_crew', 'project_participant', 'account_participant'] exclude_actor = False # Send to everyone including the actor + @property + def dispatch_roles(self) -> list[str]: + """Target roles based on Update visibility state.""" + # TODO: Use match/case matching here. If states use a Python Enum, Mypy will + # do an exhaustiveness check, so the closing RuntimeError is not needed. + # https://github.com/python/mypy/issues/6366 + visibility = self.fragment.visibility_state + if visibility.PUBLIC: + return ['project_crew', 'project_participant', 'account_follower'] + if visibility.PARTICIPANTS: + return ['project_crew', 'project_participant'] + if visibility.MEMBERS: + return ['project_crew', 'project_participant', 'account_member'] + + raise RuntimeError("Unknown update visibility state") + class ProposalSubmittedNotification( DocumentHasProject, Notification[Proposal, None], type='proposal_submitted' @@ -135,7 +155,7 @@ class ProposalSubmittedNotification( title = __("When I submit a proposal") description = __("Confirmation for your records") - roles = ['creator'] + dispatch_roles = ['creator'] exclude_actor = False # This notification is for the actor # Email is typically fine. Messengers may be too noisy @@ -157,7 +177,7 @@ class ProjectStartingNotification( title = __("When a project I’ve registered for is about to start") description = __("You will be notified 5-10 minutes before the starting time") - roles = ['project_crew', 'project_participant'] + dispatch_roles = ['project_crew', 'project_participant'] # This is a notification triggered without an actor @@ -171,7 +191,7 @@ class NewCommentNotification(Notification[Commentset, Comment], type='comment_ne title = __("When there is a new comment on something I’m involved in") exclude_actor = True - roles = ['replied_to_commenter', 'document_subscriber'] + dispatch_roles = ['replied_to_commenter', 'document_subscriber'] class CommentReplyNotification(Notification[Comment, Comment], type='comment_reply'): @@ -183,7 +203,7 @@ class CommentReplyNotification(Notification[Comment, Comment], type='comment_rep # document_model = Parent comment (being replied to) # fragment_model = Child comment (the reply that triggered notification) - roles = ['replied_to_commenter'] + dispatch_roles = ['replied_to_commenter'] # --- Project crew notifications ------------------------------------------------------- @@ -200,7 +220,7 @@ class ProjectCrewMembershipNotification( title = __("When a project crew member is added or removed") description = __("Crew members have access to the project’s settings and data") - roles = ['member', 'project_crew'] + dispatch_roles = ['member', 'project_crew'] exclude_actor = True # Alerts other users of actor's actions; too noisy for actor @@ -212,7 +232,7 @@ class ProjectCrewMembershipRevokedNotification( ): """Notification of being removed from crew membership (including role changes).""" - roles = ['member', 'project_crew'] + dispatch_roles = ['member', 'project_crew'] exclude_actor = True # Alerts other users of actor's actions; too noisy for actor @@ -224,7 +244,7 @@ class ProposalReceivedNotification( category = notification_categories.project_crew title = __("When my project receives a new proposal") - roles = ['project_editor'] + dispatch_roles = ['project_editor'] exclude_actor = True # Don't notify editor of proposal they submitted @@ -238,7 +258,7 @@ class RegistrationReceivedNotification( category = notification_categories.project_crew title = __("When someone registers for my project") - roles = ['project_promoter'] + dispatch_roles = ['project_promoter'] exclude_actor = True @@ -256,7 +276,7 @@ class OrganizationAdminMembershipNotification( title = __("When account admins change") description = __("Account admins control all projects under the account") - roles = ['member', 'account_admin'] + dispatch_roles = ['member', 'account_admin'] exclude_actor = True # Alerts other users of actor's actions; too noisy for actor @@ -268,7 +288,7 @@ class OrganizationAdminMembershipRevokedNotification( ): """Notification of being granted admin membership (including role changes).""" - roles = ['member', 'account_admin'] + dispatch_roles = ['member', 'account_admin'] exclude_actor = True # Alerts other users of actor's actions; too noisy for actor @@ -283,4 +303,4 @@ class CommentReportReceivedNotification( category = notification_categories.site_admin title = __("When a comment is reported as spam") - roles = ['comment_moderator'] + dispatch_roles = ['comment_moderator'] diff --git a/funnel/models/project.py b/funnel/models/project.py index 3f110ddfe..409364db5 100644 --- a/funnel/models/project.py +++ b/funnel/models/project.py @@ -5,7 +5,7 @@ from __future__ import annotations from collections import OrderedDict, defaultdict -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from datetime import datetime, timedelta from enum import ReprEnum from typing import TYPE_CHECKING, Any, Literal, Self, cast, overload @@ -20,9 +20,9 @@ from baseframe import __, localize_timezone from coaster.sqlalchemy import ( DynamicAssociationProxy, - LazyRoleSet, ManagedState, StateManager, + role_check, with_roles, ) from coaster.utils import LabeledEnum, buid, utcnow @@ -102,11 +102,12 @@ class Project(UuidMixin, BaseScopedNameMixin[int, Account], Model): account: Mapped[Account] = with_roles( relationship(foreign_keys=[account_id], back_populates='projects'), read={'all'}, - # If account grants an 'admin' role, make it 'account_admin' here + # Remap account roles for use in project grants_via={ None: { + 'owner': 'account_owner', 'admin': 'account_admin', - 'follower': 'account_participant', + 'follower': 'account_follower', 'member': 'account_member', } }, @@ -686,6 +687,31 @@ def __format__(self, format_spec: str) -> str: return self.joined_title return format(self.joined_title, format_spec) + @role_check('member_participant') + def has_member_participant_role( + self, actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Confirm if the actor is both a participant and an account member.""" + if actor is None: + return False + roles = self.roles_for(actor) + if 'participant' in roles and 'account_member' in roles: + return True + return False + + @has_member_participant_role.iterable + def _(self) -> Iterable[Account]: + """All participants who are also account members.""" + # TODO: This iterable causes a loop of SQL queries. It will be far more + # efficient to SQL JOIN the data sources once they are single-table sources. + # Rsvp needs to merge into ProjectMembership, and AccountMembership needs to + # replace the hacky "membership project" data source. + return ( + account + for account in self.actors_with({'participant'}) + if 'account_member' in self.roles_for(account) + ) + @with_roles(call={'editor'}) @cfp_state.transition( cfp_state.OPENABLE, @@ -767,7 +793,7 @@ def title_suffix(self) -> str: with_roles(title_suffix, read={'all'}) @property - def title_parts(self) -> list[str]: + def title_parts(self) -> tuple[str] | tuple[str, str]: """ Return the hierarchy of titles of this project. @@ -780,9 +806,9 @@ def title_parts(self) -> list[str]: """ if self.short_title == self.title: # Project title does not derive from account title, so use both - return [self.account.title, self.title] + return (self.account.title, self.title) # Project title extends account title, so account title is not needed - return [self.title] + return (self.title,) with_roles(title_parts, read={'all'}) @@ -928,21 +954,19 @@ def rsvp_for(self, account: Account | None, create: bool = False) -> Rsvp | None return Rsvp.get_for(self, account, create) def rsvps_with(self, state: RsvpStateEnum) -> Query[Rsvp]: + # pylint: disable=protected-access return self.rsvps.join(Account).filter( - Account.state.ACTIVE, - Rsvp._state == state, # pylint: disable=protected-access + Account.state.ACTIVE, Rsvp._state == state ) def rsvp_counts(self) -> dict[str, int]: + # pylint: disable=protected-access return { row[0]: row[1] - for row in db.session.query( - Rsvp._state, # pylint: disable=protected-access - sa.func.count(Rsvp._state), # pylint: disable=protected-access - ) + for row in db.session.query(Rsvp._state, sa.func.count(Rsvp._state)) .join(Account) .filter(Account.state.ACTIVE, Rsvp.project == self) - .group_by(Rsvp._state) # pylint: disable=protected-access + .group_by(Rsvp._state) .all() } @@ -959,13 +983,12 @@ def update_schedule_timestamps(self) -> None: self.start_at = self.schedule_start_at self.end_at = self.schedule_end_at - def roles_for( - self, actor: Account | None = None, anchors: Sequence = () - ) -> LazyRoleSet: - roles = super().roles_for(actor, anchors) - # https://github.com/hasgeek/funnel/pull/220#discussion_r168718052 - roles.add('reader') - return roles + @role_check('reader') + def has_reader_role( + self, _actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Unconditionally grant reader role (for now).""" + return True def is_safe_to_delete(self) -> bool: """Return True if project has no proposals.""" diff --git a/funnel/models/proposal.py b/funnel/models/proposal.py index ad75bd4e1..63c9fb7d9 100644 --- a/funnel/models/proposal.py +++ b/funnel/models/proposal.py @@ -12,8 +12,8 @@ from baseframe.filters import preview from coaster.sqlalchemy import ( DynamicAssociationProxy, - LazyRoleSet, StateManager, + role_check, with_roles, ) from coaster.utils import LabeledEnum @@ -539,21 +539,19 @@ def has_sponsors(self) -> bool: sponsors = DynamicAssociationProxy[Account]('sponsor_memberships', 'member') - def roles_for( - self, actor: Account | None = None, anchors: Sequence = () - ) -> LazyRoleSet: - roles = super().roles_for(actor, anchors) - if self.state.DRAFT: - if 'reader' in roles: - # https://github.com/hasgeek/funnel/pull/220#discussion_r168724439 - roles.remove('reader') - else: - roles.add('reader') - - if roles.has_any(('project_participant', 'submitter')): - roles.add('commenter') - - return roles + @role_check('reader') + def has_reader_role( + self, _actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Grant reader role if the proposal is not a draft.""" + return not self.state.DRAFT + + @role_check('commenter') + def has_commenter_role( + self, actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Grant 'commenter' role to any participant or submitter.""" + return self.roles_for(actor).has_any(('project_participant', 'submitter')) @classmethod def all_public(cls) -> Query[Self]: diff --git a/funnel/models/saved.py b/funnel/models/saved.py index 53a30fc91..441bf3273 100644 --- a/funnel/models/saved.py +++ b/funnel/models/saved.py @@ -2,10 +2,9 @@ from __future__ import annotations -from collections.abc import Sequence from datetime import datetime -from coaster.sqlalchemy import LazyRoleSet, with_roles +from coaster.sqlalchemy import with_roles from .account import Account from .base import Mapped, Model, NoIdMixin, db, relationship, sa, sa_orm @@ -68,7 +67,9 @@ class SavedSession(NoIdMixin, Model): nullable=False, primary_key=True, ) - account: Mapped[Account] = relationship(back_populates='saved_sessions') + account: Mapped[Account] = with_roles( + relationship(back_populates='saved_sessions'), grants={'owner'} + ) #: Session that was saved session_id: Mapped[int] = sa_orm.mapped_column( sa.ForeignKey('session.id', ondelete='CASCADE'), @@ -90,14 +91,6 @@ class SavedSession(NoIdMixin, Model): sa.UnicodeText, nullable=True ) - def roles_for( - self, actor: Account | None = None, anchors: Sequence = () - ) -> LazyRoleSet: - roles = super().roles_for(actor, anchors) - if actor is not None and actor == self.account: - roles.add('owner') - return roles - @classmethod def migrate_account(cls, old_account: Account, new_account: Account) -> None: """Migrate one account's data to another when merging accounts.""" diff --git a/funnel/models/sync_ticket.py b/funnel/models/sync_ticket.py index d07b1e313..d75c4f647 100644 --- a/funnel/models/sync_ticket.py +++ b/funnel/models/sync_ticket.py @@ -4,10 +4,10 @@ import base64 import os -from collections.abc import Iterable, Sequence +from collections.abc import Iterable from typing import TYPE_CHECKING, Any, Self -from coaster.sqlalchemy import LazyRoleSet, with_roles +from coaster.sqlalchemy import with_roles from .account import Account, AccountEmail from .base import ( @@ -258,8 +258,8 @@ class TicketParticipant( participant_id: Mapped[int | None] = sa_orm.mapped_column( sa.ForeignKey('account.id'), default=None, nullable=True ) - participant: Mapped[Account | None] = relationship( - back_populates='ticket_participants' + participant: Mapped[Account | None] = with_roles( + relationship(back_populates='ticket_participants'), grants={'member'} ) project_id: Mapped[int] = sa_orm.mapped_column( sa.ForeignKey('project.id'), default=None, nullable=False @@ -270,8 +270,9 @@ class TicketParticipant( grants_via={None: project_child_role_map}, ) - scanned_contacts: Mapped[ContactExchange] = relationship( - passive_deletes=True, back_populates='ticket_participant' + scanned_contacts: Mapped[ContactExchange] = with_roles( + relationship(passive_deletes=True, back_populates='ticket_participant'), + grants_via={'account': {'scanner'}}, ) ticket_events: Mapped[list[TicketEvent]] = relationship( @@ -295,18 +296,6 @@ class TicketParticipant( 'scanner': {'read': {'email'}}, } - 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.participant: - roles.add('member') - cx = db.session.get(ContactExchange, (actor.id, self.id)) - if cx is not None: - roles.add('scanner') - return roles - @property def avatar(self) -> str: return ( diff --git a/funnel/models/update.py b/funnel/models/update.py index 4bd3a6db4..c03436d30 100644 --- a/funnel/models/update.py +++ b/funnel/models/update.py @@ -7,7 +7,7 @@ from typing import Any, Self from baseframe import __ -from coaster.sqlalchemy import LazyRoleSet, StateManager, auto_init_default, with_roles +from coaster.sqlalchemy import StateManager, auto_init_default, role_check, with_roles from coaster.utils import LabeledEnum from .account import Account @@ -31,7 +31,7 @@ ) from .project import Project -__all__ = ['Update'] +__all__ = ['Update', 'VISIBILITY_STATE'] class UPDATE_STATE(LabeledEnum): # noqa: N801 @@ -42,12 +42,15 @@ class UPDATE_STATE(LabeledEnum): # noqa: N801 class VISIBILITY_STATE(LabeledEnum): # noqa: N801 PUBLIC = (1, 'public', __("Public")) - RESTRICTED = (2, 'restricted', __("Restricted")) + PARTICIPANTS = (2, 'participants', __("Participants")) + MEMBERS = (3, 'members', __("Members")) class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): __tablename__ = 'update' + # FIXME: Why is this a state? There's no state change in the product design. + # It's a permanent subtype identifier _visibility_state: Mapped[int] = sa_orm.mapped_column( 'visibility_state', sa.SmallInteger, @@ -93,45 +96,30 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): datasets={'primary'}, grants_via={ None: { + 'reader': {'project_reader'}, # Project reader is NOT update reader 'editor': {'editor', 'project_editor'}, - 'participant': {'reader', 'project_participant'}, - 'crew': {'reader', 'project_crew'}, + 'participant': {'project_participant'}, + 'account_follower': {'account_follower'}, + 'account_member': {'account_member'}, + 'member_participant': {'member_participant'}, + 'crew': {'project_crew', 'reader'}, } }, ) parent: Mapped[Project] = sa_orm.synonym('project') - # Relationship to project that exists only when the Update is not restricted, for - # the purpose of inheriting the account_participant role. We do this because - # RoleMixin does not have a mechanism for conditional grant of roles. A relationship - # marked as `grants_via` will always grant the role unconditionally, so the only - # control at the moment is to make the relationship itself conditional. The affected - # mechanism is not `roles_for` but `actors_with`, which is currently not meant to be - # redefined in a subclass - _project_when_unrestricted: Mapped[Project] = with_roles( - relationship( - viewonly=True, - uselist=False, - primaryjoin=sa.and_( - project_id == Project.id, _visibility_state == VISIBILITY_STATE.PUBLIC - ), - ), - grants_via={None: {'account_participant': 'account_participant'}}, - ) - body, body_text, body_html = MarkdownCompositeDocument.create( 'body', nullable=False ) - #: Update number, for Project updates, assigned when the update is published + #: Update serial number, only assigned when the update is published number: Mapped[int | None] = with_roles( sa_orm.mapped_column(default=None), read={'all'} ) - #: Like pinned tweets. You can keep posting updates, - #: but might want to pin an update from a week ago. + #: Pin an update above future updates is_pinned: Mapped[bool] = with_roles( - sa_orm.mapped_column(default=False), read={'all'} + sa_orm.mapped_column(default=False), read={'all'}, write={'editor'} ) published_by_id: Mapped[int | None] = sa_orm.mapped_column( @@ -185,6 +173,8 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 'body_text', weights={'name': 'A', 'title': 'A', 'body_text': 'B'}, regconfig='english', + # FIXME: Search preview will give partial access to Update.body even if the + # user does not have the necessary 'reader' role hltext=lambda: sa.func.concat_ws( visual_field_delimiter, Update.title, Update.body_html ), @@ -198,6 +188,8 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 'read': {'name', 'title', 'urls'}, 'call': {'features', 'visibility_state', 'state', 'url_for'}, }, + 'project_crew': {'read': {'body'}}, + 'editor': {'write': {'title', 'body'}, 'read': {'body'}}, 'reader': {'read': {'body'}}, } @@ -213,10 +205,8 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 'edited_at', 'created_by', 'is_pinned', - 'is_restricted', 'is_currently_restricted', - 'visibility_label', - 'state_label', + 'visibility', 'urls', 'uuid_b58', }, @@ -231,10 +221,8 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 'edited_at', 'created_by', 'is_pinned', - 'is_restricted', 'is_currently_restricted', - 'visibility_label', - 'state_label', + 'visibility', 'urls', 'uuid_b58', }, @@ -249,17 +237,47 @@ def __repr__(self) -> str: """Represent :class:`Update` as a string.""" return f'' - @property - def visibility_label(self) -> str: - return self.visibility_state.label.title - - with_roles(visibility_label, read={'all'}) + @role_check('reader') + def has_reader_role( + self, actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Check if the given actor is a reader based on the Update's visibility.""" + if not self.state.PUBLISHED: + # Update must be published to allow anyone other than crew to read + return False + if self.visibility_state.PUBLIC: + return True + roles = self.roles_for(actor) + if self.visibility_state.PARTICIPANTS: + return 'project_participant' in roles + if self.visibility_state.MEMBERS: + return 'account_member' in roles + + raise RuntimeError("This update has an unexpected state") + + # 'reader' is a non-enumerated role, like `all`, `auth` and `anon` @property - def state_label(self) -> str: - return self.state.label.title - - with_roles(state_label, read={'all'}) + def visibility(self) -> str: + """Return visibility state name.""" + return self.visibility_state.label.name + + @visibility.setter + def visibility(self, value: str) -> None: + """Set visibility state (interim until visibility as state is resolved).""" + # FIXME: Move to using an Enum so we don't reproduce the enumeration here + match value: + case 'public': + vstate = VISIBILITY_STATE.PUBLIC + case 'participants': + vstate = VISIBILITY_STATE.PARTICIPANTS + case 'members': + vstate = VISIBILITY_STATE.MEMBERS + case _: + raise ValueError("Unknown visibility state") + self._visibility_state = vstate # type: ignore[assignment] + + with_roles(visibility, read={'all'}, write={'editor'}) state.add_conditional_state( 'UNPUBLISHED', @@ -311,51 +329,17 @@ def delete(self, actor: Account) -> None: @with_roles(call={'editor'}) @state.transition(state.DELETED, state.DRAFT) - def undo_delete(self) -> None: + def undelete(self) -> None: self.deleted_by = None self.deleted_at = None - @with_roles(call={'editor'}) - @visibility_state.transition(visibility_state.RESTRICTED, visibility_state.PUBLIC) - def make_public(self) -> None: - pass - - @with_roles(call={'editor'}) - @visibility_state.transition(visibility_state.PUBLIC, visibility_state.RESTRICTED) - def make_restricted(self) -> None: - pass - - @property - def is_restricted(self) -> bool: - return bool(self.visibility_state.RESTRICTED) - - @is_restricted.setter - def is_restricted(self, value: bool) -> None: - if value and self.visibility_state.PUBLIC: - self.make_restricted() - elif not value and self.visibility_state.RESTRICTED: - self.make_public() - - with_roles(is_restricted, read={'all'}) - @property def is_currently_restricted(self) -> bool: - return self.is_restricted and not self.current_roles.reader + """Check if this update is not available for the current user.""" + return not self.current_roles.reader with_roles(is_currently_restricted, read={'all'}) - def roles_for( - self, actor: Account | None = None, anchors: Sequence = () - ) -> LazyRoleSet: - roles = super().roles_for(actor, anchors) - if not self.visibility_state.RESTRICTED: - # Everyone gets reader role when the post is not restricted. - # If it is, 'reader' must be mapped from 'participant' in the project, - # specified above in the grants_via annotation on project. - roles.add('reader') - - return roles - @classmethod def all_published_public(cls) -> Query[Self]: return cls.query.join(Project).filter( diff --git a/funnel/templates/js/update.js.jinja2 b/funnel/templates/js/update.js.jinja2 index e93572c62..7d5972905 100644 --- a/funnel/templates/js/update.js.jinja2 +++ b/funnel/templates/js/update.js.jinja2 @@ -26,7 +26,10 @@