feat: allow discussion notification customization

CHANGELOG.md

@@ -17,6 +17,7 @@
 - Add api endpoint /me/org_topics/ [#3070](https://github.com/opendatateam/udata/pull/3070)
 - Expose dataservices in RDF catalog [#3058](https://github.com/opendatateam/udata/pull/3058)
 - CORS: always returns 204 on OPTIONS request [#3046](https://github.com/opendatateam/udata/pull/3046) 
+- Allow discussion notification customization [#3072](https://github.com/opendatateam/udata/pull/3072)
 
 ## 9.0.0 (2024-06-07)
 

udata/core/discussions/tasks.py

@@ -1,11 +1,18 @@
+import fnmatch
+
+from urllib.parse import urlparse
+
+from flask import current_app
+
 from udata import mail
 from udata.i18n import lazy_gettext as _
 from udata.core.dataset.models import Dataset
-from udata.core.reuse.models import Reuse
 from udata.core.post.models import Post
+from udata.core.reuse.models import Reuse
+from udata.core.topic.models import Topic
 from udata.tasks import connect, get_logger
 
-from .models import Discussion, Message
+from .models import Discussion
 from .signals import (
     on_new_discussion, on_new_discussion_comment, on_discussion_closed
 )
@@ -22,15 +29,48 @@ def owner_recipients(discussion):
         return []
 
 
+def get_external_url(discussion):
+    url = None
+    if (meta_url := discussion.extras.get('notification', {}).get('external_url')):
+        meta_url_parsed = urlparse(meta_url)
+        if any(
+            fnmatch.fnmatch(meta_url_parsed.netloc, pattern)
+            for pattern in current_app.config['DISCUSSION_ALLOWED_EXTERNAL_DOMAINS']
+        ):
+            url = f'{meta_url}#discussion-{discussion.id}'
+    if not url:
+        url = getattr(discussion, 'external_url', None)
+    return url
+
+
+def get_subject_type(discussion):
+    if (meta_name := discussion.extras.get('notification', {}).get('model_name')):
+        if meta_name in current_app.config['DISCUSSION_ALTERNATE_MODEL_NAMES']:
+            return meta_name
+    return discussion.subject.verbose_name
+
+
 @connect(on_new_discussion, by_id=True)
 def notify_new_discussion(discussion_id):
     discussion = Discussion.objects.get(pk=discussion_id)
-    if isinstance(discussion.subject, (Dataset, Reuse, Post)):
+    if isinstance(discussion.subject, (Dataset, Reuse, Post, Topic)):
         recipients = owner_recipients(discussion)
+        subject_type = get_subject_type(discussion)
         subject = _('Your %(type)s have a new discussion',
-                    type=discussion.subject.verbose_name)
-        mail.send(subject, recipients, 'new_discussion',
-                  discussion=discussion, message=discussion.discussion[0])
+                    type=subject_type)
+        external_url = get_external_url(discussion)
+        if external_url:
+            mail.send(
+                subject,
+                recipients,
+                'new_discussion',
+                discussion=discussion,
+                message=discussion.discussion[0],
+                external_url=external_url,
+                subject_type=subject_type
+            )
+        else:
+            log.warning(f'No external url could be computed for discussion {discussion.id}')
     else:
         log.warning('Unrecognized discussion subject type %s',
                     type(discussion.subject))
@@ -40,15 +80,25 @@ def notify_new_discussion(discussion_id):
 def notify_new_discussion_comment(discussion_id, message=None):
     discussion = Discussion.objects.get(pk=discussion_id)
     message = discussion.discussion[message]
-    if isinstance(discussion.subject, (Dataset, Reuse, Post)):
+    if isinstance(discussion.subject, (Dataset, Reuse, Post, Topic)):
         recipients = owner_recipients(discussion) + [
             m.posted_by for m in discussion.discussion]
         recipients = list({u.id: u for u in recipients if u != message.posted_by}.values())
         subject = _('%(user)s commented your discussion',
                     user=message.posted_by.fullname)
-
-        mail.send(subject, recipients, 'new_discussion_comment',
-                  discussion=discussion, message=message)
+        external_url = get_external_url(discussion)
+        if external_url:
+            mail.send(
+                subject,
+                recipients,
+                'new_discussion_comment',
+                discussion=discussion,
+                message=message,
+                external_url=external_url,
+                subject_type=get_subject_type(discussion)
+            )
+        else:
+            log.warning(f'No external url could be computed for discussion {discussion.id}')
     else:
         log.warning('Unrecognized discussion subject type %s',
                     type(discussion.subject))
@@ -58,13 +108,24 @@ def notify_new_discussion_comment(discussion_id, message=None):
 def notify_discussion_closed(discussion_id, message=None):
     discussion = Discussion.objects.get(pk=discussion_id)
     message = discussion.discussion[message]
-    if isinstance(discussion.subject, (Dataset, Reuse, Post)):
+    if isinstance(discussion.subject, (Dataset, Reuse, Post, Topic)):
         recipients = owner_recipients(discussion) + [
             m.posted_by for m in discussion.discussion]
         recipients = list({u.id: u for u in recipients if u != message.posted_by}.values())
         subject = _('A discussion has been closed')
-        mail.send(subject, recipients, 'discussion_closed',
-                  discussion=discussion, message=message)
+        external_url = get_external_url(discussion)
+        if external_url:
+            mail.send(
+                subject,
+                recipients,
+                'discussion_closed',
+                discussion=discussion,
+                message=message,
+                external_url=external_url,
+                subject_type=get_subject_type(discussion),
+            )
+        else:
+            log.warning(f'No external url could be computed for discussion {discussion.id}')
     else:
         log.warning('Unrecognized discussion subject type %s',
                     type(discussion.subject))

udata/core/topic/models.py

@@ -1,5 +1,6 @@
 from flask import url_for
 from mongoengine.signals import pre_save
+from udata.i18n import lazy_gettext as _
 from udata.models import db, SpatialCoverage
 from udata.search import reindex
 from udata.tasks import as_task_param
@@ -40,6 +41,8 @@ class Topic(db.Document, Owned, db.Datetimed):
         'queryset_class': OwnedQuerySet,
     }
 
+    verbose_name = _('topic')
+
     def __str__(self):
         return self.name
 

udata/settings.py

@@ -232,7 +232,7 @@ class Defaults(object):
     # A list of tuples, each tuple describing a group with its title and
     # a list of licenses associated. Translations are not supported.
     # Example:
-    # LICENSE_GROUPS = [ 
+    # LICENSE_GROUPS = [
     #     ("Autorités administratives", [
     #         {"value": "lov2", "recommended": True, "description": "Recommandée", "code": "etalab-2.0"},
     #         {"value": "notspecified", "description": "Le Code des relations entre le public et l’administration ne s’applique pas"}]),
@@ -264,7 +264,7 @@ class Defaults(object):
 
     HARVEST_VALIDATION_CONTACT_FORM = None
 
-    HARVEST_MAX_CATALOG_SIZE_IN_MONGO = None # Defaults to the size of a MongoDB document 
+    HARVEST_MAX_CATALOG_SIZE_IN_MONGO = None # Defaults to the size of a MongoDB document
     HARVEST_GRAPHS_S3_BUCKET = None # If the catalog is bigger than `HARVEST_MAX_CATALOG_SIZE_IN_MONGO` store the graph inside S3 instead of MongoDB
     HARVEST_GRAPHS_S3_FILENAME_PREFIX = '' # Useful to store the graphs inside a subfolder of the bucket. For example by setting `HARVEST_GRAPHS_S3_FILENAME_PREFIX = 'graphs/'`
 
@@ -473,6 +473,11 @@ class Defaults(object):
     ###########################################################################
     MATTERMOST_WEBHOOK = None
 
+    # Discussion settings
+    ###########################################################################
+    DISCUSSION_ALLOWED_EXTERNAL_DOMAINS = ['*.data.gouv.fr']
+    DISCUSSION_ALTERNATE_MODEL_NAMES = []
+
 
 class Testing(object):
     '''Sane values for testing. Should be applied as override'''

udata/templates/mail/discussion_closed.html

@@ -4,7 +4,7 @@
 {% block body %}
 <p style="margin: 0;padding: 0;">{{
     _('%(user)s closed an discussion on your %(type)s %(subject)s',
-        type=discussion.subject.verbose_name,
+        type=subject_type,
         user=(
             '<a href="'|safe
             + url_for('api.user', user=message.posted_by, _external=True)
@@ -14,7 +14,7 @@
         ),
         subject=(
             '<a href="'|safe
-            + discussion.external_url
+            + external_url
             + '">'|safe
             + discussion.subject|string
             + '</a>'|safe
@@ -38,7 +38,7 @@
         <td align="center">
             {{ mail_button(
                 _('See the discussion'),
-                discussion.external_url
+                external_url
             ) }}
         </td>
     </tr>

udata/templates/mail/discussion_closed.txt

@@ -2,7 +2,7 @@
 
 {% block body %}
 {{ _('%(user)s closed an discussion on your %(type)s %(subject)s',
-    type=discussion.subject.verbose_name,
+    type=subject_type,
     user=message.posted_by.fullname,
     subject=discussion.subject|string
 ) }}.
@@ -12,5 +12,5 @@
 
 
 {{ _('You can see the discussion on this page:') }}
-{{ discussion.external_url }}
+{{ external_url }}
 {% endblock %}

udata/templates/mail/new_discussion.html

@@ -4,7 +4,7 @@
 {% block body %}
 <p style="margin: 0;padding: 0;">{{
     _('%(user)s submitted a new discussion on your %(type)s %(subject)s',
-    type=discussion.subject.verbose_name,
+    type=subject_type,
     user=(
         '<a href="'|safe
         + url_for('api.user', user=discussion.user, _external=True)
@@ -14,7 +14,7 @@
     ),
     subject=(
         '<a href="'|safe
-        + discussion.external_url
+        + external_url
         + '">'|safe
         + discussion.subject|string
         + '</a>'|safe
@@ -36,7 +36,7 @@
         <td align="center">
             {{ mail_button(
                 _('See the discussion'),
-                discussion.external_url
+                external_url
             ) }}
         </td>
     </tr>

udata/templates/mail/new_discussion.txt

@@ -2,7 +2,7 @@
 
 {% block body %}
 {{ _('%(user)s submitted a new discussion on your %(type)s %(subject)s',
-        type=discussion.subject.verbose_name,
+        type=subject_type,
         user=discussion.user.fullname,
         subject=discussion.subject|string
 ) }}.
@@ -11,5 +11,5 @@
 {{ _('Title') }}: {{ discussion.title }}
 
 {{ _('You can see the discussion on this page:') }}
-{{ discussion.subject.external_url }}
+{{ external_url }}
 {% endblock %}

udata/templates/mail/new_discussion_comment.html

@@ -4,7 +4,7 @@
 {% block body %}
 <p style="margin: 0;padding: 0;">{{
     _('%(user)s commented an discussion on your %(type)s %(subject)s',
-        type=discussion.subject.verbose_name,
+        type=subject_type,
         user=(
             '<a href="'|safe
             + url_for('api.user', user=message.posted_by, _external=True)
@@ -14,7 +14,7 @@
         ),
         subject=(
             '<a href="'|safe
-            + discussion.external_url
+            + external_url
             + '">'|safe
             + discussion.subject|string
             + '</a>'|safe
@@ -37,7 +37,7 @@
         <td align="center">
             {{ mail_button(
                 _('See the discussion'),
-                discussion.external_url
+                external_url
             ) }}
         </td>
     </tr>

udata/templates/mail/new_discussion_comment.txt

@@ -2,7 +2,7 @@
 
 {% block body %}
 {{ _('%(user)s commented an discussion on your %(type)s %(subject)s',
-    type=discussion.subject.verbose_name,
+    type=subject_type,
     user=message.posted_by.fullname,
     subject=discussion.subject|string
 ) }}.
@@ -12,5 +12,5 @@
 
 
 {{ _('You can see the discussion on this page:') }}
-{{ discussion.external_url }}
+{{ external_url }}
 {% endblock %}

udata/tests/test_discussions.py

@@ -19,6 +19,7 @@
 from udata.core.dataset.factories import DatasetFactory
 from udata.core.organization.factories import OrganizationFactory
 from udata.core.user.factories import UserFactory, AdminFactory
+from udata.core.topic.factories import TopicFactory
 from udata.tests.helpers import capture_mails
 from udata.utils import faker
 
@@ -138,7 +139,7 @@ def test_spam_by_owner(self):
                 }
             })
             self.assertStatus(response, 201)
-            
+
         with assert_not_emit(on_new_potential_spam):
             response = self.post(url_for('api.discussion', id=response.json['id']), {
                 'comment': 'A comment with spam by owner'
@@ -735,6 +736,130 @@ def test_new_discussion_mail(self):
         self.assertEqual(len(mails), 1)
         self.assertEqual(mails[0].recipients[0], owner.email)
 
+    @pytest.mark.options(
+        DISCUSSION_ALLOWED_EXTERNAL_DOMAINS=['*.example.com'],
+        DISCUSSION_ALTERNATE_MODEL_NAMES=['CUSTOM_MODEL']
+    )
+    def test_new_discussion_mail_with_extras(self):
+        user = UserFactory()
+        owner = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=user)
+        discussion = Discussion.objects.create(
+            # Topic will be able to nofify if an external url is provided
+            subject=TopicFactory(owner=owner),
+            user=user,
+            title=faker.sentence(),
+            discussion=[message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion(discussion.id)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' in mail.subject
+        assert 'CUSTOM_MODEL' in mail.body
+        assert f'https://sub.example.com/custom/#discussion-{discussion.id}' in mail.body
+
+    @pytest.mark.options(
+        DISCUSSION_ALLOWED_EXTERNAL_DOMAINS=['*.example.com'],
+    )
+    def test_new_discussion_mail_with_extras_no_model_name(self):
+        user = UserFactory()
+        owner = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=user)
+        discussion = Discussion.objects.create(
+            # Topic will be able to nofify if an external url is provided
+            subject=TopicFactory(owner=owner),
+            user=user,
+            title=faker.sentence(),
+            discussion=[message],
+            extras={
+                'notification': {
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion(discussion.id)
+
+        mail = mails[0]
+        assert 'topic' in mail.subject
+        assert f'https://sub.example.com/custom/#discussion-{discussion.id}' in mail.body
+
+    def test_new_discussion_mail_with_extras_not_allowed(self):
+        user = UserFactory()
+        owner = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=user)
+        discussion = Discussion.objects.create(
+            # Topic has no external_url, so we're testing this case with Dataset
+            subject=DatasetFactory(owner=owner),
+            user=user,
+            title=faker.sentence(),
+            discussion=[message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion(discussion.id)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' not in mail.subject
+        assert 'https://sub.example.com/custom/' not in mail.body
+
+    def test_new_discussion_mail_with_extras_not_an_url(self):
+        user = UserFactory()
+        owner = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=user)
+        discussion = Discussion.objects.create(
+            # Topic has no external_url, so we're testing this case with Dataset
+            subject=DatasetFactory(owner=owner),
+            user=user,
+            title=faker.sentence(),
+            discussion=[message],
+            extras={
+                'notification': {
+                    'external_url': 'xxx'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion(discussion.id)
+
+        mail = mails[0]
+        assert 'xxx' not in mail.body
+        assert f"/{discussion.subject.slug}/#" in mail.body
+
+    def test_new_discussion_mail_topic_no_extras(self):
+        user = UserFactory()
+        owner = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=user)
+        discussion = Discussion.objects.create(
+            subject=TopicFactory(owner=owner),
+            user=user,
+            title=faker.sentence(),
+            discussion=[message],
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion(discussion.id)
+
+        # Topic has no external_url, no email should be sent and no error thrown
+        assert len(mails) == 0
+
+
     def test_new_discussion_comment_mail(self):
         owner = UserFactory()
         poster = UserFactory()
@@ -761,6 +886,80 @@ def test_new_discussion_comment_mail(self):
             self.assertIn(mail.recipients[0], expected_recipients)
             self.assertNotIn(commenter.email, mail.recipients)
 
+    @pytest.mark.options(
+        DISCUSSION_ALLOWED_EXTERNAL_DOMAINS=['*.example.com'],
+        DISCUSSION_ALTERNATE_MODEL_NAMES=['CUSTOM_MODEL']
+    )
+    def test_new_discussion_comment_mail_with_extras(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        new_message = Message(content=faker.sentence(), posted_by=commenter)
+        discussion = Discussion.objects.create(
+            # Topic will be able to nofify if an external url is provided
+            subject=TopicFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[new_message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion_comment(discussion.id, message=0)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' in mail.body
+        assert f'https://sub.example.com/custom/#discussion-{discussion.id}' in mail.body
+
+    def test_new_discussion_comment_mail_with_extras_not_allowed(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        new_message = Message(content=faker.sentence(), posted_by=commenter)
+        discussion = Discussion.objects.create(
+            # Topic has no external_url, so we're testing this case with Dataset
+            subject=DatasetFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[new_message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion_comment(discussion.id, message=0)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' not in mail.body
+        assert 'https://sub.example.com/custom/' not in mail.body
+
+    def test_new_discussion_comment_mail_topic_no_extras(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        new_message = Message(content=faker.sentence(), posted_by=commenter)
+        discussion = Discussion.objects.create(
+            subject=TopicFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[new_message],
+        )
+
+        with capture_mails() as mails:
+            notify_new_discussion_comment(discussion.id, message=0)
+
+        # Topic has no external_url, no email should be sent and no error thrown
+        assert len(mails) == 0
+
     def test_closed_discussion_mail(self):
         owner = UserFactory()
         poster = UserFactory()
@@ -785,3 +984,79 @@ def test_closed_discussion_mail(self):
         for mail in mails:
             self.assertIn(mail.recipients[0], expected_recipients)
             self.assertNotIn(owner.email, mail.recipients)
+
+    @pytest.mark.options(
+        DISCUSSION_ALLOWED_EXTERNAL_DOMAINS=['*.example.com'],
+        DISCUSSION_ALTERNATE_MODEL_NAMES=['CUSTOM_MODEL']
+    )
+    def test_closed_discussion_mail_with_extras(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=commenter)
+        closing_message = Message(content=faker.sentence(), posted_by=owner)
+        discussion = Discussion.objects.create(
+            subject=TopicFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[message, closing_message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_discussion_closed(discussion.id, message=0)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' in mail.body
+        assert f'https://sub.example.com/custom/#discussion-{discussion.id}' in mail.body
+
+    def test_closed_discussion_mail_with_extras_not_allowed(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=commenter)
+        closing_message = Message(content=faker.sentence(), posted_by=owner)
+        discussion = Discussion.objects.create(
+            # Topic has no external_url, so we're testing this case with Dataset
+            subject=DatasetFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[message, closing_message],
+            extras={
+                'notification': {
+                    'model_name': 'CUSTOM_MODEL',
+                    'external_url': 'https://sub.example.com/custom/'
+                }
+            },
+        )
+
+        with capture_mails() as mails:
+            notify_discussion_closed(discussion.id, message=0)
+
+        mail = mails[0]
+        assert 'CUSTOM_MODEL' not in mail.body
+        assert 'https://sub.example.com/custom/' not in mail.body
+
+    def test_closed_discussion_mail_topic_no_extras(self):
+        owner = UserFactory()
+        poster = UserFactory()
+        commenter = UserFactory()
+        message = Message(content=faker.sentence(), posted_by=commenter)
+        closing_message = Message(content=faker.sentence(), posted_by=owner)
+        discussion = Discussion.objects.create(
+            subject=TopicFactory(owner=owner),
+            user=poster,
+            title=faker.sentence(),
+            discussion=[message, closing_message],
+        )
+
+        with capture_mails() as mails:
+            notify_discussion_closed(discussion.id, message=0)
+
+        # Topic has no external_url, no email should be sent and no error thrown
+        assert len(mails) == 0