Send flag notification emails to moderators

Send flag notification emails to all of a group's moderators rather than
just to the group's creator.

Also allow all moderators to moderate (hide) flagged annotations, not
just the creator.

h/security/permission_map.py

@@ -13,9 +13,21 @@
 """
 
 import h.security.predicates as p
+from h.models import GroupMembershipRoles
 from h.security.permissions import Permission
 from h.security.predicates import resolve_predicates
 
+# The logic for who is allowed to moderate annotations in a group unfortunately
+# has to be duplicated in the PERMISSION_MAP below and elsewhere when sending
+# email notifications to moderators.  To help keep the two implementations in
+# sync both are derived from this constant.
+GROUP_MODERATE_PREDICATES = {
+    GroupMembershipRoles.OWNER: [p.group_has_user_as_owner],
+    GroupMembershipRoles.ADMIN: [p.group_has_user_as_admin],
+    GroupMembershipRoles.MODERATOR: [p.group_has_user_as_moderator],
+}
+
+
 PERMISSION_MAP = {
     # Admin pages
     Permission.AdminPage.HIGH_RISK: [[p.user_is_admin]],
@@ -55,7 +67,7 @@
         [p.group_has_user_as_moderator],
     ],
     Permission.Group.MEMBER_ADD: [[p.group_matches_authenticated_client_authority]],
-    Permission.Group.MODERATE: [[p.group_created_by_user]],
+    Permission.Group.MODERATE: GROUP_MODERATE_PREDICATES.values(),
     # --------------------------------------------------------------------- #
     # Annotations
     Permission.Annotation.CREATE: [[p.authenticated]],

h/security/predicates.py

@@ -137,11 +137,6 @@ def group_joinable_by_authority(_identity, context):
     return context.group.joinable_by == JoinableBy.authority
 
 
-@requires(authenticated_user, group_found)
-def group_created_by_user(identity, context):
-    return context.group.creator and context.group.creator.id == identity.user.id
-
-
 @requires(authenticated_user, group_found)
 def group_has_user_as_owner(identity, context):
     return _group_has_user_as_role(identity, context, GroupMembershipRoles.OWNER)

h/services/group_members.py

@@ -1,10 +1,10 @@
 import logging
 from functools import partial
 
-from sqlalchemy import select
+from sqlalchemy import or_, select
 
 from h import session
-from h.models import GroupMembership
+from h.models import Group, GroupMembership, GroupMembershipRoles
 
 log = logging.getLogger(__name__)
 
@@ -24,6 +24,29 @@ def __init__(self, db, user_fetcher, publish):
         self.user_fetcher = user_fetcher
         self.publish = publish
 
+    def get(self, group: Group, roles: list[GroupMembershipRoles] | None = None):
+        """
+        Return `group`'s memberships.
+
+        If `roles` is None return all of `group`'s memberships.
+
+        If `roles` is not None return only those memberships matching the given role(s).
+
+        If multiple roles are given return all memberships matching *any* of
+        the given roles.
+        """
+        query = select(GroupMembership).where(GroupMembership.group == group)
+
+        if roles:
+            if len(roles) == 1:
+                query = query.where(GroupMembership.roles.contains(roles[0]))
+            else:
+                query = query.where(
+                    or_(GroupMembership.roles.contains(role) for role in roles)
+                )
+
+        return self.db.scalars(query)
+
     def add_members(self, group, userids):
         """
         Add the users indicated by userids to this group's members.

h/views/api/flags.py

@@ -3,6 +3,7 @@
 from h import links
 from h.emails import flag_notification
 from h.security import Permission
+from h.security.permission_map import GROUP_MODERATE_PREDICATES
 from h.tasks import mailer
 from h.views.api.config import api_config
 
@@ -18,19 +19,23 @@
 def create(context, request):
     request.find_service(name="flag").create(request.user, context.annotation)
 
-    _email_group_admin(request, context.annotation)
+    _email_group_moderators(request, context.annotation)
 
     return HTTPNoContent()
 
 
-def _email_group_admin(request, annotation):
+def _email_group_moderators(request, annotation):
     incontext_link = links.incontext_link(request, annotation)
     if incontext_link is None:
         incontext_link = annotation.target_uri
 
-    group = annotation.group
-    if group.creator and group.creator.email:
-        send_params = flag_notification.generate(
-            request, group.creator.email, incontext_link
-        )
-        mailer.send.delay(*send_params)
+    group_members_service = request.find_service(name="group_members")
+
+    memberships = group_members_service.get(
+        annotation.group, roles=list(GROUP_MODERATE_PREDICATES.keys())
+    )
+
+    for membership in memberships:
+        if email := membership.user.email:
+            send_params = flag_notification.generate(request, email, incontext_link)
+            mailer.send.delay(*send_params)

tests/functional/api/moderation_test.py

@@ -1,16 +1,17 @@
 import pytest
 
+from h.models import GroupMembership, GroupMembershipRoles
+
 
 class TestPutHide:
-    def test_it_returns_http_204_for_group_creator(
+    def test_it_returns_http_204_for_group_moderator(
         self, app, group_annotation, user_with_token
     ):
         _, token = user_with_token
         headers = {"Authorization": str(f"Bearer {token.value}")}
 
         res = app.put(f"/api/annotations/{group_annotation.id}/hide", headers=headers)
 
-        # The creator of a group has moderation rights over the annotations in that group
         assert res.status_code == 204
 
     def test_it_returns_http_404_if_annotation_is_in_world_group(
@@ -51,7 +52,7 @@ def test_it_returns_http_404_if_annotation_is_private(
 
 
 class TestDeleteHide:
-    def test_it_returns_http_204_for_group_creator(
+    def test_it_returns_http_204_for_group_moderator(
         self, app, group_annotation, user_with_token
     ):
         _, token = user_with_token
@@ -61,7 +62,6 @@ def test_it_returns_http_204_for_group_creator(
             f"/api/annotations/{group_annotation.id}/hide", headers=headers
         )
 
-        # The creator of a group has moderation rights over the annotations in that group
         assert res.status_code == 204
 
     def test_it_returns_http_404_if_annotation_is_in_world_group(
@@ -117,7 +117,9 @@ def other_user(db_session, factories):
 
 @pytest.fixture
 def group(user, db_session, factories):
-    group = factories.Group(creator=user)
+    group = factories.Group(
+        memberships=[GroupMembership(user=user, roles=[GroupMembershipRoles.MODERATOR])]
+    )
     db_session.commit()
     return group
 

tests/functional/moderation_test.py

@@ -1,44 +1,23 @@
-import pytest
+from h.models import GroupMembership, GroupMembershipRoles
 
 
 class TestModeration:
-    def test_moderator_flag_listing(
-        self, app, flagged_annotation, moderator_with_token
-    ):
-        _, token = moderator_with_token
-
-        headers = {"Authorization": f"Bearer {token.value}"}
-        annotation_url = f"/api/annotations/{flagged_annotation.id}"
-        res = app.get(annotation_url, headers=headers)
-
-        assert "moderation" in res.json
-        assert res.json["moderation"]["flagCount"] > 0
-
-
-@pytest.fixture
-def group(db_session, factories, moderator):
-    group = factories.OpenGroup(creator=moderator)
-    db_session.commit()
-    return group
-
-
-@pytest.fixture
-def flagged_annotation(group, db_session, factories):
-    ann = factories.Annotation(groupid=group.pubid, shared=True)
-    factories.Flag(annotation=ann)
-    db_session.commit()
-    return ann
-
-
-@pytest.fixture
-def moderator(db_session, factories):
-    user = factories.User()
-    db_session.commit()
-    return user
-
-
-@pytest.fixture
-def moderator_with_token(moderator, db_session, factories):
-    token = factories.DeveloperToken(user=moderator)
-    db_session.commit()
-    return (moderator, token)
+    def test_it(self, app, db_session, factories):
+        group = factories.OpenGroup()
+        annotation = factories.Annotation(group=group, shared=True)
+        factories.Flag(annotation=annotation)
+        moderator = factories.User(
+            memberships=[
+                GroupMembership(group=group, roles=[GroupMembershipRoles.MODERATOR])
+            ]
+        )
+        token = factories.DeveloperToken(user=moderator)
+        db_session.commit()
+
+        response = app.get(
+            f"/api/annotations/{annotation.id}",
+            headers={"Authorization": f"Bearer {token.value}"},
+        )
+
+        assert "moderation" in response.json
+        assert response.json["moderation"]["flagCount"] > 0

tests/unit/h/security/permits_test.py

@@ -3,7 +3,7 @@
 import pytest
 from pyramid.security import Allowed, Denied
 
-from h.models import GroupMembership
+from h.models import GroupMembership, GroupMembershipRoles
 from h.security import Identity, Permission
 from h.security.permits import PERMISSION_MAP, identity_permits
 from h.traversal import AnnotationContext
@@ -67,7 +67,7 @@ def PERMISSION_MAP(self):
 
 
 class TestIdentityPermitsIntegrated:
-    def test_it(self, user, group, annotation):
+    def test_it(self, user, annotation):
         # We aren't going to go bonkers here, but a couple of tests to show
         # this actually holds together. This isn't really to inform us of any
         # particular failure, but just give us sensitivity if this doesn't work
@@ -79,11 +79,11 @@ def test_it(self, user, group, annotation):
         # A user can delete their own annotation
         assert identity_permits(identity, anno_context, Permission.Annotation.DELETE)
 
-        # Once a user is the creator of a group they can moderate
+        # Once a user is an owner of a group they can moderate
         assert not identity_permits(
             identity, anno_context, Permission.Annotation.MODERATE
         )
-        group.creator = user
+        identity.user.memberships[0].roles = [GroupMembershipRoles.OWNER]
         assert identity_permits(identity, anno_context, Permission.Annotation.MODERATE)
 
         # Once a user is an admin they can do admin things

tests/unit/h/security/predicates_test.py

@@ -164,16 +164,6 @@ def test_group_joinable_by_authority(self, group_context, joinable_by):
 
         assert result == (joinable_by == JoinableBy.authority)
 
-    def test_group_created_by_user(self, identity, group_context, factories):
-        group_context.group.creator = None
-        assert not predicates.group_created_by_user(identity, group_context)
-
-        group_context.group.creator = factories.User.build(id="different_user")
-        assert not predicates.group_created_by_user(identity, group_context)
-
-        group_context.group.creator.id = identity.user.id
-        assert predicates.group_created_by_user(identity, group_context)
-
     @pytest.mark.parametrize(
         "role,expected_result",
         [

tests/unit/h/services/group_members_test.py

@@ -4,10 +4,76 @@
 import pytest
 from sqlalchemy import select
 
-from h.models import GroupMembership, User
+from h.models import GroupMembership, GroupMembershipRoles, User
 from h.services.group_members import GroupMembersService, group_members_factory
 
 
+class TestGet:
+    def test_it(self, group_members_service, db_session, factories):
+        group, other_group = factories.Group.build_batch(size=2)
+        users = factories.User.build_batch(size=2)
+        memberships = [GroupMembership(group=group, user=user) for user in users]
+        db_session.add_all(
+            [*memberships, GroupMembership(group=other_group, user=users[0])]
+        )
+
+        assert list(group_members_service.get(group)) == memberships
+
+    def test_roles(self, group_members_service, db_session, factories):
+        group = factories.Group.build()
+        admins = factories.User.build_batch(size=2)
+        moderator = factories.User.build()
+        memberships = [
+            GroupMembership(group=group, user=user, roles=[GroupMembershipRoles.ADMIN])
+            for user in admins
+        ]
+        db_session.add_all(
+            [
+                *memberships,
+                GroupMembership(
+                    group=group, user=moderator, roles=[GroupMembershipRoles.MODERATOR]
+                ),
+            ]
+        )
+
+        assert (
+            list(group_members_service.get(group, roles=[GroupMembershipRoles.ADMIN]))
+            == memberships
+        )
+
+    def test_multiple_roles(self, group_members_service, db_session, factories):
+        group = factories.Group.build()
+        admin = factories.User.build()
+        moderator = factories.User.build()
+        member = factories.User.build()
+        memberships = [
+            GroupMembership(
+                group=group, user=admin, roles=[GroupMembershipRoles.ADMIN]
+            ),
+            GroupMembership(
+                group=group, user=moderator, roles=[GroupMembershipRoles.MODERATOR]
+            ),
+        ]
+        db_session.add_all(
+            [
+                *memberships,
+                GroupMembership(
+                    group=group, user=member, roles=[GroupMembershipRoles.MEMBER]
+                ),
+            ]
+        )
+
+        assert (
+            list(
+                group_members_service.get(
+                    group,
+                    roles=[GroupMembershipRoles.ADMIN, GroupMembershipRoles.MODERATOR],
+                )
+            )
+            == memberships
+        )
+
+
 class TestMemberJoin:
     def test_it_adds_user_to_group(
         self, group_members_service, factories, caplog, db_session