feat(notifications): implement email reminder use case #4216
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thevaibhav-dixit
force-pushed
the
onboarding-messages
branch
from
63f9829 to
6f3630a
Compare
core/notifications/src/primitives.rs
Outdated
| @@ -120,6 +120,7 @@ pub enum UserNotificationCategory { | |||
| AdminNotification, | |||
| Marketing, | |||
| Price, | |||
| Onboarding, | |||
There was a problem hiding this comment.
This is not Onboarding - its security right?
There was a problem hiding this comment.
could fit in both categories
thevaibhav-dixit
force-pushed
the
onboarding-messages
branch
from
3a76453 to
33968fc
Compare
UncleSamtoshi
force-pushed
the
onboarding-messages
branch
from
8a805ec to
2ae3397
Compare
core/notifications/src/job/mod.rs
Outdated
| ) -> Result<(), JobError> { | ||
| let data = data.into(); | ||
| // TODO: I think reusing the id here might cause a bug | ||
| match JobBuilder::new_with_id(LINK_EMAIL_REMINDER_ID, "link_email_reminder") |
There was a problem hiding this comment.
Since we are recursively calling back into this for pagination I think this may be a bug @thevaibhav-dixit @bodymindarts
UncleSamtoshi
force-pushed
the
onboarding-messages
branch
from
2ae3397 to
28aa0ae
Compare
Comment on lines
19
to
21
| const ACCOUNT_LIVENESS_THRESHOLD_DAYS: i64 = 21; | ||
| const ACCOUNT_AGED_THRESHOLD_DAYS: i64 = 21; | ||
| const NOTIFICATION_COOL_OFF_THRESHOLD_DAYS: i64 = 90; |
There was a problem hiding this comment.
why not directly convert to duration here ?
| @@ -74,6 +74,7 @@ impl From<UserNotificationCategory> for proto::NotificationCategory { | |||
| } | |||
| UserNotificationCategory::Marketing => proto::NotificationCategory::Marketing, | |||
| UserNotificationCategory::Price => proto::NotificationCategory::Price, | |||
| _ => unimplemented!(), | |||
| tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, | ||
| search_id: GaloyUserId, | ||
| ) -> Result<(Vec<GaloyUserId>, bool), EmailReminderProjectionError> { | ||
| let last_transaction_at_threshold = Utc::now() |
There was a problem hiding this comment.
I would put math into SQL query
| const ACCOUNT_AGED_THRESHOLD_DAYS: i64 = 21; | ||
| const NOTIFICATION_COOL_OFF_THRESHOLD_DAYS: i64 = 90; | ||
|
|
||
| pub fn new(pool: &PgPool) -> Self { |
thevaibhav-dixit
force-pushed
the
onboarding-messages
branch
from
1eb2d80 to
cf9b414
Compare
thevaibhav-dixit
changed the title
core/notifications/locales/en.yml
Outdated
| @@ -60,3 +60,6 @@ transaction.paid_invoice.body_display_currency: "+%{displayCurrencyAmount} | %{f | |||
|
|
|||
| price_changed.title: "Bitcoin is on the move!" | |||
| price_changed.body: "Bitcoin is up %{percent_increase}% in the last day to %{price}!" | |||
|
|
|||
| onboarding.link_email_reminder.title: "Link Email to Secure Account" | |||
There was a problem hiding this comment.
note: change this to security
thevaibhav-dixit
force-pushed
the
onboarding-messages
branch
from
6da7742 to
b13f801
Compare
bodymindarts
previously approved these changes
Mar 22, 2024
bodymindarts
approved these changes
Mar 22, 2024
This was referenced Mar 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.