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

Send notification based on a future datetime #625

Merged
merged 5 commits into from
Jun 19, 2020
Merged

Send notification based on a future datetime #625

merged 5 commits into from
Jun 19, 2020

Conversation

mikecasas
Copy link
Contributor

This PR adds the ability to send a notification based on a future datetime. The logic uses a new field in the Notification table named SendOn. To keep the current functionality with no breaking changes, the SendOn date is defaulted to UtcNow in the framework.

In the Notificaiton repository class, an additional WHERE cause is added to GetNotifications so the List/IEnumerable of Notifications only includes notifications with a SendOn date before now.

A developer would use this by creating their own Notification object and adding a send on date that meets their needs.

Not sure why the email address was not being added to the notification.
@sbwalker
Copy link
Member

sbwalker commented Jun 18, 2020

Framework upgrade scripts need to follow a specific naming convention - Master.##.##.##.##.sql for Master DB scripts and Tenant.##.##.##.##.sql for Tenant DB scripts. The initial ##.##.## corresponds to the framework version they belong to, and the last ## is a sequential patch number to maintain ordering. The upgrade script for the Notification table change should be Tenant.01.00.01.01.sql.

@sbwalker
Copy link
Member

should the update script also contain an initialization command to set the SendOn field to the CreatedOn date ( so that any existing records have valid values ) ?

@mikecasas
Copy link
Contributor Author

I thought about the update, but I figured any existing notifications would have already been sent. I can add it though. I will update the script name too.

@sbwalker sbwalker merged commit 65df054 into oqtane:master Jun 19, 2020
@thabaum
Copy link
Contributor

thabaum commented Jun 21, 2020

@mikecasas thank you for holding off on this one till my PR went through. I appreciate it.

@mikecasas mikecasas deleted the feature-email branch June 22, 2020 10:00
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.

3 participants