diff --git a/docs/_extra/api-reference/schemas/annotation.yaml b/docs/_extra/api-reference/schemas/annotation.yaml index 68e7f0253ca..9e0735934cf 100644 --- a/docs/_extra/api-reference/schemas/annotation.yaml +++ b/docs/_extra/api-reference/schemas/annotation.yaml @@ -199,3 +199,24 @@ Annotation: description: The annotation creator's display name example: "Felicity Nunsun" - type: null + mentions: + type: array + items: + type: object + properties: + userid: + type: string + pattern: "acct:^[A-Za-z0-9._]{3,30}@.*$" + description: user account ID in the format `"acct:@"` + example: "acct:felicity_nunsun@hypothes.is" + username: + type: string + description: The username of the user at the time of the mention + display_name: + type: string + description: The display name of the user + link: + type: string + format: uri + description: The link to the user profile + description: An array of user mentions the annotation text diff --git a/h/models/mention.py b/h/models/mention.py index 3e5a3f3aab5..fe27aca014f 100644 --- a/h/models/mention.py +++ b/h/models/mention.py @@ -6,7 +6,7 @@ from h.models import helpers -class Mention(Base, Timestamps): # pragma: nocover +class Mention(Base, Timestamps): __tablename__ = "mention" id: Mapped[int] = mapped_column(sa.Integer, autoincrement=True, primary_key=True) @@ -17,7 +17,9 @@ class Mention(Base, Timestamps): # pragma: nocover nullable=False, ) """FK to annotation.id""" - annotation = sa.orm.relationship("Annotation", back_populates="mentions") + annotation = sa.orm.relationship( + "Annotation", back_populates="mentions", uselist=False + ) user_id: Mapped[int] = mapped_column( sa.Integer, diff --git a/h/presenters/mention_json.py b/h/presenters/mention_json.py new file mode 100644 index 00000000000..ebd61188f41 --- /dev/null +++ b/h/presenters/mention_json.py @@ -0,0 +1,18 @@ +from typing import Any + +from h.models import Mention + + +class MentionJSONPresenter: + """Present a mention in the JSON format returned by API requests.""" + + def __init__(self, mention: Mention): + self._mention = mention + + def asdict(self) -> dict[str, Any]: + return { + "userid": self._mention.user.userid, + "username": self._mention.username, + "display_name": self._mention.user.display_name, + "link": self._mention.user.uri, + } diff --git a/h/services/__init__.py b/h/services/__init__.py index a48d0f253a1..5b8fad3cc78 100644 --- a/h/services/__init__.py +++ b/h/services/__init__.py @@ -12,6 +12,7 @@ ) from h.services.email import EmailService from h.services.job_queue import JobQueueService +from h.services.mention import MentionService from h.services.subscription import SubscriptionService @@ -43,6 +44,7 @@ def includeme(config): # pragma: no cover config.register_service_factory( "h.services.annotation_write.service_factory", iface=AnnotationWriteService ) + config.register_service_factory("h.services.mention.factory", iface=MentionService) # Other services config.register_service_factory( diff --git a/h/services/annotation_json.py b/h/services/annotation_json.py index a88f7368690..4014f3fdde4 100644 --- a/h/services/annotation_json.py +++ b/h/services/annotation_json.py @@ -2,9 +2,12 @@ from h.models import Annotation, User from h.presenters import DocumentJSONPresenter +from h.presenters.mention_json import MentionJSONPresenter from h.security import Identity, identity_permits from h.security.permissions import Permission +from h.services import MentionService from h.services.annotation_read import AnnotationReadService +from h.services.feature import FeatureService from h.services.flag import FlagService from h.services.links import LinksService from h.services.user import UserService @@ -16,12 +19,14 @@ class AnnotationJSONService: """A service for generating API compatible JSON for annotations.""" - def __init__( + def __init__( # noqa: PLR0913 self, annotation_read_service: AnnotationReadService, links_service: LinksService, flag_service: FlagService, user_service: UserService, + mention_service: MentionService, + feature_service: FeatureService, ): """ Instantiate the service. @@ -30,11 +35,14 @@ def __init__( :param links_service: LinksService instance :param flag_service: FlagService instance :param user_service: UserService instance + :param mention_service: MentionService instance """ self._annotation_read_service = annotation_read_service self._links_service = links_service self._flag_service = flag_service self._user_service = user_service + self._mention_service = mention_service + self._feature_service = feature_service def present(self, annotation: Annotation): """ @@ -73,6 +81,11 @@ def present(self, annotation: Annotation): "links": self._links_service.get_all(annotation), } ) + if self._feature_service.enabled("at_mentions"): # pragma: no cover + model["mentions"] = [ + MentionJSONPresenter(mention).asdict() + for mention in annotation.mentions + ] model.update(user_info(self._user_service.fetch(annotation.userid))) @@ -151,6 +164,8 @@ def present_all_for_user(self, annotation_ids, user: User): # which ultimately depends on group permissions, causing a # group lookup for every annotation without this Annotation.group, + # Optimise access to the mentions + Annotation.mentions, ], ) @@ -184,4 +199,6 @@ def factory(_context, request): links_service=request.find_service(name="links"), flag_service=request.find_service(name="flag"), user_service=request.find_service(name="user"), + mention_service=request.find_service(MentionService), + feature_service=request.find_service(name="feature"), ) diff --git a/h/services/annotation_write.py b/h/services/annotation_write.py index dade8d1bac0..3be303bb596 100644 --- a/h/services/annotation_write.py +++ b/h/services/annotation_write.py @@ -13,6 +13,7 @@ from h.services.annotation_metadata import AnnotationMetadataService from h.services.annotation_read import AnnotationReadService from h.services.job_queue import JobQueueService +from h.services.mention import MentionService from h.traversal.group import GroupContext from h.util.group_scope import url_in_scope @@ -22,19 +23,23 @@ class AnnotationWriteService: """A service for storing and retrieving annotations.""" - def __init__( + def __init__( # noqa: PLR0913 self, db_session: Session, has_permission: Callable, queue_service: JobQueueService, annotation_read_service: AnnotationReadService, annotation_metadata_service: AnnotationMetadataService, + mention_service: MentionService, + feature_service, ): self._db = db_session self._has_permission = has_permission self._queue_service = queue_service self._annotation_read_service = annotation_read_service self._annotation_metadata_service = annotation_metadata_service + self._mention_service = mention_service + self._feature_service = feature_service def create_annotation(self, data: dict) -> Annotation: """ @@ -88,6 +93,9 @@ def create_annotation(self, data: dict) -> Annotation: schedule_in=60, ) + if self._feature_service.enabled("at_mentions"): # pragma: no cover + self._mention_service.update_mentions(annotation) + return annotation def update_annotation( @@ -281,4 +289,6 @@ def service_factory(_context, request) -> AnnotationWriteService: queue_service=request.find_service(name="queue_service"), annotation_read_service=request.find_service(AnnotationReadService), annotation_metadata_service=request.find_service(AnnotationMetadataService), + mention_service=request.find_service(MentionService), + feature_service=request.find_service(name="feature"), ) diff --git a/h/services/html.py b/h/services/html.py new file mode 100644 index 00000000000..30e8ecebef6 --- /dev/null +++ b/h/services/html.py @@ -0,0 +1,22 @@ +# noqa: A005 + +from html.parser import HTMLParser + + +class LinkParser(HTMLParser): + def __init__(self): + super().__init__() + self._links = [] + + def handle_starttag(self, tag, attrs): + if tag == "a": + self._links.append(dict(attrs)) + + def get_links(self) -> list[dict]: + return self._links + + +def parse_html_links(html: str) -> list[dict]: + parser = LinkParser() + parser.feed(html) + return parser.get_links() diff --git a/h/services/mention.py b/h/services/mention.py new file mode 100644 index 00000000000..e7a7abf7eba --- /dev/null +++ b/h/services/mention.py @@ -0,0 +1,79 @@ +import logging +from collections import OrderedDict + +from sqlalchemy import delete +from sqlalchemy.orm import Session + +from h.models import Annotation, Mention +from h.services.html import parse_html_links +from h.services.user import UserService + +MENTION_ATTRIBUTE = "data-hyp-mention" +MENTION_USERID = "data-userid" +MENTION_LIMIT = 5 + +logger = logging.getLogger(__name__) + + +class MentionService: + """A service for managing user mentions.""" + + def __init__(self, session: Session, user_service: UserService): + self._session = session + self._user_service = user_service + + def update_mentions(self, annotation: Annotation) -> None: + self._session.flush() + + # Only shared annotations can have mentions + if not annotation.shared: + return + mentioning_user = self._user_service.fetch(annotation.userid) + # NIPSA users do not send mentions + if mentioning_user.nipsa: + return + + mentioned_userids = OrderedDict.fromkeys(self._parse_userids(annotation.text)) + mentioned_users = self._user_service.fetch_all(mentioned_userids) + self._session.execute( + delete(Mention).where(Mention.annotation_id == annotation.id) + ) + + for i, user in enumerate(mentioned_users): + if i >= MENTION_LIMIT: + logger.warning( + "Annotation %s has more than %s mentions", + annotation.id, + MENTION_LIMIT, + ) + break + # NIPSA users do not receive mentions + if user.nipsa: + continue + # Only allow mentions if the annotation is in the public group + # or the annotation is in one of mentioned user's groups + if not ( + annotation.groupid == "__world__" or annotation.group in user.groups + ): + continue + + mention = Mention( + annotation_id=annotation.id, user_id=user.id, username=user.username + ) + self._session.add(mention) + + @staticmethod + def _parse_userids(text: str) -> list[str]: + links = parse_html_links(text) + return [ + user_id + for link in links + if MENTION_ATTRIBUTE in link and (user_id := link.get(MENTION_USERID)) + ] + + +def factory(_context, request) -> MentionService: + """Return a MentionService instance for the passed context and request.""" + return MentionService( + session=request.db, user_service=request.find_service(name="user") + ) diff --git a/tests/common/factories/__init__.py b/tests/common/factories/__init__.py index 75c5fd62995..a4c2c2148f9 100644 --- a/tests/common/factories/__init__.py +++ b/tests/common/factories/__init__.py @@ -16,6 +16,7 @@ from tests.common.factories.group import Group, OpenGroup, RestrictedGroup from tests.common.factories.group_scope import GroupScope from tests.common.factories.job import ExpungeUserJob, Job, SyncAnnotationJob +from tests.common.factories.mention import Mention from tests.common.factories.organization import Organization from tests.common.factories.setting import Setting from tests.common.factories.subscriptions import Subscriptions diff --git a/tests/common/factories/mention.py b/tests/common/factories/mention.py new file mode 100644 index 00000000000..d795913fd71 --- /dev/null +++ b/tests/common/factories/mention.py @@ -0,0 +1,17 @@ +import factory + +from h import models + +from .annotation import Annotation +from .base import ModelFactory +from .user import User + + +class Mention(ModelFactory): + class Meta: + model = models.Mention + sqlalchemy_session_persistence = "flush" + + annotation = factory.SubFactory(Annotation) + user = factory.SubFactory(User) + username = factory.LazyAttribute(lambda obj: obj.user.username) diff --git a/tests/common/fixtures/services.py b/tests/common/fixtures/services.py index 2c447815faf..d1ace603099 100644 --- a/tests/common/fixtures/services.py +++ b/tests/common/fixtures/services.py @@ -2,6 +2,7 @@ import pytest +from h.services import MentionService from h.services.analytics import AnalyticsService from h.services.annotation_delete import AnnotationDeleteService from h.services.annotation_json import AnnotationJSONService @@ -20,6 +21,7 @@ ) from h.services.developer_token import DeveloperTokenService from h.services.email import EmailService +from h.services.feature import FeatureService from h.services.flag import FlagService from h.services.group import GroupService from h.services.group_create import GroupCreateService @@ -60,6 +62,7 @@ "bulk_stats_service", "developer_token_service", "email_service", + "feature_service", "flag_service", "group_create_service", "group_delete_service", @@ -70,6 +73,7 @@ "group_update_service", "links_service", "list_organizations_service", + "mention_service", "mock_service", "moderation_service", "nipsa_service", @@ -310,6 +314,16 @@ def user_update_service(mock_service): return mock_service(UserUpdateService, name="user_update") +@pytest.fixture +def mention_service(mock_service): + return mock_service(MentionService) + + +@pytest.fixture +def feature_service(mock_service): + return mock_service(FeatureService, name="feature") + + @pytest.fixture def email_service(mock_service): return mock_service(EmailService) diff --git a/tests/unit/h/models/mention_test.py b/tests/unit/h/models/mention_test.py new file mode 100644 index 00000000000..270d279f291 --- /dev/null +++ b/tests/unit/h/models/mention_test.py @@ -0,0 +1,8 @@ +def test_repr(factories): + annotation = factories.Annotation() + mention = factories.Mention(annotation=annotation) + + assert ( + repr(mention) + == f"Mention(id={mention.id}, annotation_id={mention.annotation.id!r}, user_id={mention.user.id})" + ) diff --git a/tests/unit/h/presenters/mention_json_test.py b/tests/unit/h/presenters/mention_json_test.py new file mode 100644 index 00000000000..23cf9db902f --- /dev/null +++ b/tests/unit/h/presenters/mention_json_test.py @@ -0,0 +1,26 @@ +import pytest + +from h.models import Mention +from h.presenters.mention_json import MentionJSONPresenter + + +class TestMentionJSONPresenter: + def test_as_dict(self, user, annotation): + mention = Mention(annotation=annotation, user=user, username=user.username) + + data = MentionJSONPresenter(mention).asdict() + + assert data == { + "userid": user.userid, + "username": user.username, + "display_name": user.display_name, + "link": user.uri, + } + + @pytest.fixture + def user(self, factories): + return factories.User.build() + + @pytest.fixture + def annotation(self, factories): + return factories.Annotation.build() diff --git a/tests/unit/h/services/annotation_json_test.py b/tests/unit/h/services/annotation_json_test.py index 5b563200dbf..6c66e7aec15 100644 --- a/tests/unit/h/services/annotation_json_test.py +++ b/tests/unit/h/services/annotation_json_test.py @@ -17,7 +17,7 @@ def test_present( ): annotation.created = datetime(2016, 2, 24, 18, 3, 25, 768) # noqa: DTZ001 annotation.updated = datetime(2016, 2, 29, 10, 24, 5, 564) # noqa: DTZ001 - annotation.references = ["referenced-id-1", "referenced-id-2"] + annotation.references = ["pUQBbZOcRA2h_5259SY98Q", "473NFWUVSiqlSDLmpBbNOg"] annotation.extra = {"extra-1": "foo", "extra-2": "bar"} result = service.present(annotation) @@ -47,6 +47,7 @@ def test_present( "extra-1": "foo", "extra-2": "bar", "user_info": {"display_name": user_service.fetch.return_value.display_name}, + "mentions": [], } DocumentJSONPresenter.assert_called_once_with(annotation.document) @@ -188,7 +189,12 @@ def test_present_all_for_user( annotation_read_service.get_annotations_by_id.assert_called_once_with( ids=sentinel.annotation_ids, - eager_load=[Annotation.document, Annotation.moderation, Annotation.group], + eager_load=[ + Annotation.document, + Annotation.moderation, + Annotation.group, + Annotation.mentions, + ], ) flag_service.all_flagged.assert_called_once_with(user, sentinel.annotation_ids) flag_service.flag_counts.assert_called_once_with(sentinel.annotation_ids) @@ -201,13 +207,21 @@ def test_present_all_for_user( @pytest.fixture def service( - self, annotation_read_service, links_service, flag_service, user_service + self, + annotation_read_service, + links_service, + flag_service, + user_service, + mention_service, + feature_service, ): return AnnotationJSONService( annotation_read_service=annotation_read_service, links_service=links_service, flag_service=flag_service, user_service=user_service, + mention_service=mention_service, + feature_service=feature_service, ) @pytest.fixture @@ -246,6 +260,8 @@ def test_it( flag_service, links_service, user_service, + mention_service, + feature_service, ): service = factory(sentinel.context, pyramid_request) @@ -254,6 +270,8 @@ def test_it( links_service=links_service, flag_service=flag_service, user_service=user_service, + mention_service=mention_service, + feature_service=feature_service, ) assert service == AnnotationJSONService.return_value diff --git a/tests/unit/h/services/annotation_write_test.py b/tests/unit/h/services/annotation_write_test.py index c28781b1bcc..bc593726448 100644 --- a/tests/unit/h/services/annotation_write_test.py +++ b/tests/unit/h/services/annotation_write_test.py @@ -20,6 +20,7 @@ def test_create_annotation( update_document_metadata, queue_service, annotation_read_service, + mention_service, _validate_group, # noqa: PT019 db_session, ): @@ -41,6 +42,7 @@ def test_create_annotation( tag="storage.create_annotation", schedule_in=60, ) + mention_service.update_mentions.assert_called_once_with(anno) assert anno == Any.instance_of(Annotation).with_attrs( { @@ -279,6 +281,8 @@ def svc( queue_service, annotation_read_service, annotation_metadata_service, + mention_service, + feature_service, ): return AnnotationWriteService( db_session=db_session, @@ -286,6 +290,8 @@ def svc( queue_service=queue_service, annotation_read_service=annotation_read_service, annotation_metadata_service=annotation_metadata_service, + mention_service=mention_service, + feature_service=feature_service, ) @pytest.fixture @@ -321,6 +327,8 @@ def test_it( queue_service, annotation_read_service, annotation_metadata_service, + mention_service, + feature_service, ): svc = service_factory(sentinel.context, pyramid_request) @@ -330,6 +338,8 @@ def test_it( queue_service=queue_service, annotation_read_service=annotation_read_service, annotation_metadata_service=annotation_metadata_service, + mention_service=mention_service, + feature_service=feature_service, ) assert svc == AnnotationWriteService.return_value diff --git a/tests/unit/h/services/html_test.py b/tests/unit/h/services/html_test.py new file mode 100644 index 00000000000..03c7b4f8f0a --- /dev/null +++ b/tests/unit/h/services/html_test.py @@ -0,0 +1,13 @@ +from h.services.html import parse_html_links + + +def test_parse_html_links(): + html = 'Hello @devdata_user' + links = parse_html_links(html) + assert links == [{"data-hyp-mention": None, "data-userid": "devdata_user"}] + + +def test_parse_html_links_no_links(): + html = "

Hello world

" + links = parse_html_links(html) + assert links == [] diff --git a/tests/unit/h/services/mention_test.py b/tests/unit/h/services/mention_test.py new file mode 100644 index 00000000000..5ca404353e5 --- /dev/null +++ b/tests/unit/h/services/mention_test.py @@ -0,0 +1,111 @@ +from unittest.mock import sentinel + +import pytest + +from h.services.mention import MentionService, factory + + +class TestMentionService: + def test_update_mentions(self, service, annotation, mentioned_user): + service.update_mentions(annotation) + + assert len(annotation.mentions) == 1 + assert annotation.mentions[0].user == mentioned_user + + def test_update_mentions_with_nipsa_mentioned_user( + self, service, annotation, mentioned_user + ): + mentioned_user.nipsa = True + + service.update_mentions(annotation) + + assert len(annotation.mentions) == 0 + + def test_update_mentions_with_nipsa_mentioning_user( + self, service, annotation, annotation_slim + ): + annotation_slim.user.nipsa = True + + service.update_mentions(annotation) + + assert len(annotation.mentions) == 0 + + def test_update_mentions_with_more_than_mention_limit( + self, + service, + user_service, + annotation, + mentioned_user, + factories, + patch, + ): + mentioned_user2 = factories.User() + annotation.text = ( + f'Hello @{mentioned_user.display_name}' + f'Hello @{mentioned_user2.display_name}' + ) + user_service.fetch_all.return_value = [mentioned_user, mentioned_user2] + limit = patch("h.services.mention.MENTION_LIMIT", new=1, autospec=False) + + service.update_mentions(annotation) + + assert len(annotation.mentions) == limit + + def test_update_mentions_with_groupid_not_world( + self, service, annotation, factories + ): + group = factories.Group() + annotation.groupid = group.pubid + + service.update_mentions(annotation) + + assert len(annotation.mentions) == 0 + + def test_update_mentions_with_private_annotation(self, service, annotation): + annotation.shared = False + + service.update_mentions(annotation) + + assert len(annotation.mentions) == 0 + + @pytest.fixture + def annotation(self, annotation_slim, mentioned_user): + annotation = annotation_slim.annotation + annotation.text = f'Hello @{mentioned_user.display_name}' + annotation.shared = True + return annotation + + @pytest.fixture + def annotation_slim(self, factories): + slim = factories.AnnotationSlim() + slim.user.nipsa = False + return slim + + @pytest.fixture + def mentioned_user(self, factories): + return factories.User(nipsa=False) + + @pytest.fixture + def user_service(self, user_service, annotation_slim, mentioned_user): + user_service.fetch.return_value = annotation_slim.user + user_service.fetch_all.return_value = [mentioned_user] + return user_service + + @pytest.fixture + def service(self, db_session, user_service): + return MentionService(db_session, user_service) + + +class TestFactory: + def test_it(self, pyramid_request, user_service, MentionService): + service = factory(sentinel.context, pyramid_request) + + MentionService.assert_called_once_with( + session=pyramid_request.db, + user_service=user_service, + ) + assert service == MentionService.return_value + + @pytest.fixture + def MentionService(self, patch): + return patch("h.services.mention.MentionService")