feat(notifications-v2): Read from notification settings for weekly reports #56621
Conversation
This file contains hidden or 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
github-actions
bot
added
the
Scope: Backend
scefali
reviewed
Sep 20, 2023
Codecov Report
@@ Coverage Diff @@
## master #56621 +/- ##
==========================================
- Coverage 78.63% 78.63% -0.01%
==========================================
Files 5083 5084 +1
Lines 219150 219178 +28
Branches 37102 37113 +11
==========================================
+ Hits 172336 172343 +7
- Misses 41230 41252 +22
+ Partials 5584 5583 -1
|
scefali
reviewed
Sep 20, 2023
| @@ -553,3 +553,30 @@ def get_notification_provider_value_for_recipient_and_type( | |||
| return value | |||
|
|
|||
| return NotificationSettingsOptionEnum.NEVER | |||
|
|
|||
| def get_users_for_weekly_reports(self) -> list[int]: | |||
There was a problem hiding this comment.
@snigdhas Seems like we can use a combination of get_notification_value_for_recipient_and_type and get_notification_provider_value_for_recipient_and_type to make this logic super simple, right?
There was a problem hiding this comment.
We could, but we'd end up calling self._get_layered_setting_options N times for N users, and each call would do iterate over the entire dict. We could store the layered settings in memory instead of computing it but I'm not sure that's necessary
scefali
approved these changes
Sep 21, 2023
michellewzhang
pushed a commit
that referenced
this pull request
Sep 21, 2023
…ports (#56621) Read from the new notifications tables for weekly reports instead of user options.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Read from the new notifications tables for weekly reports instead of user options.