Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: allow discussion notification customization #3072

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abulte
Copy link
Contributor

@abulte abulte commented Jul 1, 2024

Adds customizations for discussion notification emails:

  • external_url: the URL the notification email points to
  • model_name: the model name used in the notification email

Also allows notifications for Topic.

The expected payload is in Discussion.extras. It will be used for subsequent notifications of comments on discussion and discussion closing. A comment made on a discussion pointing to an external site will always point to this external site, even if made on another frontend.

{
  "notification": {
    "model_name": "bouquet",
    "external_url": "https://ecologie.data.gouv.fr/bouquets/xxx"
  }
}

Both parameters are protected through an access list, to avoid ill-intentioned people injecting links or other arbitrary text in the notification emails:

  • DISCUSSION_ALLOWED_EXTERNAL_DOMAINS (accepts wildcards)
  • DISCUSSION_ALTERNATE_MODEL_NAMES

Both parameters can be omitted, the default behavior will apply. For Topic, since there's no detail page on data.gouv.fr (yet), no notification will be sent if external_url is not supplied.

NB1: there's a grammatical error in the English notification text Your %(type)s have a new discussion, not fixed here because it would involve updating the translations.
NB2: the translation of model_name is not handled. This is not a problem for ecologie.data.gouv.fr and might not be easy to do (how to add a translation for something variable? or include translations in payload?).

Cf ecolabdata/ecospheres#263 for requirements and more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant