feat(notifs): move organization notification settings to the user level#12131
Open
maximevast wants to merge 19 commits into
Open
feat(notifs): move organization notification settings to the user level#12131maximevast wants to merge 19 commits into
maximevast wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
OpenAPI ChangesOperations
Schemas
|
Contributor
|
Preview Environment |
maximevast
changed the title
maximevast
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 13 files
Confidence score: 4/5
- This PR looks safe to merge overall, with moderate confidence issues limited to API typing completeness rather than a clear runtime break.
- In
clients/packages/client/src/v1.ts, both GET and PATCH notification-settings operations are missing documented 404 response types, which can create a client contract mismatch and weaker error handling for non-member access paths. - Because both findings are severity 5/10 and focused on typed response coverage, the risk appears manageable but worth addressing soon to keep generated client behavior aligned with the API spec.
- Pay close attention to
clients/packages/client/src/v1.ts- add the missing 404 response typings for notification-settings GET/PATCH to prevent contract drift.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| } | ||
| }, | ||
| ) | ||
| async def update_my_notification_settings( |
Contributor
There was a problem hiding this comment.
Big nit-pick, but if GET | PATCH /me methods are called get|update_authenticated, then maybe we should name these methods
update_authenticated_notification_settings?
Contributor
Author
There was a problem hiding this comment.
good point, clauding now...
pieterbeulque
approved these changes
Jun 3, 2026
…er)/settings/SettingsPage.tsx Co-authored-by: Pieter Beulque <pieterbeulque@gmail.com>
…arsource/polar into maxime/user-org-notifs-settings-wire
Contributor
|
🚀 Expo preview is ready!
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
**Related Issue: #12101
What
Moves organization notification settings (
new_order,new_subscription)from the org level to the user level, so each member controls their own
emails instead of one shared org-wide toggle.
Why
Adding granularity to the notifications settings; allowing users independently enable or disable notifications for an organisation.
How
GET/PATCH /users/me/organizations/{id}/notification-settingsendpoints; dispatch now filters per-member in
send_to_org_members.members (soft-deleted included).
Rollout (no downtime)
The new column is nullable for now. While it's
NULL, both backend dispatchand the frontend fall back to the org-level value, so behavior is unchanged
until the backfill runs.
Follow-up PR (cleanup): run the backfill, make the column non-null, drop
the org-level fallbacks, and remove the org-level
notification_settingscolumn. Marked in-code with
TODO (maxime).Tests
Service-level filtering + fallback (
tests/notifications/test_service.py) andthe endpoint contract incl. 404 on non-member (
tests/user/test_endpoints.py).Checklist
uv run task lint && uv run task lint_types)Summary by cubic
Move organization notification settings to the user level so each member controls emails for new orders and subscriptions. Web and mobile now read/write per-user preferences, and notification delivery filters per member.
New Features
/v1/users/me/organizations/{organization_id}/notification-settingswith 404 for non‑members; OpenAPI operations use “authenticated” naming; endpoints useAuthorizeUser;@polar-sh/clientupdated; new hooksuseUserOrganizationNotificationSettingsanduseUpdateUserOrganizationNotificationSettings.Migration
server/scripts/backfill_user_organization_notification_settings.pyto copy org-level settings to memberships.Written for commit 1072761. Summary will update on new commits.