Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add member-only visibility for project updates #1988

Merged
merged 27 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d0d8dbb
Add member-only visibility for project updates
jace Mar 12, 2024
82cb6e1
Don't allow visibility change in a published update
jace Mar 12, 2024
dba34d3
Fix role check
jace Mar 12, 2024
3ef9d6c
Use ConditionalRole in Update; add new 'recipient' role alongside 're…
jace Mar 18, 2024
078a607
Test for 'reader' role granted to all in a public update
jace Mar 18, 2024
727a5b8
Fix separation between owner and admin in Account roles; better Updat…
jace Mar 18, 2024
1afda48
Added update notification template for update in project
djamg Mar 18, 2024
df5c8c9
Participant update access test
jace Mar 18, 2024
dfb0dda
Redo Update 'recipient' role and prepare for 'account_follower' role
jace Mar 22, 2024
799f9f6
Allow a notification to target roles based on document/fragment state
jace Mar 25, 2024
b9c298c
Migrate update notification and fix templates
jace Mar 25, 2024
67aeb12
Draft feature specs for project updates
jace Mar 25, 2024
a237873
Support rolled-up ProjectUpdateNotification; place members above part…
jace Mar 25, 2024
58c7128
Merge branch 'main' into update-for-members
jace Mar 25, 2024
02020ff
Merge branch 'main' into update-for-members
jace Mar 25, 2024
aecc9b9
Replace all custom `roles_for` implementations with `role_check` or `…
jace Mar 25, 2024
8d41188
Add scenarios for posting public updates
zainabbawa Mar 25, 2024
a541eb8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 25, 2024
ffb5393
Update has `published_at` for timestamp
jace Mar 26, 2024
de44da5
Inherit account_member role
jace Mar 26, 2024
0f15d6b
Switch SMS templates based on account+project titles
jace Mar 26, 2024
f0b265e
Migrate notification preferences for the notification type
jace Mar 26, 2024
558badc
Rename Notification.roles to dispatch_roles for clarity
jace Mar 26, 2024
f5ee2ca
Add anchors to all role_check functions
jace Mar 26, 2024
1fbe809
Added tests to check member roles
djamg Mar 26, 2024
78dd7d3
Remove extra space
jace Mar 26, 2024
1ad6fc4
Formatting fixes
jace Mar 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 19 additions & 6 deletions funnel/forms/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from baseframe import __, forms

from ..models import Update
from ..models import VISIBILITY_STATE, Update

__all__ = ['UpdateForm', 'UpdatePinForm']

Expand All @@ -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"),
),
],
)


Expand Down
3 changes: 2 additions & 1 deletion funnel/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@
"ModelUrlProtocol",
"ModelUuidProtocol",
"NewCommentNotification",
"NewUpdateNotification",
"NoIdMixin",
"Notification",
"NotificationFor",
Expand Down Expand Up @@ -130,6 +129,7 @@
"ProjectRsvpStateEnum",
"ProjectSponsorMembership",
"ProjectStartingNotification",
"ProjectUpdateNotification",
"Proposal",
"ProposalLabelProxy",
"ProposalLabelProxyWrapper",
Expand Down Expand Up @@ -170,6 +170,7 @@
"UrlType",
"User",
"UuidMixin",
"VISIBILITY_STATE",
"Venue",
"VenueRoom",
"VideoError",
Expand Down
7 changes: 4 additions & 3 deletions funnel/models/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ from .notification_types import (
CommentReplyNotification,
CommentReportReceivedNotification,
NewCommentNotification,
NewUpdateNotification,
OrganizationAdminMembershipNotification,
OrganizationAdminMembershipRevokedNotification,
ProjectCrewMembershipNotification,
ProjectCrewMembershipRevokedNotification,
ProjectStartingNotification,
ProjectUpdateNotification,
ProposalReceivedNotification,
ProposalSubmittedNotification,
RegistrationCancellationNotification,
Expand Down Expand Up @@ -228,7 +228,7 @@ from .typing import (
ModelUrlProtocol,
ModelUuidProtocol,
)
from .update import Update
from .update import VISIBILITY_STATE, Update
from .utils import (
AccountAndAnchor,
IncompleteUserMigrationError,
Expand Down Expand Up @@ -322,7 +322,6 @@ __all__ = [
"ModelUrlProtocol",
"ModelUuidProtocol",
"NewCommentNotification",
"NewUpdateNotification",
"NoIdMixin",
"Notification",
"NotificationFor",
Expand Down Expand Up @@ -354,6 +353,7 @@ __all__ = [
"ProjectRsvpStateEnum",
"ProjectSponsorMembership",
"ProjectStartingNotification",
"ProjectUpdateNotification",
"Proposal",
"ProposalLabelProxy",
"ProposalLabelProxyWrapper",
Expand Down Expand Up @@ -394,6 +394,7 @@ __all__ = [
"UrlType",
"User",
"UuidMixin",
"VISIBILITY_STATE",
"Venue",
"VenueRoom",
"VideoError",
Expand Down
56 changes: 28 additions & 28 deletions funnel/models/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]:
"""
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 8 additions & 7 deletions funnel/models/account_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand All @@ -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
36 changes: 21 additions & 15 deletions funnel/models/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')
Expand Down
24 changes: 8 additions & 16 deletions funnel/models/contact_exchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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."""
Expand Down
Loading
Loading