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

Allow customizing notification templates per project #3369

Merged
merged 10 commits into from
May 15, 2023

Conversation

vindex10
Copy link
Contributor

@vindex10 vindex10 commented May 7, 2023

There is a way to have customizable templates for dispatch today, by mounting a volume with a customized templates dir as part of the docker compose:

  web:
    image: dispatch-local
    ...
    volumes:
      - "../dispatch-templates/messaging-email-templates:/usr/local/lib/python3.11/site-packages/dispatch/messaging/email/templates"

This approach does allow to customize the templates "deployment"-wide.

In this PR I suggest a small extension of this functionality. You can create a folder per project (identified by project id):

dispatch/messaging/email/templates/project_id/<project_id>/base.mjml

This will be used at the first place if exists, otherwise will gracefully fall back to the default template like "/templates/base.mjml".

I was a bit afraid to use "slug" of the project name, because this requires more careful work on escaping system paths and making sure we don't expose something by engineering a weird project name. This can be expanded in future though, by introducing another subdir "templates/project_slug/<project_slug>".

@kevgliss kevgliss added the enhancement New feature or request label May 8, 2023
@kevgliss
Copy link
Contributor

kevgliss commented May 8, 2023

This looks great @vindex10! As this approach fallbacks gracefully, I see no reason why it can't be merged. Thanks for the contribution.

As far as documentation, we don't currently have a good space for something like this. Maybe we could create new page for "Messaging" related settings here: https://netflix.github.io/dispatch/docs/administration/settings ?

@vindex10
Copy link
Contributor Author

vindex10 commented May 8, 2023

Sounds good! I could also include information about the recent markdown enhancement in there :)

src/dispatch/messaging/email/utils.py Show resolved Hide resolved
src/dispatch/messaging/email/utils.py Show resolved Hide resolved
src/dispatch/messaging/email/utils.py Outdated Show resolved Hide resolved
@mvilanova
Copy link
Contributor

@vindex10 can you run black against the files you changed? It will fix the formatting issues the lint tests are complaining about.

@vindex10
Copy link
Contributor Author

Yes, of course, i will change this together with adding the page to the docs!

@vindex10 vindex10 requested a review from mvilanova May 13, 2023 00:35
@vindex10
Copy link
Contributor Author

Could you rerun the playwright tests please? They seem to have failed because of the network problem

@kevgliss
Copy link
Contributor

I think this looks good @vindex10 if you're happy with it I can merge it in.

@vindex10
Copy link
Contributor Author

I definitely am, @kevgliss :) please do! Thank you!

@kevgliss kevgliss merged commit 5a05be2 into Netflix:master May 15, 2023
@kevgliss
Copy link
Contributor

Thanks! This will be added in our next release.

vindex10 added a commit to LetsData-net/dispatch that referenced this pull request May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants