diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 768962e6f..ee3791cd5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: '--remove-duplicate-keys', ] - repo: https://github.com/pre-commit/mirrors-isort - rev: v5.7.0 + rev: v5.8.0 hooks: - id: isort additional_dependencies: @@ -56,12 +56,12 @@ repos: - id: requirements-txt-fixer - id: trailing-whitespace - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.790 + rev: v0.812 hooks: - id: mypy additional_dependencies: [sqlalchemy-stubs==0.4] - repo: https://gitlab.com/pycqa/flake8 - rev: 3.8.4 + rev: 3.9.1 hooks: - id: flake8 additional_dependencies: diff --git a/funnel/models/__init__.py b/funnel/models/__init__.py index 7ea2f15cd..514ca25b2 100644 --- a/funnel/models/__init__.py +++ b/funnel/models/__init__.py @@ -50,6 +50,7 @@ from .membership_mixin import * # isort:skip from .organization_membership import * # isort:skip from .project_membership import * # isort:skip +from .sponsor_membership import * # isort:skip from .proposal_membership import * # isort:skip from .site_membership import * # isort:skip from .moderation import * # isort:skip diff --git a/funnel/models/commentset_membership.py b/funnel/models/commentset_membership.py index bcf0c7bb0..03c041934 100644 --- a/funnel/models/commentset_membership.py +++ b/funnel/models/commentset_membership.py @@ -1,3 +1,5 @@ +from typing import Set + from werkzeug.utils import cached_property from coaster.sqlalchemy import immutable, with_roles @@ -5,12 +7,12 @@ from . import User, db from .commentvote import Commentset from .helpers import reopen -from .membership_mixin import ImmutableMembershipMixin +from .membership_mixin import ImmutableUserMembershipMixin __all__ = ['CommentsetMembership'] -class CommentsetMembership(ImmutableMembershipMixin, db.Model): +class CommentsetMembership(ImmutableUserMembershipMixin, db.Model): """Membership roles for users who are commentset users and subscribers.""" __tablename__ = 'commentset_membership' @@ -57,7 +59,7 @@ class CommentsetMembership(ImmutableMembershipMixin, db.Model): ) @cached_property - def offered_roles(self): + def offered_roles(self) -> Set[str]: """ Roles offered by this membership record. diff --git a/funnel/models/membership_mixin.py b/funnel/models/membership_mixin.py index 72588f3a5..b1f146c16 100644 --- a/funnel/models/membership_mixin.py +++ b/funnel/models/membership_mixin.py @@ -15,6 +15,7 @@ from ..typing import OptionalMigratedTables from . import BaseMixin, UuidMixin, db +from .profile import Profile from .user import User __all__ = [ @@ -53,6 +54,8 @@ class ImmutableMembershipMixin(UuidMixin, BaseMixin): """Support class for immutable memberships.""" __uuid_primary_key__ = True + #: Can granted_by be null? Only in memberships based on legacy data + __null_granted_by__ = False #: List of columns that will be copied into a new row when a membership is amended __data_columns__: Iterable[str] = () #: Parent column (declare as synonym of 'profile_id' or 'project_id' in subclasses) @@ -74,7 +77,7 @@ class ImmutableMembershipMixin(UuidMixin, BaseMixin): read={'subject', 'editor'}, ) #: Record type - record_type = immutable( + record_type: db.Column = immutable( with_roles( db.Column( db.Integer, @@ -86,23 +89,6 @@ class ImmutableMembershipMixin(UuidMixin, BaseMixin): ) ) - # mypy type declaration - user_id: db.Column - - @declared_attr # type: ignore[no-redef] - def user_id(cls): # skipcq: PYL-E0102 - return db.Column( - None, - db.ForeignKey('user.id', ondelete='CASCADE'), - nullable=False, - index=True, - ) - - @with_roles(read={'subject', 'editor'}, grants={'subject'}) - @declared_attr - def user(cls): - return immutable(db.relationship(User, foreign_keys=[cls.user_id])) - @declared_attr def revoked_by_id(cls): """Id of user who revoked the membership.""" @@ -125,7 +111,9 @@ def granted_by_id(cls): for granted_by. """ return db.Column( - None, db.ForeignKey('user.id', ondelete='SET NULL'), nullable=True + None, + db.ForeignKey('user.id', ondelete='SET NULL'), + nullable=cls.__null_granted_by__, ) @with_roles(read={'subject', 'editor'}, grants={'editor'}) @@ -154,42 +142,8 @@ def is_active(cls): # NOQA: N805 def is_invite(self) -> bool: return self.record_type == MEMBERSHIP_RECORD_TYPE.INVITE - @with_roles(read={'subject', 'editor'}) - @hybrid_property - def is_self_granted(self) -> bool: - """Return True if the subject of this record is also the granting actor.""" - return self.user_id == self.granted_by_id - - @with_roles(read={'subject', 'editor'}) - @hybrid_property - def is_self_revoked(self) -> bool: - """Return True if the subject of this record is also the revoking actor.""" - return self.user_id == self.revoked_by_id - - @declared_attr - def __table_args__(cls): - if cls.parent_id is not None: - return ( - db.Index( - 'ix_' + cls.__tablename__ + '_active', - cls.parent_id.name, - 'user_id', - unique=True, - postgresql_where=db.text('revoked_at IS NULL'), - ), - ) - else: - return ( - db.Index( - 'ix_' + cls.__tablename__ + '_active', - 'user_id', - unique=True, - postgresql_where=db.text('revoked_at IS NULL'), - ), - ) - @cached_property - def offered_roles(self) -> Set: + def offered_roles(self) -> Set[str]: """Return roles offered by this membership record.""" return set() @@ -205,9 +159,13 @@ def revoke(self, actor: User) -> None: self.revoked_at = db.func.utcnow() self.revoked_by = actor + def copy_template(self: MembershipType, **kwargs) -> MembershipType: + """Make a copy of self for customization.""" + raise NotImplementedError("Subclasses must implement copy_template") + @with_roles(call={'editor'}) def replace( - self, actor: User, accept=False, **roles: Dict[str, Any] + self: MembershipType, actor: User, accept=False, **roles: Dict[str, Any] ) -> MembershipType: """Replace this membership record with changes to role columns.""" if self.revoked_at is not None: @@ -237,9 +195,7 @@ def replace( self.revoked_at = db.func.utcnow() self.revoked_by = actor - new = type(self)( - user=self.user, parent_id=self.parent_id, granted_by=self.granted_by - ) + new = self.copy_template(parent_id=self.parent_id, granted_by=self.granted_by) # if existing record type is INVITE, then ACCEPT or amend as new INVITE # else replace it with AMEND @@ -259,7 +215,9 @@ def replace( db.session.add(new) return new - def merge_and_replace(self, actor: User, other: MembershipType) -> MembershipType: + def merge_and_replace( + self: MembershipType, actor: User, other: MembershipType + ) -> MembershipType: """Replace this record by merging roles from an independent record.""" if self.__class__ is not other.__class__: raise TypeError("Merger requires membership records of the same type") @@ -293,14 +251,72 @@ def merge_and_replace(self, actor: User, other: MembershipType) -> MembershipTyp return replacement @with_roles(call={'subject'}) - def accept(self, actor: User) -> MembershipType: + def accept(self: MembershipType, actor: User) -> MembershipType: """Accept a membership invitation.""" if self.record_type != MEMBERSHIP_RECORD_TYPE.INVITE: raise MembershipRecordTypeError("This membership record is not an invite") - if actor != self.user: + if 'subject' not in self.roles_for(actor): raise ValueError("Invite must be accepted by the invited user") return self.replace(actor, accept=True) + +class ImmutableUserMembershipMixin(ImmutableMembershipMixin): + """Support class for immutable memberships for users.""" + + # mypy type declaration + user_id: db.Column + + @declared_attr # type: ignore[no-redef] + def user_id(cls): # skipcq: PYL-E0102 + return db.Column( + None, + db.ForeignKey('user.id', ondelete='CASCADE'), + nullable=False, + index=True, + ) + + @with_roles(read={'subject', 'editor'}, grants={'subject'}) + @declared_attr + def user(cls): + return immutable(db.relationship(User, foreign_keys=[cls.user_id])) + + @declared_attr + def __table_args__(cls): + if cls.parent_id is not None: + return ( + db.Index( + 'ix_' + cls.__tablename__ + '_active', + cls.parent_id.name, + 'user_id', + unique=True, + postgresql_where=db.text('revoked_at IS NULL'), + ), + ) + else: + return ( + db.Index( + 'ix_' + cls.__tablename__ + '_active', + 'user_id', + unique=True, + postgresql_where=db.text('revoked_at IS NULL'), + ), + ) + + @with_roles(read={'subject', 'editor'}) + @hybrid_property + def is_self_granted(self) -> bool: + """Return True if the subject of this record is also the granting actor.""" + return self.user_id == self.granted_by_id + + @with_roles(read={'subject', 'editor'}) + @hybrid_property + def is_self_revoked(self) -> bool: + """Return True if the subject of this record is also the revoking actor.""" + return self.user_id == self.revoked_by_id + + def copy_template(self: MembershipType, **kwargs) -> MembershipType: + return type(self)(user=self.user, **kwargs) + @classmethod def migrate_user(cls, old_user: User, new_user: User) -> OptionalMigratedTables: """ @@ -343,5 +359,117 @@ def migrate_user(cls, old_user: User, new_user: User) -> OptionalMigratedTables: cls.query.filter(cls.user == old_user).update( {'user_id': new_user.id}, synchronize_session=False ) + # Also update the revoked_by and granted_by user accounts + cls.query.filter(cls.revoked_by == old_user).update( + {'revoked_by_id': new_user.id}, synchronize_session=False + ) + cls.query.filter(cls.granted_by == old_user).update( + {'granted_by_id': new_user.id}, synchronize_session=False + ) + db.session.flush() + return None + + +class ImmutableProfileMembershipMixin(ImmutableMembershipMixin): + """Support class for immutable memberships for profiles.""" + + # mypy type declaration + profile_id: db.Column + + @declared_attr # type: ignore[no-redef] + def profile_id(cls): # skipcq: PYL-E0102 + return db.Column( + None, + db.ForeignKey('profile.id', ondelete='CASCADE'), + nullable=False, + index=True, + ) + + @declared_attr + def __table_args__(cls): + if cls.parent_id is not None: + return ( + db.Index( + 'ix_' + cls.__tablename__ + '_active', + cls.parent_id.name, + 'profile_id', + unique=True, + postgresql_where=db.text('revoked_at IS NULL'), + ), + ) + else: + return ( + db.Index( + 'ix_' + cls.__tablename__ + '_active', + 'profile_id', + unique=True, + postgresql_where=db.text('revoked_at IS NULL'), + ), + ) + + @with_roles(read={'subject', 'editor'}) + @hybrid_property + def is_self_granted(self) -> bool: + """Return True if the subject of this record is also the granting actor.""" + return 'subject' in self.roles_for(self.granted_by) + + @with_roles(read={'subject', 'editor'}) + @hybrid_property + def is_self_revoked(self) -> bool: + """Return True if the subject of this record is also the revoking actor.""" + return 'subject' in self.roles_for(self.revoked_by) + + def copy_template(self: MembershipType, **kwargs) -> MembershipType: + return type(self)(profile=self.profile, **kwargs) + + @with_roles(read={'subject', 'editor'}, grants_via={None: {'admin': 'subject'}}) + @declared_attr + def profile(cls): + return immutable(db.relationship(Profile, foreign_keys=[cls.profile_id])) + + @classmethod + def migrate_profile( + cls, old_profile: Profile, new_profile: Profile + ) -> OptionalMigratedTables: + """ + Migrate memberhip records from one profile to another. + + If both profiles have active records, they are merged into a new record in the + new profile's favour. All revoked records for the old profile are transferred to + the new profile. + """ + # Look up all active membership records of the subclass's type for the old + # profile. `cls` here represents the subclass. + old_profile_records = cls.query.filter( + cls.profile == old_profile, cls.revoked_at.is_(None) + ).all() + # Look up all conflicting memberships for the new profile. Limit lookups by + # parent except when the membership type doesn't have a parent. + if cls.parent_id is not None: + new_profile_records = cls.query.filter( + cls.profile == new_profile, + cls.revoked_at.is_(None), + cls.parent_id.in_([r.parent_id for r in old_profile_records]), + ).all() + else: + new_profile_records = cls.query.filter( + cls.profile == new_profile, + cls.revoked_at.is_(None), + ).all() + new_profile_records_by_parent = {r.parent_id: r for r in new_profile_records} + + for record in old_profile_records: + if record.parent_id in new_profile_records_by_parent: + # Where there is a conflict, merge the records + new_profile_records_by_parent[record.parent_id].merge_and_replace( + new_profile, record + ) + db.session.flush() + + # Transfer all revoked records and non-conflicting active records. At this point + # no filter is necessary as the conflicting records have all been merged. + cls.query.filter(cls.profile == old_profile).update( + {'profile_id': new_profile.id}, synchronize_session=False + ) db.session.flush() return None diff --git a/funnel/models/organization_membership.py b/funnel/models/organization_membership.py index d2b005083..9a39d920c 100644 --- a/funnel/models/organization_membership.py +++ b/funnel/models/organization_membership.py @@ -1,16 +1,18 @@ +from typing import Set + from werkzeug.utils import cached_property from coaster.sqlalchemy import DynamicAssociationProxy, immutable, with_roles from . import db from .helpers import reopen -from .membership_mixin import ImmutableMembershipMixin +from .membership_mixin import ImmutableUserMembershipMixin from .user import Organization, User __all__ = ['OrganizationMembership'] -class OrganizationMembership(ImmutableMembershipMixin, db.Model): +class OrganizationMembership(ImmutableUserMembershipMixin, db.Model): """ A user can be an administrator of an organization and optionally an owner. @@ -21,7 +23,10 @@ class OrganizationMembership(ImmutableMembershipMixin, db.Model): __tablename__ = 'organization_membership' - # List of role columns in this model + # Legacy data has no granted_by + __null_granted_by__ = True + + #: List of role columns in this model __data_columns__ = ('is_owner',) __roles__ = { @@ -78,7 +83,7 @@ class OrganizationMembership(ImmutableMembershipMixin, db.Model): is_owner = immutable(db.Column(db.Boolean, nullable=False, default=False)) @cached_property - def offered_roles(self): + def offered_roles(self) -> Set[str]: """Roles offered by this membership record.""" roles = {'admin'} if self.is_owner: diff --git a/funnel/models/project_membership.py b/funnel/models/project_membership.py index 68c01eb13..bd2a8a0f3 100644 --- a/funnel/models/project_membership.py +++ b/funnel/models/project_membership.py @@ -8,7 +8,7 @@ from . import db from .helpers import reopen -from .membership_mixin import ImmutableMembershipMixin +from .membership_mixin import ImmutableUserMembershipMixin from .project import Project from .user import User @@ -24,12 +24,15 @@ } -class ProjectCrewMembership(ImmutableMembershipMixin, db.Model): +class ProjectCrewMembership(ImmutableUserMembershipMixin, db.Model): """Users can be crew members of projects, with specified access rights.""" __tablename__ = 'project_crew_membership' - # List of is_role columns in this model + #: Legacy data has no granted_by + __null_granted_by__ = True + + #: List of is_role columns in this model __data_columns__ = ('is_editor', 'is_promoter', 'is_usher') __roles__ = { @@ -110,7 +113,7 @@ def __table_args__(cls): return tuple(args) @cached_property - def offered_roles(self): + def offered_roles(self) -> Set[str]: """Roles offered by this membership record.""" roles = set() if self.is_editor: diff --git a/funnel/models/proposal_membership.py b/funnel/models/proposal_membership.py index 7761bc351..4742a6111 100644 --- a/funnel/models/proposal_membership.py +++ b/funnel/models/proposal_membership.py @@ -1,3 +1,5 @@ +from typing import Set + from sqlalchemy.ext.declarative import declared_attr from werkzeug.utils import cached_property @@ -6,14 +8,14 @@ from . import db from .helpers import reopen -from .membership_mixin import ImmutableMembershipMixin +from .membership_mixin import ImmutableUserMembershipMixin from .proposal import Proposal from .user import User __all__ = ['ProposalMembership'] -class ProposalMembership(ImmutableMembershipMixin, db.Model): +class ProposalMembership(ImmutableUserMembershipMixin, db.Model): """Users can be presenters or reviewers on proposals.""" __tablename__ = 'proposal_membership' @@ -81,7 +83,7 @@ def __table_args__(cls): return tuple(args) @cached_property - def offered_roles(self): + def offered_roles(self) -> Set[str]: """Roles offered by this membership record.""" roles = set() if self.is_reviewer: diff --git a/funnel/models/reorder_mixin.py b/funnel/models/reorder_mixin.py index 54cccced4..b868138aa 100644 --- a/funnel/models/reorder_mixin.py +++ b/funnel/models/reorder_mixin.py @@ -31,6 +31,18 @@ class ReorderMixin: #: Subclass must offer a SQLAlchemy query (this is standard from base classes) query: Query + @property + def parent_scoped_reorder_query_filter(self: Reorderable): + """ + Return a query filter that includes a scope limitation to the parent. + + Used alongside the :attr:`seq` column to retrieve a sequence value. Subclasses + may need to override if they have additional criteria relative to the parent, + such as needing to exclude revoked membership records. + """ + cls = self.__class__ + return cls.parent_id == self.parent_id + def reorder_item(self: Reorderable, other: Reorderable, before: bool) -> None: """Reorder self before or after other item.""" cls = self.__class__ @@ -60,7 +72,7 @@ def reorder_item(self: Reorderable, other: Reorderable, before: bool) -> None: items_to_reorder = ( cls.query.filter( - cls.parent_id == self.parent_id, + self.parent_scoped_reorder_query_filter, cls.seq >= min(self.seq, other.seq), cls.seq <= max(self.seq, other.seq), ) @@ -82,9 +94,8 @@ def reorder_item(self: Reorderable, other: Reorderable, before: bool) -> None: new_seq_number = self.seq # Temporarily give self an out-of-bounds number - self.seq = db.select( - [db.func.coalesce(db.func.max(cls.seq) + 1, 1)], - cls.parent_id == self.parent_id, + self.seq = db.select([db.func.coalesce(db.func.max(cls.seq) + 1, 1)]).where( + self.parent_scoped_reorder_query_filter ) # Flush it so the db doesn't complain when there's a unique constraint db.session.flush() diff --git a/funnel/models/site_membership.py b/funnel/models/site_membership.py index 4d8d6961a..03e4469b6 100644 --- a/funnel/models/site_membership.py +++ b/funnel/models/site_membership.py @@ -1,15 +1,17 @@ +from typing import Set + from sqlalchemy.ext.declarative import declared_attr from werkzeug.utils import cached_property from . import User, db from .helpers import reopen -from .membership_mixin import ImmutableMembershipMixin +from .membership_mixin import ImmutableUserMembershipMixin __all__ = ['SiteMembership'] -class SiteMembership(ImmutableMembershipMixin, db.Model): +class SiteMembership(ImmutableUserMembershipMixin, db.Model): """Membership roles for users who are site administrators.""" __tablename__ = 'site_membership' @@ -58,7 +60,7 @@ def __table_args__(cls): return tuple(args) @cached_property - def offered_roles(self): + def offered_roles(self) -> Set[str]: """ Roles offered by this membership record. diff --git a/funnel/models/sponsor_membership.py b/funnel/models/sponsor_membership.py new file mode 100644 index 000000000..feaf3af1a --- /dev/null +++ b/funnel/models/sponsor_membership.py @@ -0,0 +1,158 @@ +from typing import Set + +from sqlalchemy.ext.declarative import declared_attr + +from werkzeug.utils import cached_property + +from coaster.sqlalchemy import DynamicAssociationProxy, immutable, with_roles + +from . import db +from .helpers import reopen +from .membership_mixin import MEMBERSHIP_RECORD_TYPE, ImmutableProfileMembershipMixin +from .profile import Profile +from .project import Project +from .reorder_mixin import ReorderMixin + +__all__ = ['SponsorMembership'] + + +class SponsorMembership(ReorderMixin, ImmutableProfileMembershipMixin, db.Model): + """Sponsor of a project.""" + + __tablename__ = 'sponsor_membership' + + # List of data columns in this model that must be copied into revisions + __data_columns__ = ('seq', 'is_promoted', 'label') + + __roles__ = { + 'all': {'read': {'urls', 'profile', 'project', 'is_promoted', 'label', 'seq'}} + } + + project_id = immutable( + db.Column(None, db.ForeignKey('project.id', ondelete='CASCADE'), nullable=False) + ) + project = immutable( + db.relationship( + Project, + backref=db.backref( + 'all_sponsor_memberships', + lazy='dynamic', + cascade='all', + passive_deletes=True, + ), + ) + ) + parent = db.synonym('project') + parent_id = db.synonym('project_id') + + #: Sequence number. Not immutable, and may be overwritten by ReorderMixin as a + #: side-effect of reordering other records. This is not considered a revision. + #: However, it can be argued that relocating a sponsor in the list constitutes a + #: change that must be recorded as a revision. We may need to change our opinion + #: on `seq` being mutable in a future iteration. + seq = db.Column(db.Integer, nullable=False) + + #: Is this sponsor being promoted for commercial reasons? Projects may have a legal + #: obligation to reveal this. This column records a declaration from the project. + is_promoted = immutable(db.Column(db.Boolean, nullable=False)) + + #: Optional label, indicating the type of sponsor + label = immutable(db.Column(db.Unicode, nullable=True)) + + # This model does not offer a large text field for promotional messages, since + # revision control on such a field is a distinct problem from membership + # revisioning. The planned Page model can be used instead, with this model getting + # a page id reference column whenever that model is ready. + + @declared_attr + def __table_args__(cls): + """Table arguments.""" + args = list(super().__table_args__) + # Add unique constraint on :attr:`seq` for active records + args.append( + db.Index( + 'ix_sponsor_membership_seq', + 'project_id', + 'seq', + unique=True, + postgresql_where=db.text('revoked_at IS NULL'), + ), + ) + return tuple(args) + + def __init__(self, **kwargs): + super().__init__(**kwargs) + # Assign a default value to `seq` + if self.seq is None: + self.seq = db.select( + [db.func.coalesce(db.func.max(SponsorMembership.seq) + 1, 1)] + ).where(self.parent_scoped_reorder_query_filter) + + @property + def parent_scoped_reorder_query_filter(self): + """ + Return a query filter that includes a scope limitation to active records. + + Used by: + * :meth:`__init__` to assign an initial sequence number, and + * :class:`ReorderMixin` to reassign sequence numbers + """ + cls = self.__class__ + # During __init__, if the constructor only received `project`, it doesn't yet + # know `project_id`. Therefore we have to be prepared for two possible returns + if self.project_id is not None: + return db.and_(cls.project_id == self.project_id, cls.is_active) + return db.and_(cls.project == self.project, cls.is_active) + + @cached_property + def offered_roles(self) -> Set[str]: + """Return empty set as this membership does not offer any roles on Project.""" + return set() + + +@reopen(Project) +class __Project: + sponsor_memberships = with_roles( + db.relationship( + SponsorMembership, + lazy='dynamic', + primaryjoin=db.and_( + SponsorMembership.project_id == Project.id, + SponsorMembership.is_active, + ), + order_by=SponsorMembership.seq, + viewonly=True, + ), + read={'all'}, + ) + + sponsors = DynamicAssociationProxy('sponsor_memberships', 'profile') + + +@reopen(Profile) +class __Profile: + sponsor_memberships = db.relationship( + SponsorMembership, + lazy='dynamic', + primaryjoin=db.and_( + SponsorMembership.profile_id == Profile.id, + SponsorMembership.is_active, + ), + order_by=SponsorMembership.granted_at.desc(), + viewonly=True, + ) + + sponsor_membership_invites = with_roles( + db.relationship( + SponsorMembership, + lazy='dynamic', + primaryjoin=db.and_( + SponsorMembership.profile_id == Profile.id, + SponsorMembership.record_type == MEMBERSHIP_RECORD_TYPE.INVITE, # type: ignore[has-type] + SponsorMembership.is_active, + ), + order_by=SponsorMembership.granted_at.desc(), + viewonly=True, + ), + read={'admin'}, + ) diff --git a/funnel/static/css/app.css b/funnel/static/css/app.css index dabdc622e..3d3a62dd2 100644 --- a/funnel/static/css/app.css +++ b/funnel/static/css/app.css @@ -3632,6 +3632,11 @@ a.loginbutton.hidden, .project-details__box__content--lesspadding { padding: 0; } +.project-banner + .project-details__box + .project-details__box__content--nopadding { + padding: 0 !important; +} .project-banner .project-details__box .card__calendar { padding: 0; } diff --git a/funnel/static/sass/_project.scss b/funnel/static/sass/_project.scss index 9854cf6e4..73b9589e8 100644 --- a/funnel/static/sass/_project.scss +++ b/funnel/static/sass/_project.scss @@ -199,6 +199,9 @@ .project-details__box__content--lesspadding { padding: 0; } + .project-details__box__content--nopadding { + padding: 0 !important; + } .card__calendar { padding: 0; diff --git a/funnel/templates/project_layout.html.jinja2 b/funnel/templates/project_layout.html.jinja2 index 179910aa5..ebbe0dfa7 100644 --- a/funnel/templates/project_layout.html.jinja2 +++ b/funnel/templates/project_layout.html.jinja2 @@ -379,9 +379,9 @@ {{ project_details(project) }} -

{% trans %}Hosted by{% endtrans %}

+

{% trans %}Hosted by{% endtrans %}

-
+
@@ -389,16 +389,56 @@ + {%- else %} +
+
+ +
{{ project.profile.title|initials }}
+ +
+
{% endif %} {{ project.profile.title }}
{% if project.profile.description.html %} -
{{ project.profile.description.html|preview(min=200, max=300) }}{% trans %}more{% endtrans %}{{ faicon(icon='caret-right-solid', baseline=false, css_class='mui--align-middle') }}
+
{{ project.profile.description.html|preview(min=200, max=300) }} {% trans %}more{% endtrans %}{{ faicon(icon='caret-right-solid', baseline=false, css_class='mui--align-middle') }}
{% endif %}
+ {% if project.sponsor_memberships %} +

{% trans %}Supported by{% endtrans %}

+ {%- endif %} + {% for sponsor in project.sponsor_memberships %} +
+
+
+
+
+ {%- if sponsor.profile.logo_url.url %} + + + + {%- else %} +
+
+ +
{{ sponsor.profile.title|initials }}
+ +
+
+ {% endif %} + {{ sponsor.profile.title }} +
+ {% if sponsor.profile.description.html %} +
{{ sponsor.profile.description.html|preview(min=200, max=300) }} {% trans %}more{% endtrans %}{{ faicon(icon='caret-right-solid', baseline=false, css_class='mui--align-middle') }}
+ {% endif %} +
+
+
+
+ {% endfor %} {% block basecontentcolumninner %}{% endblock %}
diff --git a/migrations/versions/41b3af7e4449_migrating_preview_video_to_video_field.py b/migrations/versions/41b3af7e4449_migrating_preview_video_to_video_field.py index 45ba456b2..981f5b604 100644 --- a/migrations/versions/41b3af7e4449_migrating_preview_video_to_video_field.py +++ b/migrations/versions/41b3af7e4449_migrating_preview_video_to_video_field.py @@ -9,20 +9,19 @@ from textwrap import dedent import csv import re +import urllib.parse from alembic import op from sqlalchemy.sql import column, table import sqlalchemy as sa -from funnel.models.video import make_video_url, parse_video_url - # revision identifiers, used by Alembic. revision = '41b3af7e4449' down_revision = '530c22761e27' branch_labels = None depends_on = None - +# --- Tables --------------------------------------------------------------------------- proposal = table( 'proposal', column('id', sa.Integer()), @@ -32,9 +31,104 @@ ) +# --- Functions ------------------------------------------------------------------------ + troublesome_filename = 'preview-video-troublesome.csv' +def parse_video_url(video_url: str): + video_source = 'raw' + video_id = video_url + + parsed = urllib.parse.urlparse(video_url) + if parsed.netloc is None: + raise ValueError("Invalid video URL") + + if parsed.netloc in ['youtube.com', 'www.youtube.com', 'm.youtube.com']: + if parsed.path == '/watch': + queries = urllib.parse.parse_qs(parsed.query) + if 'v' in queries and queries['v']: + video_id = queries['v'][0] + video_source = 'youtube' + else: + raise ValueError( + f"{video_url}: YouTube video URLs need to be in the format: " + "https://www.youtube.com/watch?v=dQw4w9WgXcQ" + ) + elif parsed.path.startswith('/embed'): + video_id = parsed.path.lstrip('/embed/') + if video_id: + video_id = video_id + video_source = 'youtube' + else: + raise ValueError( + f"{video_url}: YouTube video URLs need to be in the format: " + "https://www.youtube.com/watch?v=dQw4w9WgXcQ" + ) + else: + raise ValueError( + f"{video_url}: YouTube video URLs need to be in the format: " + "https://www.youtube.com/watch?v=dQw4w9WgXcQ" + ) + elif parsed.netloc == 'youtu.be': + video_id = parsed.path.lstrip('/') + if video_id: + video_id = video_id + video_source = 'youtube' + else: + raise ValueError( + "YouTube short URLs need to be in the format: " + "https://youtu.be/dQw4w9WgXcQ" + ) + elif parsed.netloc in ['vimeo.com', 'www.vimeo.com']: + video_id = parsed.path.lstrip('/') + if video_id: + video_id = video_id + video_source = 'vimeo' + else: + raise ValueError( + "Vimeo video URLs need to be in the format: " + "https://vimeo.com/336892869" + ) + elif parsed.netloc == 'drive.google.com': + if parsed.path.startswith('/open'): + queries = urllib.parse.parse_qs(parsed.query) + if 'id' in queries and queries['id']: + video_id = queries['id'][0] + video_source = 'googledrive' + else: + raise ValueError( + f"{video_url}: Google drive video URLs need to be in the format: " + "https://drive.google.com/open?id=1rwHdWYnF4asdhsnDwLECoqZQy4o or " + "https://drive.google.com/file/d/1rwHdWYnF4asdhsnDwLECoqZQy4o/view" + ) + elif parsed.path.startswith('/file/d/'): + video_id = parsed.path.lstrip('/file/d/').rstrip('/view').rstrip('/preview') + video_source = 'googledrive' + else: + raise ValueError( + f"{video_url}: Google drive video URLs need to be in the format: " + "https://drive.google.com/open?id=1rwHdWYnF4asdhsnDwLECoqZQy4o or " + "https://drive.google.com/file/d/1rwHdWYnF4asdhsnDwLECoqZQy4o/view" + ) + return video_source, video_id + + +def make_video_url(video_source: str, video_id: str): + if video_source == 'youtube': + return f'https://www.youtube.com/watch?v={video_id}' + elif video_source == 'vimeo': + return f'https://vimeo.com/{video_id}' + elif video_source == 'googledrive': + return f'https://drive.google.com/file/d/{video_id}/view' + elif video_source == 'raw': + return video_id + raise ValueError("Unknown video source") + + +# --- Migrations ----------------------------------------------------------------------- + + def upgrade(): conn = op.get_bind() diff --git a/migrations/versions/bd465803af3a_add_sponsormembership.py b/migrations/versions/bd465803af3a_add_sponsormembership.py new file mode 100644 index 000000000..2d557cc8e --- /dev/null +++ b/migrations/versions/bd465803af3a_add_sponsormembership.py @@ -0,0 +1,73 @@ +"""Add SponsorMembership. + +Revision ID: bd465803af3a +Revises: c3483d25178c +Create Date: 2021-04-22 05:02:07.027690 + +""" + +from alembic import op +from sqlalchemy.dialects import postgresql +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'bd465803af3a' +down_revision = 'c3483d25178c' +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + 'sponsor_membership', + sa.Column('granted_at', sa.TIMESTAMP(timezone=True), nullable=False), + sa.Column('revoked_at', sa.TIMESTAMP(timezone=True), nullable=True), + sa.Column('record_type', sa.Integer(), nullable=False), + sa.Column('project_id', sa.Integer(), nullable=False), + sa.Column('seq', sa.Integer(), nullable=False), + sa.Column('is_promoted', sa.Boolean(), nullable=False), + sa.Column('label', sa.Unicode(), nullable=True), + sa.Column('profile_id', sa.Integer(), nullable=False), + sa.Column('revoked_by_id', sa.Integer(), nullable=True), + sa.Column('granted_by_id', sa.Integer(), nullable=False), + sa.Column('id', postgresql.UUID(), nullable=False), + sa.Column('created_at', sa.TIMESTAMP(timezone=True), nullable=False), + sa.Column('updated_at', sa.TIMESTAMP(timezone=True), nullable=False), + sa.ForeignKeyConstraint(['granted_by_id'], ['user.id'], ondelete='SET NULL'), + sa.ForeignKeyConstraint(['profile_id'], ['profile.id'], ondelete='CASCADE'), + sa.ForeignKeyConstraint(['project_id'], ['project.id'], ondelete='CASCADE'), + sa.ForeignKeyConstraint(['revoked_by_id'], ['user.id'], ondelete='SET NULL'), + sa.CheckConstraint( + 'record_type IN (0, 1, 2, 3)', name='sponsor_membership_record_type_check' + ), + sa.PrimaryKeyConstraint('id'), + ) + op.create_index( + 'ix_sponsor_membership_active', + 'sponsor_membership', + ['project_id', 'profile_id'], + unique=True, + postgresql_where=sa.text('revoked_at IS NULL'), + ) + op.create_index( + op.f('ix_sponsor_membership_profile_id'), + 'sponsor_membership', + ['profile_id'], + unique=False, + ) + op.create_index( + 'ix_sponsor_membership_seq', + 'sponsor_membership', + ['project_id', 'seq'], + unique=True, + postgresql_where=sa.text('revoked_at IS NULL'), + ) + + +def downgrade(): + op.drop_index('ix_sponsor_membership_seq', table_name='sponsor_membership') + op.drop_index( + op.f('ix_sponsor_membership_profile_id'), table_name='sponsor_membership' + ) + op.drop_index('ix_sponsor_membership_active', table_name='sponsor_membership') + op.drop_table('sponsor_membership') diff --git a/migrations/versions/d0097ec29880_fix_membership_granted_by.py b/migrations/versions/d0097ec29880_fix_membership_granted_by.py new file mode 100644 index 000000000..c69045290 --- /dev/null +++ b/migrations/versions/d0097ec29880_fix_membership_granted_by.py @@ -0,0 +1,52 @@ +"""Fix membership granted_by. + +Revision ID: d0097ec29880 +Revises: bd465803af3a +Create Date: 2021-04-22 05:20:50.774828 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'd0097ec29880' +down_revision = 'bd465803af3a' +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column( + 'commentset_membership', + 'granted_by_id', + existing_type=sa.INTEGER(), + nullable=False, + ) + op.alter_column( + 'proposal_membership', + 'granted_by_id', + existing_type=sa.INTEGER(), + nullable=False, + ) + op.alter_column( + 'site_membership', 'granted_by_id', existing_type=sa.INTEGER(), nullable=False + ) + + +def downgrade(): + op.alter_column( + 'site_membership', 'granted_by_id', existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + 'proposal_membership', + 'granted_by_id', + existing_type=sa.INTEGER(), + nullable=True, + ) + op.alter_column( + 'commentset_membership', + 'granted_by_id', + existing_type=sa.INTEGER(), + nullable=True, + ) diff --git a/tests/conftest.py b/tests/conftest.py index 7a99d1dca..56f47f5f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -320,7 +320,7 @@ def org_uu(db_session, user_ridcully, user_librarian, user_ponder_stibbons): @pytest.fixture -def org_citywatch(db_session, user_vetinari, user_vimes): +def org_citywatch(db_session, user_vetinari, user_vimes, user_carrot): """ City Watch of Ankh-Morpork (a sub-organization). diff --git a/tests/unit/models/test_site_membership.py b/tests/unit/models/test_site_membership.py index 54303228e..a8b9f975b 100644 --- a/tests/unit/models/test_site_membership.py +++ b/tests/unit/models/test_site_membership.py @@ -18,7 +18,7 @@ def invalidate_cache(user): invalidate_cached_property(user, attr) -def test_siteadmin_roles(db_session, user_mort): +def test_siteadmin_roles(db_session, user_mort, user_death): """`SiteMembership` grants siteadmin roles.""" assert user_mort.active_site_membership is None assert user_mort.is_site_admin is False @@ -29,6 +29,7 @@ def test_siteadmin_roles(db_session, user_mort): # Create membership granting all siteadmin roles membership = SiteMembership( user=user_mort, + granted_by=user_death, is_comment_moderator=True, is_user_moderator=True, is_site_editor=True, @@ -90,6 +91,7 @@ def test_site_membership_migrate_user_transfer(db_session, user_death, user_mort # Create membership granting all siteadmin roles to Mort membership = SiteMembership( user=user_mort, + granted_by=user_death, is_comment_moderator=True, is_user_moderator=True, is_site_editor=True, @@ -123,6 +125,7 @@ def test_site_membership_migrate_user_retain(db_session, user_death, user_mort): # Create membership granting all siteadmin roles to Mort and then revoke it old_membership = SiteMembership( user=user_mort, + granted_by=user_death, is_comment_moderator=True, is_user_moderator=True, is_site_editor=True, @@ -135,6 +138,7 @@ def test_site_membership_migrate_user_retain(db_session, user_death, user_mort): # Create membership granting all siteadmin roles to Death membership = SiteMembership( user=user_death, + granted_by=user_death, is_comment_moderator=True, is_user_moderator=True, is_site_editor=True, @@ -174,6 +178,7 @@ def test_site_membership_migrate_user_merge(db_session, user_death, user_mort): # Create membership granting one siteadmin role to Mort mort_membership = SiteMembership( user=user_mort, + granted_by=user_death, is_comment_moderator=True, is_user_moderator=False, is_site_editor=False, @@ -184,6 +189,7 @@ def test_site_membership_migrate_user_merge(db_session, user_death, user_mort): # Create membership granting one siteadmin role to Death death_membership = SiteMembership( user=user_death, + granted_by=user_death, is_comment_moderator=False, is_user_moderator=True, is_site_editor=False, diff --git a/tests/unit/models/test_sponsor_membership.py b/tests/unit/models/test_sponsor_membership.py new file mode 100644 index 000000000..a2dbce5b0 --- /dev/null +++ b/tests/unit/models/test_sponsor_membership.py @@ -0,0 +1,218 @@ +"""Test SponsorMembership.""" +import pytest + +from coaster.sqlalchemy import ImmutableColumnError +from funnel.models import SponsorMembership + + +@pytest.fixture +def citywatch_sponsor(db_session, project_expo2010, org_citywatch, user_vetinari): + """Add City Watch as a sponsor of Expo 2010.""" + sponsor = SponsorMembership( + project=project_expo2010, + profile=org_citywatch.profile, + granted_by=user_vetinari, + is_promoted=False, + seq=1, + ) + db_session.add(sponsor) + return sponsor + + +@pytest.fixture +def uu_sponsor(db_session, project_expo2010, org_uu, user_vetinari): + """Add Unseen University as a sponsor of Expo 2010.""" + sponsor = SponsorMembership( + project=project_expo2010, + profile=org_uu.profile, + granted_by=user_vetinari, + is_promoted=False, + seq=2, + ) + db_session.add(sponsor) + return sponsor + + +@pytest.fixture +def dibbler_sponsor(db_session, project_expo2010, user_dibbler, user_vetinari): + """Add CMOT Dibbler as a promoted sponsor of Expo 2010.""" + sponsor = SponsorMembership( + project=project_expo2010, + profile=user_dibbler.profile, + granted_by=user_vetinari, + is_promoted=True, + label="Snack Stand", + seq=3, + ) + db_session.add(sponsor) + return sponsor + + +def test_auto_seq( + db_session, project_expo2010, org_citywatch, org_uu, user_dibbler, user_vetinari +): + """Sequence numbers are auto-issued in commit order.""" + sponsor1 = SponsorMembership( + project=project_expo2010, + profile=org_citywatch.profile, + granted_by=user_vetinari, + is_promoted=False, + ) + db_session.add(sponsor1) + db_session.commit() + + sponsor2 = SponsorMembership( + project=project_expo2010, + profile=org_uu.profile, + granted_by=user_vetinari, + is_promoted=False, + ) + db_session.add(sponsor2) + db_session.commit() + + sponsor3 = SponsorMembership( + project=project_expo2010, + profile=user_dibbler.profile, + granted_by=user_vetinari, + is_promoted=True, + label="Snack Stand", + ) + db_session.add(sponsor3) + db_session.commit() + + # We have sponsors in sequence + assert sponsor1.seq == 1 + assert sponsor2.seq == 2 + assert sponsor3.seq == 3 + + +def test_expo_has_sponsors( + db_session, + project_expo2010, + dibbler_sponsor, + uu_sponsor, + citywatch_sponsor, + org_citywatch, + org_uu, + user_dibbler, +): + """Project Expo 2010 has sponsors in a specific order.""" + db_session.commit() + assert list(project_expo2010.sponsors) == [ + org_citywatch.profile, + org_uu.profile, + user_dibbler.profile, + ] + + +def test_expo_sponsor_reorder( + db_session, project_expo2010, citywatch_sponsor, uu_sponsor, dibbler_sponsor +): + """Sponsors can be re-ordered.""" + db_session.commit() + + # We have sponsors in sequence + assert citywatch_sponsor.seq == 1 + assert uu_sponsor.seq == 2 + assert dibbler_sponsor.seq == 3 + + dibbler_sponsor.reorder_before(citywatch_sponsor) + db_session.commit() + + assert citywatch_sponsor.seq == 2 + assert uu_sponsor.seq == 3 + assert dibbler_sponsor.seq == 1 + + +def test_expo_sponsor_seq_reissue( + db_session, + project_expo2010, + citywatch_sponsor, + uu_sponsor, + dibbler_sponsor, + user_dibbler, + user_wolfgang, +): + """If the the last sponsor is dropped, the next sponsor gets their spot.""" + db_session.commit() + + # We have sponsors in sequence + assert citywatch_sponsor.seq == 1 + assert uu_sponsor.seq == 2 + assert dibbler_sponsor.seq == 3 + + # Dibbler removes self and introduces Wolfgang + dibbler_sponsor.revoke(actor=user_dibbler) + wolfgang_sponsor = SponsorMembership( + project=project_expo2010, + profile=user_wolfgang.profile, + granted_by=user_dibbler, + is_promoted=True, + label="Bite Stand", + ) + db_session.add(wolfgang_sponsor) + db_session.commit() + # Wolfgang gets the same last position in the sequence, and can reorder + assert wolfgang_sponsor.seq == 3 + wolfgang_sponsor.reorder_before(uu_sponsor) + db_session.commit() + assert citywatch_sponsor.seq == 1 + assert wolfgang_sponsor.seq == 2 + assert uu_sponsor.seq == 3 + # Dibbler's old record is untouched as it's revoked + assert dibbler_sponsor.seq == 3 + assert dibbler_sponsor.is_active is False + assert list(project_expo2010.sponsors) == [ + citywatch_sponsor.profile, + wolfgang_sponsor.profile, + uu_sponsor.profile, + ] + + +def test_change_promoted_flag(db_session, project_expo2010, citywatch_sponsor): + """Change sponsor is_promoted flag.""" + assert citywatch_sponsor.is_promoted is False + # Flag can be changed with a revision + new_record = citywatch_sponsor.replace( + actor=citywatch_sponsor.granted_by, is_promoted=True + ) + assert new_record != citywatch_sponsor + assert new_record.is_promoted is True + + noop_record = new_record.replace(actor=new_record.granted_by, is_promoted=True) + assert noop_record == new_record + + with pytest.raises(ImmutableColumnError): + new_record.is_promoted = False + + +def test_change_label(db_session, project_expo2010, citywatch_sponsor): + """Change sponsor label.""" + assert citywatch_sponsor.label is None + # Flag can be changed with a revision + new_record = citywatch_sponsor.replace( + actor=citywatch_sponsor.granted_by, label="Guards! Guards!" + ) + assert new_record != citywatch_sponsor + assert new_record.label == "Guards! Guards!" + + noop_record = new_record.replace( + actor=new_record.granted_by, label="Guards! Guards!" + ) + assert noop_record == new_record + + with pytest.raises(ImmutableColumnError): + new_record.label = None + + +def test_sponsor_offered_roles(db_session, project_expo2010, citywatch_sponsor): + """Sponsors don't get a role from the sponsor membership.""" + assert citywatch_sponsor.offered_roles == set() + + +def test_sponsor_subject_role( + db_session, citywatch_sponsor, user_vimes, user_rincewind +): + """Sponsor profile admins get subject role on the membership.""" + assert 'subject' in citywatch_sponsor.roles_for(user_vimes) + assert 'subject' not in citywatch_sponsor.roles_for(user_rincewind)