-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Feature/disable email notifications #903
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
base: main
Are you sure you want to change the base?
Feature/disable email notifications #903
Conversation
@Eshwar1212-maker is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the ability for users to toggle email notifications on and off through the Settings UI. It extends the existing backend email notification infrastructure by connecting it to a new frontend component that allows users to control their email preferences.
Key Changes
- Added
emailNotifications
field to the User model in the database schema with a default value oftrue
- Updated the email sending logic to respect user preferences before sending notifications
- Created a new Settings UI component for managing email notification preferences
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
libraries/nestjs-libraries/src/database/prisma/schema.prisma |
Added emailNotifications boolean field to User model |
libraries/nestjs-libraries/src/database/prisma/users/users.service.ts |
Added service methods for getting/updating email notification preferences |
libraries/nestjs-libraries/src/database/prisma/users/users.repository.ts |
Added repository methods for email notification database operations |
libraries/nestjs-libraries/src/database/prisma/organizations/organization.repository.ts |
Updated query to include emailNotifications field when fetching user data |
libraries/nestjs-libraries/src/database/prisma/notifications/notification.service.ts |
Updated email sending logic to check user preferences before sending |
apps/backend/src/api/routes/settings.controller.ts |
Added API endpoints for getting and updating email notification preferences |
apps/frontend/src/components/settings/email.notifications.component.tsx |
New React component for email notification settings UI |
apps/frontend/src/components/layout/settings.component.tsx |
Integrated email notifications component into settings layout |
Comments suppressed due to low confidence (4)
libraries/nestjs-libraries/src/database/prisma/users/users.service.ts:60
- Missing closing brace for the getEmailNotifications method. The method should be properly closed with a closing brace.
return this._usersRepository.getEmailNotifications(id);
libraries/nestjs-libraries/src/database/prisma/users/users.repository.ts:123
- Missing closing brace for the getEmailNotifications method. The method should be properly closed with a closing brace.
return this._user.model.user.findUnique({
libraries/nestjs-libraries/src/database/prisma/notifications/notification.service.ts:88
- Missing closing brace for the for loop. The loop should be properly closed after the if statement.
}
apps/backend/src/api/routes/settings.controller.ts:145
- [nitpick] Variable name 'userValue' is ambiguous. Consider renaming to 'userSettings' or 'emailSettings' to better describe what the variable contains.
const userValue = await this._usersService.getEmailNotifications(user.id);
libraries/nestjs-libraries/src/database/prisma/notifications/notification.service.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…otification.service.ts Co-authored-by: Copilot <[email protected]>
Thanks for the PR, I would love to have an option to receive email notifications for failed posts but not for successful posts. |
@Eshwar1212-maker, thank you! |
Also, it's better to call the menu items notifications, and also add options for the in-app |
Yup, I'll fix that today later when I'm free! |
Hey! Thanks for the feedback I’ve updated the UI section title to “Notifications” and kept the current toggle for email notifications under that section, as discussed. I just pushed this part . Just to clarify — the original ticket mentioned using a single boolean ( And about failed/successful post preferences, are you saying that I should get rid of everything I implemented and redo it so users can just have options for failed/successful post preferences? I wanna clarify this because it goes against what's in the original ticket. And then also on top of that you want me to add a toggle for general notifications? My current implementation followed the ticket word for word just for email notifications. I just wanna clarify if what I did is fine, but if you want me to add all the different features, you're asking it's almost a whole new ticket isn't it? Thank you. |
What kind of change does this PR introduce?
Feature — Adds email notification toggling for users in the Settings UI to opt out of emails. This connects the existing backend logic (sendEmailToOrg) to the frontend with a working Button component.
eg: Bug fix, feature, docs update, ...
Why was this change needed?
This is an active issue requested by a user, I will link it here:
#163
Other information:
I wanted to open a new request in the 'community support tickets' page on discord but currently don't have any permission.
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Screen.Recording.2025-07-29.at.6.34.31.PM.mov