honor mutes when sending push notifications #1145
Conversation
SatsAllDay
changed the title
| @@ -1227,7 +1227,7 @@ export const createMentions = async (item, models) => { | |||
|
|
|||
| // only send if mention is new to avoid duplicates | |||
| if (mention.createdAt.getTime() === mention.updatedAt.getTime()) { | |||
| notifyMention(user.id, item) | |||
| notifyMention({ models, userId: user.id, item }) | |||
There was a problem hiding this comment.
I considered altering the surrounding code in this context to only invoke this notifyMention function for users that didn't mute OP, but instead I kept that logic within notifyMention for consistency with the other notifyXXX functions. It's slightly dangerous to assume that the caller of notifyXXX is doing the mute filtering, IMO.
It would probably be more performant to do it here, instead of doing a DB call for each mentioned user, so I am open to refactoring if we think that's a major concern. But for consistency and all that, I did it this way.
api/resolvers/user.js
Outdated
| @@ -112,6 +112,19 @@ export function viewValueGroup () { | |||
| ) vv` | |||
| } | |||
|
|
|||
| const isMuted = async ({ models, me, mutedId }) => { | |||
There was a problem hiding this comment.
Extracted this to a reusable function for code de-dupe.
* territory subscriptions * mentions * user subscriptions Also, don't allow you to mute a subscribed user, or vice versa
update mute/subscribe error messages for consistency
WalkthroughWalkthroughThe recent updates aim to improve the notification system by enhancing mute functionality across various components. Changes involve passing structured data for notification functions, consolidating mute logic in a shared library, and enhancing error handling in UI components. These modifications ensure consistent respect for user muting preferences throughout the application. Changes
Assessment against linked issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
SatsAllDay
changed the title
Description
Closes #717
This PR updates the push notification code paths to ensure that user-configured mutes are checked before sending push notifications, if applicable. The following code paths were updated:
This PR also updates the API validation to disallow users from muting and subscribing to the same user - only one or the other is allowed. No migration is performed, so if any user got themselves into this state, they can keep it until they change it. Then they can't go back.
In order to explain this behavior to users, I updated the UI code that toggles subscriptions/mutes to show the error message from the API request in the error toast, instead of a generic "Failed to <mute|subscribe>" message.
Screenshots
N/A
Additional Context
N/A
Checklist
Are your changes backwards compatible? Please answer below:
Yes. No database changes or migrations. The only notable change is that users will no longer be able to mute and subscribe to the same user. However, if they've already done that, it will still be honored. They just can't re-configure such a thing.
Did you QA this? Could we deploy this straight to production? Please answer below:
Yes.
For frontend changes: Tested on mobile? Please answer below:
N/A
Did you introduce any new environment variables? If so, call them out explicitly here:
No.