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.

Sign up for GitHub

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

Jump to bottom

Support custom title and body for email notifications #476

Merged
merged 8 commits into from
Aug 1, 2024
Merged

Support custom title and body for email notifications #476

merged 8 commits into from
Aug 1, 2024

Conversation

pvannierop
Copy link
Contributor

Problems

  • At present, notifications via email use the title and body for push notifications
  • Documentation on how to enable email notifications is missing
  • Bug: storing of email adress of subjects does not work correctly

Solutions

  • This PR will implement email-specific title and body
  • Add documentation (only for emails scheduled via the notification endpoint)
  • Fix the problem with storing User email addresses.

Notes

  • Docs on sending emails via the NotificationProtocol were not added. An issue was created for this.
  • HTLM email is not supported at this moment.

@pvannierop pvannierop self-assigned this Jul 23, 2024
@pvannierop pvannierop mentioned this pull request Jul 23, 2024
Open
@pvannierop pvannierop changed the base branch from master to dev July 23, 2024 06:57
yatharthranjan
yatharthranjan reviewed Jul 24, 2024
View reviewed changes
...ain/resources/db/changelog/changes/00000000000002_update_schema-20240704090100_changelog.yml Outdated
@@ -6,6 +6,6 @@ databaseChangeLog:
- addColumn:
columns:
- column:
name: email
name: email_address
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the User entity code, this is still referenced as @Column(name = "email"), so was it correct previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Fixed!

yatharthranjan
yatharthranjan approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@yatharthranjan yatharthranjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@pvannierop pvannierop merged commit 7f37c89 into dev Aug 1, 2024
4 checks passed
@Bdegraaf1234 Bdegraaf1234 deleted the notifications-via-email branch August 2, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yatharthranjan yatharthranjan yatharthranjan approved these changes

Assignees

@pvannierop pvannierop

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pvannierop @yatharthranjan