From d0d8dbbdab793c9848f57f4daf533ed37dc4626f Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Tue, 12 Mar 2024 15:02:03 +0530 Subject: [PATCH 01/25] Add member-only visibility for project updates --- funnel/forms/update.py | 24 ++- funnel/models/__init__.py | 1 + funnel/models/__init__.pyi | 3 +- funnel/models/notification_types.py | 2 +- funnel/models/update.py | 141 ++++++++++-------- funnel/templates/js/update.js.jinja2 | 5 +- funnel/views/update.py | 2 +- ...20b31eb6_update_visibility_state_change.py | 50 +++++++ 8 files changed, 158 insertions(+), 70 deletions(-) create mode 100644 migrations/versions/0afd20b31eb6_update_visibility_state_change.py diff --git a/funnel/forms/update.py b/funnel/forms/update.py index 9fcb4b2cd..e1648467a 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,23 @@ 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?"), + default=VISIBILITY_STATE[VISIBILITY_STATE.PUBLIC].name, + choices=[ + ( + VISIBILITY_STATE[VISIBILITY_STATE.PUBLIC].name, + __("Public; account followers will be notified"), + ), + ( + VISIBILITY_STATE[VISIBILITY_STATE.PARTICIPANTS].name, + __("Only project participants"), + ), + ( + VISIBILITY_STATE[VISIBILITY_STATE.MEMBERS].name, + __("Only account members"), + ), + ], ) diff --git a/funnel/models/__init__.py b/funnel/models/__init__.py index cd27c13a9..142a55990 100644 --- a/funnel/models/__init__.py +++ b/funnel/models/__init__.py @@ -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..84b8aa0f3 100644 --- a/funnel/models/__init__.pyi +++ b/funnel/models/__init__.pyi @@ -228,7 +228,7 @@ from .typing import ( ModelUrlProtocol, ModelUuidProtocol, ) -from .update import Update +from .update import VISIBILITY_STATE, Update from .utils import ( AccountAndAnchor, IncompleteUserMigrationError, @@ -394,6 +394,7 @@ __all__ = [ "UrlType", "User", "UuidMixin", + "VISIBILITY_STATE", "Venue", "VenueRoom", "VideoError", diff --git a/funnel/models/notification_types.py b/funnel/models/notification_types.py index 3ba50ba9b..64443ba1a 100644 --- a/funnel/models/notification_types.py +++ b/funnel/models/notification_types.py @@ -122,7 +122,7 @@ class NewUpdateNotification( "Typically contains critical information such as video conference links" ) - roles = ['project_crew', 'project_participant', 'account_participant'] + roles = ['project_crew', 'reader'] exclude_actor = False # Send to everyone including the actor diff --git a/funnel/models/update.py b/funnel/models/update.py index 4bd3a6db4..a7ceb9954 100644 --- a/funnel/models/update.py +++ b/funnel/models/update.py @@ -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' + # XXX: Why is this a state? There's no state change in the product design -- it's + # more of a permanent subtype identifier _visibility_state: Mapped[int] = sa_orm.mapped_column( 'visibility_state', sa.SmallInteger, @@ -94,21 +97,21 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): grants_via={ None: { 'editor': {'editor', 'project_editor'}, - 'participant': {'reader', 'project_participant'}, - 'crew': {'reader', 'project_crew'}, + 'participant': {'project_participant'}, + 'crew': {'project_crew'}, } }, ) 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( + # Relationships to project that exist only when the Update has specific visibility + # states, for the purpose of inheriting the `reader` 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_public: Mapped[Project] = with_roles( relationship( viewonly=True, uselist=False, @@ -116,7 +119,34 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): project_id == Project.id, _visibility_state == VISIBILITY_STATE.PUBLIC ), ), - grants_via={None: {'account_participant': 'account_participant'}}, + grants_via={ + None: { + 'account_participant': 'reader', + 'project_participant': 'reader', + 'account_member': 'reader', + } + }, + ) + _project_when_participants_only: Mapped[Project] = with_roles( + relationship( + viewonly=True, + uselist=False, + primaryjoin=sa.and_( + project_id == Project.id, + _visibility_state == VISIBILITY_STATE.PARTICIPANTS, + ), + ), + grants_via={None: {'project_participant': 'reader'}}, + ) + _project_when_members_only: Mapped[Project] = with_roles( + relationship( + viewonly=True, + uselist=False, + primaryjoin=sa.and_( + project_id == Project.id, _visibility_state == VISIBILITY_STATE.MEMBERS + ), + ), + grants_via={None: {'account_member': 'reader'}}, ) body, body_text, body_html = MarkdownCompositeDocument.create( @@ -128,10 +158,10 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 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. + #: Like pinned tweets. You can keep posting updates, but might want to pin an + #: update from a week ago 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( @@ -198,7 +228,9 @@ class Update(UuidMixin, BaseScopedIdNameMixin[int, Account], Model): 'read': {'name', 'title', 'urls'}, 'call': {'features', 'visibility_state', 'state', 'url_for'}, }, + 'editor': {'write': {'title', 'body'}, 'read': {'body'}}, 'reader': {'read': {'body'}}, + 'project_crew': {'read': {'body'}}, } __datasets__ = { @@ -213,10 +245,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 +261,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', }, @@ -250,16 +278,26 @@ def __repr__(self) -> str: return f'' @property - def visibility_label(self) -> str: - return self.visibility_state.label.title - - with_roles(visibility_label, read={'all'}) - - @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', @@ -315,32 +353,16 @@ def undo_delete(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.""" + if self.visibility_state.PUBLIC or self.current_roles.project_editor: + return False + if self.visibility_state.PARTICIPANTS: + return self.current_roles.project_participant + if self.visibility_state.MEMBERS: + return self.current_roles.account_member + raise RuntimeError("Unknown visibility state") # pragma: no cover with_roles(is_currently_restricted, read={'all'}) @@ -348,10 +370,9 @@ 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. + if self.visibility_state.PUBLIC: + # Everyone gets 'reader' role when the update is public. + # TODO: Acquire it from the project instead roles.add('reader') return roles 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 @@