coderabbitai[bot]
commented
1 month ago Walkthrough
## Walkthrough The 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 | Files | Summary of Changes | |-------|--------------------| | `api/resolvers/item.js` | Updated `createMentions` function to pass structured data for `notifyMention` function. | | `api/resolvers/user.js` | Added `isMuted` import and implemented mute checks in various functions. | | `components/mute.js` | Enhanced error handling for muting/unmuting operations. | | `components/subscribeUser.js` | Improved error message handling for subscribing/unsubscribing users. | | `lib/user.js` | Introduced `isMuted` function to check if a user is muted. | | `lib/webPush.js` | Modified `notifyUserSubscribers` to exclude muted users and updated `notifyMention` for mute checks. | ## Assessment against linked issues | Objective | Addressed | Explanation | |-----------|-----------|-------------| | Respect mutes all for push notifications (#717) | ✅ | The changes, especially the introduction of `isMuted` function and its integration in relevant functions, ensure that mute preferences are respected for all push notifications, extending beyond just replies as per the objective. |Recent Review Details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**Commits
Files that changed from the base of the PR and between 46318fe11767134444f97a8d0198549b15aa6360 and cc725c6911c16e52a2125dd1ebb1dfede0e0b5ed.Files ignored due to path filters (1)
* `awards.csv` is excluded by `!**/*.csv`, `!**/*.csv`Files selected for processing (1)
* api/resolvers/item.js (1 hunks)Additional comments not posted (1)
api/resolvers/item.js (1)
`1234-1234`: LGTM! The change enhances the context passed to `notifyMention`, aligning with the PR's objectives. Consider the performance impact of DB calls for each mentioned user as noted in previous discussions.
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.