[WEB-2383] feat: Inbox Settings v2 #6644
Conversation
WalkthroughThis PR extends the notification system to support workspace-specific user preferences. It introduces a new serializer and model for workspace notifications, new endpoints (GET and PATCH) to manage these preferences, and updates to URL routing and view logic. Additionally, it adds background task functions for mention processing, a MobX store with hooks and services to manage notification settings, multiple new React components for the UI, and expanded localization and TypeScript definitions for notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant E as Workspace Endpoint
participant DB as Database/Store
participant S as Serializer
%% GET Flow:
C->>E: GET /workspaces/{slug}/user-notification-preferences
E->>DB: Query notification preferences
DB-->>E: Return preferences
E->>S: Serialize preferences
S-->>E: Return serialized data
E-->>C: 200 OK with JSON response
%% PATCH Flow:
C->>E: PATCH /workspaces/{slug}/user-notification-preferences/{transport}
E->>DB: Retrieve & validate preference
DB-->>E: Return updated record or error (e.g., 404)
E-->>C: Return success/error response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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:
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
CodeRabbit Configuration File (
|
Saurabhkmr98
changed the title
| ) | ||
| def get(self, request, slug): | ||
| workspace = Workspace.objects.get(slug=slug) | ||
| get_notification_preferences = ( |
There was a problem hiding this comment.
Line 377 to 395 can be clubbed into the following.
existing_transports = UserNotificationPreference.objects.filter(
workspace=workspace, user=request.user
).values_list('transport', flat=True)
create_transports = [
transport
for transport, _ in NotificationTransportChoices.choices
if transport not in existing_transports
]
This will be concise and improve readability of the code.
henit-chobisa
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
web/core/components/inbox/settings/update-setting.tsx (1)
45-51: The toggle switch should be center alignedAs mentioned in the previous review, the toggle switch isn't center aligned in the UI component. Check the alignment to ensure it's visually consistent.
apiserver/plane/app/views/notification/base.py (1)
387-396: Consider succinctly computingcreate_transports().
A past review comment suggested using direct set operations to compute the missing transports, which improves readability. Reusing that approach:- for transport in transports: - if transport not in get_notification_preferences.values_list("transport", flat=True): - create_transports.append(transport) + existing_transports = get_notification_preferences.values_list("transport", flat=True) + create_transports = [ + transport + for transport, _ in NotificationTransportChoices.choices + if transport not in existing_transports + ]
🧹 Nitpick comments (25)
web/core/hooks/store/notifications/index.ts (1)
3-3: LGTM! Consider using double quotes for consistency.The export statement correctly exposes the workspace notification settings hook, but uses single quotes while the rest of the file uses double quotes. Consider standardizing for consistency.
-export * from './use-workspace-notification-settings'; +export * from "./use-workspace-notification-settings";apiserver/plane/app/serializers/notification.py (1)
4-4: Line length exceeds recommended limitThis import line exceeds the recommended maximum line length (105 > 88 characters).
Consider breaking this import into multiple lines for better readability:
-from plane.db.models import Notification, UserNotificationPreference, WorkspaceUserNotificationPreference +from plane.db.models import ( + Notification, + UserNotificationPreference, + WorkspaceUserNotificationPreference, +)🧰 Tools
🪛 Ruff (0.8.2)
4-4: Line too long (105 > 88)
(E501)
web/core/hooks/store/notifications/use-workspace-notification-settings.ts (1)
9-9: Fix the error message to match the hook name.The error message references "useNotification" but the hook is named "useWorkspaceNotificationSettings".
- if (context === undefined) throw new Error("useNotification must be used within StoreProvider"); + if (context === undefined) throw new Error("useWorkspaceNotificationSettings must be used within StoreProvider");web/app/[workspaceSlug]/(projects)/notifications/settings/layout.tsx (2)
7-8: Remove extra blank line for consistency.There's an extra blank line after the imports that can be removed for better code consistency.
import { NotificationsSettingsHeader } from "./header"; -
12-13: Remove extra blank line for consistency.There's another extra blank line that can be removed.
} -apiserver/plane/app/urls/notification.py (1)
51-60: Potential URL reverse lookup conflict.Both URL patterns are assigned the same name "workspace-user-notification-preference", which could cause issues with URL reverse lookups. Consider using different names like "workspace-user-notification-preference-detail" and "workspace-user-notification-preference-list" to distinguish them.
path( "workspaces/<str:slug>/user-notification-preferences/<str:transport>/", WorkspaceUserNotificationPreferenceEndpoint.as_view(), - name="workspace-user-notification-preference", + name="workspace-user-notification-preference-detail", ), path( "workspaces/<str:slug>/user-notification-preferences/", WorkspaceUserNotificationPreferenceEndpoint.as_view(), - name="workspace-user-notification-preference", + name="workspace-user-notification-preference-list", ),web/core/components/inbox/settings/content-wrapper.tsx (2)
7-7: Remove unnecessary empty lines.There are extra empty lines that could be removed to improve code consistency.
-Also applies to: 20-20
16-19: Consider extracting Tailwind classes to named constants for readability.For improved maintainability, consider extracting complex Tailwind class combinations to named constants, especially for repeated patterns or complex responsive layouts.
+const HAMBURGER_CONTAINER_CLASSES = "block flex-shrink-0 border-b border-custom-border-200 p-4 md:hidden"; + <div className="flex h-full flex-col"> - <div className="block flex-shrink-0 border-b border-custom-border-200 p-4 md:hidden"> + <div className={HAMBURGER_CONTAINER_CLASSES}> <SidebarHamburgerToggle /> </div>packages/i18n/src/locales/pt-BR/translations.json (1)
2435-2470: New "notification_settings" section added successfully.
The new block introduces keys for Inbox Settings (page label, descriptions, notification category titles and subtitles). The translations are clear and follow the style of the rest of the file. One minor suggestion: the subtitle for "work_item_property" is identical to "task_updates_subtitle". Please confirm if the intended nuance between “atualizações de itens de trabalho” and “atualização de qualquer propriedade do item de trabalho” should be differentiated further for better user clarity.packages/i18n/src/locales/ru/translations.json (1)
2442-2474: New "notification_settings" section added with comprehensive keys.
All keys are translated consistently with the rest of the file. One detail to note is with the key"work_item_property_title". The current translation reads:
"Обновление любой свойства рабочего элемента"
Since "свойство" is a neuter noun in Russian, the adjective should agree in the genitive form—consider changing it to:
"Обновление любого свойства рабочего элемента"
This change would improve grammatical accuracy. Please verify if any other subtitles (such as for task updates) require differentiation, as they currently duplicate the text used in other parts.web/ce/components/inbox/settings/root.tsx (1)
23-27: Consider extracting duplicated grid header into a reusable component.The grid header with advanced settings, in-plane, and email columns is duplicated in both the task updates and comments sections. Consider extracting this into a reusable component or function to improve maintainability and follow the DRY principle.
+ const SettingsGridHeader = () => { + const { t } = useTranslation(); + return ( + <div className="grid gap-4 grid-cols-[50%_repeat(auto-fit,minmax(0,1fr))] text-sm text-custom-text-350 font-semibold"> + <div>{t("notification_settings.advanced_settings")}</div> + <div>{t("notification_settings.in_plane")}</div> + <div>{t("notification_settings.email")}</div> + </div> + ); + }; // Then use it in both places: <SettingsGridHeader />Also applies to: 43-47
apiserver/plane/app/views/workspace/home.py (3)
77-77: Remove unnecessary empty line.There's an extra empty line that can be removed to maintain consistent spacing.
84-91: Use a descriptive variable name instead of underscore.Using
_as a variable name indicates that the result is not used, but in this case, the result (created notification preferences) might be useful for debugging or future extensions. Consider using a descriptive variable name instead.- _ = WorkspaceUserNotificationPreference.objects.bulk_create( + created_preferences = WorkspaceUserNotificationPreference.objects.bulk_create(
5-5: Line exceeds recommended length limit.This line is 104 characters long, which exceeds the recommended PEP 8 limit of 88 characters. Consider breaking it into multiple lines for better readability.
- from plane.db.models import Workspace, WorkspaceUserNotificationPreference, NotificationTransportChoices + from plane.db.models import ( + Workspace, + WorkspaceUserNotificationPreference, + NotificationTransportChoices + )🧰 Tools
🪛 Ruff (0.8.2)
5-5: Line too long (104 > 88)
(E501)
web/ce/components/inbox/settings/update-setting-row.tsx (1)
32-34: Remove unnecessary empty className props.The divs have empty className props that don't provide any styling. These can be removed for cleaner code.
- <div className=""> + <div> <InboxSettingUpdate transport={EWorkspaceNotificationTransport.IN_APP} settings_key={settings_key} /> </div> - <div className=""> + <div> <InboxSettingUpdate transport={EWorkspaceNotificationTransport.EMAIL} settings_key={settings_key} /> </div>Also applies to: 35-37
apiserver/plane/db/models/notification.py (1)
159-167: Consider simplifying uniqueness constraintsYou have both
unique_togetherand aUniqueConstraintwith a condition that define similar constraints. While this works, it's generally cleaner to use just one approach.If the null constraint on
deleted_atis essential, you can remove theunique_togetherline and keep only the more specificUniqueConstraintwith the condition.apiserver/plane/app/views/notification/base.py (1)
426-444: Endpoint logic looks good; consider adding test coverage.
Thepatchmethod follows a straightforward partial update pattern and returns appropriate responses. It would be beneficial to add or expand unit/integration tests to ensure correct handling of edge cases (e.g., missing or invalidtransportvalues).Would you like help creating a new test or issue to track complete test coverage?
web/core/services/workspace-notification-settings.service.ts (2)
18-23: Remove redundant try/catch ().
Catching and rethrowing the same error is unnecessary. You can safely remove it for cleaner code:async fetchNotificationSettings(workspaceSlug: string): Promise<TWorkspaceUserNotification[] | undefined> { - try { const { data } = await this.get(`/api/workspaces/${workspaceSlug}/user-notification-preferences/`); return data || undefined; - } catch (error) { - throw error; - } }🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
32-40: Avoid catch-and-rethrow pattern ().
Same reasoning applies here:async updateNotificationSettings( workspaceSlug: string, transport: EWorkspaceNotificationTransport, payload: Partial<TWorkspaceUserNotification> ): Promise<TWorkspaceUserNotification | undefined> { - try { const { data } = await this.patch( `/api/workspaces/${workspaceSlug}/user-notification-preferences/${transport}/`, payload ); return data || undefined; - } catch (error) { - throw error; - } }🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
apiserver/plane/bgtasks/notification_task.py (1)
359-359: Line length warnings ().
Some lines exceed typical style guidelines (e.g., line 359, 402, 539). Consider wrapping them for better readability.Also applies to: 402-402, 539-539
🧰 Tools
🪛 Ruff (0.8.2)
359-359: Line too long (122 > 88)
(E501)
web/core/store/notifications/workspace-notification-settings.store.ts (5)
6-7: Remove or implement placeholder comments.Lines 6-7 contain comment placeholders without any actual imports. Either add the intended imports or remove these comments for cleaner code.
- // plane web services - // plane web root store
74-78: Incorrect JSDoc description.The JSDoc comment doesn't match the function's actual behavior. It describes getting "project ids by workspace slug" but the function returns notification settings for the current workspace.
- /** - * @description get project ids by workspace slug - * @param { string } workspaceSlug - * @returns { string[] | undefined } - */ + /** + * @description get notification settings for the current workspace + * @returns { Record<string, TWorkspaceUserNotification> | undefined } + */
104-107: Improve JSDoc description.The function description "handle states" is vague and doesn't clearly describe what the function does. Consider updating it to be more descriptive of the actual functionality.
- /** - * @description handle states - * @returns { TWorkspaceUserNotification[] | undefined } - */ + /** + * @description Fetch all notification settings for the current workspace + * @returns { TWorkspaceUserNotification[] | undefined } + */
133-138: Incorrect JSDoc return type.The JSDoc comment states the return type as
EWorkspaceNotificationTransport, but the function actually returns aPromise<TWorkspaceUserNotification | undefined>.- /** - * @description - updates user notification settings for a transport - * @param transport - * @param settings - * @returns { EWorkspaceNotificationTransport } - */ + /** + * @description - updates user notification settings for a transport + * @param { EWorkspaceNotificationTransport } transport + * @param { Partial<TWorkspaceUserNotification> } settings + * @returns { Promise<TWorkspaceUserNotification | undefined> } + */
32-35: Consider using a more specific type for error.The
errorproperty is typed as a genericobject, which doesn't provide type safety. Consider using a more specific type or an Error instance.- error: object = {}; + error: Error | Record<string, any> = {};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
apiserver/plane/app/serializers/__init__.py(1 hunks)apiserver/plane/app/serializers/notification.py(2 hunks)apiserver/plane/app/urls/notification.py(2 hunks)apiserver/plane/app/views/__init__.py(1 hunks)apiserver/plane/app/views/notification/base.py(3 hunks)apiserver/plane/app/views/workspace/home.py(2 hunks)apiserver/plane/bgtasks/notification_task.py(4 hunks)apiserver/plane/db/models/__init__.py(1 hunks)apiserver/plane/db/models/notification.py(1 hunks)packages/constants/src/notification.ts(2 hunks)packages/i18n/src/locales/cs/translations.json(1 hunks)packages/i18n/src/locales/de/translations.json(1 hunks)packages/i18n/src/locales/en/translations.json(1 hunks)packages/i18n/src/locales/es/translations.json(1 hunks)packages/i18n/src/locales/fr/translations.json(1 hunks)packages/i18n/src/locales/id/translations.json(1 hunks)packages/i18n/src/locales/it/translations.json(1 hunks)packages/i18n/src/locales/ja/translations.json(1 hunks)packages/i18n/src/locales/ko/translations.json(1 hunks)packages/i18n/src/locales/pl/translations.json(1 hunks)packages/i18n/src/locales/pt-BR/translations.json(1 hunks)packages/i18n/src/locales/ro/translations.json(1 hunks)packages/i18n/src/locales/ru/translations.json(1 hunks)packages/i18n/src/locales/sk/translations.json(1 hunks)packages/i18n/src/locales/ua/translations.json(1 hunks)packages/i18n/src/locales/vi-VN/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)packages/i18n/src/locales/zh-TW/translations.json(1 hunks)packages/types/src/index.d.ts(1 hunks)packages/types/src/notification.d.ts(1 hunks)web/app/[workspaceSlug]/(projects)/notifications/settings/header.tsx(1 hunks)web/app/[workspaceSlug]/(projects)/notifications/settings/layout.tsx(1 hunks)web/app/[workspaceSlug]/(projects)/notifications/settings/page.tsx(1 hunks)web/ce/components/inbox/index.ts(1 hunks)web/ce/components/inbox/settings/index.ts(1 hunks)web/ce/components/inbox/settings/root.tsx(1 hunks)web/ce/components/inbox/settings/update-setting-row.tsx(1 hunks)web/core/components/inbox/index.ts(1 hunks)web/core/components/inbox/settings/content-header.tsx(1 hunks)web/core/components/inbox/settings/content-wrapper.tsx(1 hunks)web/core/components/inbox/settings/index.ts(1 hunks)web/core/components/inbox/settings/update-setting.tsx(1 hunks)web/core/constants/fetch-keys.ts(2 hunks)web/core/hooks/store/notifications/index.ts(1 hunks)web/core/hooks/store/notifications/use-workspace-notification-settings.ts(1 hunks)web/core/services/workspace-notification-settings.service.ts(1 hunks)web/core/store/notifications/workspace-notification-settings.store.ts(1 hunks)web/core/store/root.store.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (17)
apiserver/plane/app/serializers/__init__.py (1)
apiserver/plane/app/serializers/notification.py (3)
NotificationSerializer(10-18)UserNotificationPreferenceSerializer(21-24)WorkspaceUserNotificationPreferenceSerializer(26-29)
apiserver/plane/app/urls/notification.py (1)
apiserver/plane/app/views/notification/base.py (1)
WorkspaceUserNotificationPreferenceEndpoint(370-444)
web/core/hooks/store/notifications/use-workspace-notification-settings.ts (2)
web/core/store/notifications/workspace-notification-settings.store.ts (1)
IWorkspaceNotificationSettingsStore(11-28)space/core/lib/store-provider.tsx (1)
StoreContext(10-10)
web/core/components/inbox/settings/content-wrapper.tsx (1)
web/core/components/core/sidebar/sidebar-menu-hamburger-toggle.tsx (1)
SidebarHamburgerToggle(7-20)
apiserver/plane/app/views/workspace/home.py (3)
apiserver/plane/db/models/workspace.py (1)
Workspace(117-187)apiserver/plane/db/models/notification.py (2)
WorkspaceUserNotificationPreference(126-175)NotificationTransportChoices(178-180)apiserver/plane/app/views/notification/base.py (3)
get(242-271)get(348-353)get(377-421)
apiserver/plane/app/views/__init__.py (1)
apiserver/plane/app/views/notification/base.py (1)
WorkspaceUserNotificationPreferenceEndpoint(370-444)
web/app/[workspaceSlug]/(projects)/notifications/settings/header.tsx (4)
admin/core/hooks/store/use-workspace.tsx (1)
useWorkspace(6-10)packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)web/core/store/workspace/index.ts (1)
currentWorkspace(98-103)packages/i18n/src/store/index.ts (1)
t(231-252)
web/app/[workspaceSlug]/(projects)/notifications/settings/page.tsx (6)
web/core/hooks/store/notifications/use-workspace-notification-settings.ts (1)
useWorkspaceNotificationSettings(7-12)packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)web/core/constants/fetch-keys.ts (1)
NOTIFICATION_SETTINGS(280-281)web/core/components/ui/loader/settings/email.tsx (1)
EmailSettingsLoader(3-29)web/core/components/inbox/settings/content-wrapper.tsx (1)
InboxSettingContentWrapper(13-31)web/core/components/inbox/settings/content-header.tsx (1)
InboxSettingsContentHeader(9-17)
web/core/services/workspace-notification-settings.service.ts (2)
web/core/store/router.store.ts (1)
workspaceSlug(69-71)packages/types/src/notification.d.ts (1)
TWorkspaceUserNotification(9-21)
web/core/components/inbox/settings/update-setting.tsx (3)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)web/core/hooks/store/notifications/use-workspace-notification-settings.ts (1)
useWorkspaceNotificationSettings(7-12)packages/i18n/src/store/index.ts (1)
t(231-252)
apiserver/plane/app/views/notification/base.py (3)
apiserver/plane/app/serializers/notification.py (1)
WorkspaceUserNotificationPreferenceSerializer(26-29)apiserver/plane/db/models/workspace.py (1)
Workspace(117-187)apiserver/plane/db/models/notification.py (2)
WorkspaceUserNotificationPreference(126-175)NotificationTransportChoices(178-180)
web/core/store/root.store.ts (1)
web/core/store/notifications/workspace-notification-settings.store.ts (2)
IWorkspaceNotificationSettingsStore(11-28)WorkspaceNotificationSettingsStore(30-164)
apiserver/plane/app/serializers/notification.py (1)
apiserver/plane/db/models/notification.py (7)
Notification(9-45)UserNotificationPreference(57-94)WorkspaceUserNotificationPreference(126-175)Meta(37-41)Meta(86-90)Meta(120-124)Meta(159-171)
packages/constants/src/notification.ts (2)
packages/types/src/notification.d.ts (1)
TNotificationSettings(3-7)web/core/constants/fetch-keys.ts (1)
NOTIFICATION_SETTINGS(280-281)
apiserver/plane/db/models/notification.py (2)
apiserver/plane/app/serializers/notification.py (3)
Meta(16-18)Meta(22-24)Meta(27-29)apiserver/plane/db/models/user.py (3)
Meta(114-118)Meta(198-202)Meta(228-233)
apiserver/plane/bgtasks/notification_task.py (2)
apiserver/plane/db/models/notification.py (3)
NotificationTransportChoices(178-180)WorkspaceUserNotificationPreference(126-175)Notification(9-45)apiserver/plane/db/models/issue.py (5)
IssueComment(455-493)Issue(104-253)IssueSubscriber(560-585)IssueAssignee(340-365)IssueActivity(415-452)
web/core/store/notifications/workspace-notification-settings.store.ts (7)
packages/types/src/users.d.ts (1)
IUser(15-33)packages/types/src/workspace.d.ts (1)
IWorkspace(5-20)packages/types/src/notification.d.ts (1)
TWorkspaceUserNotification(9-21)web/core/services/workspace-notification-settings.service.ts (1)
WorkspaceNotificationSettingsService(12-42)web/core/store/root.store.ts (1)
CoreRootStore(39-135)web/core/store/workspace/index.ts (1)
currentWorkspace(98-103)web/core/store/router.store.ts (1)
workspaceSlug(69-71)
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/home.py
5-5: Line too long (104 > 88)
(E501)
apiserver/plane/app/serializers/notification.py
4-4: Line too long (105 > 88)
(E501)
apiserver/plane/bgtasks/notification_task.py
359-359: Line too long (122 > 88)
(E501)
402-402: Line too long (98 > 88)
(E501)
539-539: Line too long (90 > 88)
(E501)
🪛 Biome (1.9.4)
web/core/services/workspace-notification-settings.service.ts
[error] 22-22: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
[error] 39-39: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (56)
web/core/components/inbox/index.ts (1)
7-7: LGTM! Export statement for settings module is properly added.The export statement follows the existing pattern in the file and makes the notification settings components accessible through the inbox component's index.
packages/types/src/index.d.ts (1)
44-44: LGTM! Export statement for notification types is properly added.The export follows the existing pattern in the file and makes the new notification types accessible throughout the application, which is essential for TypeScript type checking of the new notification settings functionality.
web/ce/components/inbox/index.ts (1)
1-1: LGTM! Export statement for settings module in CE version is properly added.This change ensures that the Community Edition has access to the same notification settings components as the core version, maintaining feature parity.
web/ce/components/inbox/settings/index.ts (1)
1-2: Export structure looks goodThis file establishes a clean barrel export pattern, making it easy to import all inbox setting components from a single entry point.
apiserver/plane/app/views/__init__.py (1)
206-206: Properly imported the new workspace notification preference endpointThe new
WorkspaceUserNotificationPreferenceEndpointis correctly imported alongside the other notification-related endpoints, maintaining the organization of the file.web/core/components/inbox/settings/index.ts (1)
1-3: Export structure looks goodThis file establishes a clean barrel export pattern for the inbox settings components, providing a consolidated entry point for importing these components elsewhere in the application.
apiserver/plane/app/serializers/notification.py (1)
26-29: New serializer for workspace notification preferences looks goodThe
WorkspaceUserNotificationPreferenceSerializerclass is correctly implemented, inheriting fromBaseSerializerand configuring the Meta class appropriately. It will handle the serialization and deserialization of workspace-level notification preferences for different transport methods.web/core/components/inbox/settings/content-header.tsx (1)
1-17: Clean and well-structured component!The InboxSettingsContentHeader component is implemented using proper TypeScript types and follows React best practices with functional components. The styling with Tailwind CSS classes provides a clear visual hierarchy for the header content.
web/core/hooks/store/notifications/use-workspace-notification-settings.ts (1)
1-12: Well-implemented custom hook!The hook correctly follows React patterns by using the context API and providing proper error handling when used outside a StoreProvider. The hook provides a clean interface to access workspace notification settings from the MobX store.
web/app/[workspaceSlug]/(projects)/notifications/settings/layout.tsx (1)
1-24: Good layout component structure!The component implements a clean layout structure for the notifications settings page with proper TypeScript typing for props.
packages/i18n/src/locales/es/translations.json (1)
2444-2476: Comprehensive translations for notification settings!The added Spanish translations for notification settings are thorough and cover all the necessary UI elements including section headers, descriptions, and specific notification types.
packages/i18n/src/locales/ko/translations.json (1)
2442-2476: New localization keys added for notification settings feature.These translations support the new workspace notification preferences feature, providing Korean language support for inbox settings, notification types, and status messages. The translations are well-structured with both titles and descriptive subtitles for each setting.
apiserver/plane/app/urls/notification.py (1)
9-9: Import added for new workspace notification preferences endpoint.The import is correctly added to support the new URL routes below.
packages/i18n/src/locales/fr/translations.json (1)
2442-2474: New French localization keys added for notification settings feature.These translations provide proper French language support for the new notification preferences feature. The translations are well-structured with titles and descriptive subtitles that clearly explain each notification setting's purpose.
packages/i18n/src/locales/de/translations.json (1)
2398-2430: LGTM - New notification settings translations are well structured and comprehensive.The added German translations for notification settings are well-organized and cover all necessary UI elements for the new inbox settings feature. The translations maintain consistency with the existing localization patterns in the file.
packages/types/src/notification.d.ts (2)
1-7: TypeScript types properly defined for notification settings.The
TNotificationSettingstype is well-structured with appropriate properties for internationalization and type safety. This will help ensure consistent implementation of notification settings across the application.
9-21: User notification preferences type is comprehensive.The
TWorkspaceUserNotificationtype properly captures all the necessary properties for workspace-specific notification preferences, with appropriate boolean flags for different notification types. This provides good type safety for the notification settings feature.packages/i18n/src/locales/zh-TW/translations.json (1)
2444-2476: LGTM - Traditional Chinese translations consistently implemented.The Traditional Chinese translations for notification settings match the structure of the German translations and provide good coverage of all notification setting options. The translations are well-formatted and maintain consistency with existing localization patterns.
packages/i18n/src/locales/zh-CN/translations.json (4)
2432-2440: LGTM: Clean implementation of notification settings localizationThe new "notification_settings" section is well-structured with clear key naming conventions that align with the UI components. The header and description translations look good.
2441-2451: LGTM: Notification channel options and task updates translationsGood implementation of notification channel options (Plane, email, Slack) and task updates section with appropriate subtitle explanations.
2452-2462: LGTM: Property and comment notification translationsThe translations for different notification types (property changes, status, priority, etc.) are well-organized with clear titles and descriptive subtitles.
2463-2464: LGTM: Status messages for settings updatesThe success and error message translations for settings updates are clear and concise.
packages/i18n/src/locales/sk/translations.json (1)
2441-2472: LGTM: Slovak translations for notification settingsThe Slovak translations for the notification settings section are consistent with the Chinese version, maintaining the same structure and covering all the necessary UI elements.
web/app/[workspaceSlug]/(projects)/notifications/settings/header.tsx (3)
1-13: LGTM: Clean import structureThe imports are well-organized and properly grouped by category with clear comments separating them. Using "use client" directive is appropriate for this client-side component.
14-17: LGTM: Component setup with proper hooksGood use of MobX's observer pattern for reactivity, and appropriate hooks for workspace data and translations.
18-44: LGTM: Clean breadcrumb navigation implementationThe header with breadcrumb navigation is well-structured, providing a clear path from notifications to settings. The use of icons enhances the UI, and the translation keys are consistent with the added translations.
apiserver/plane/app/serializers/__init__.py (1)
118-122: LGTM: Improved import formatting with new serializerThe change to a multi-line import format improves readability, and the addition of the
WorkspaceUserNotificationPreferenceSerializeraligns with the new notification settings feature.packages/i18n/src/locales/ro/translations.json (1)
2436-2468: Notification Settings (Romanian): Looks well-structured and consistent!The newly added
"notification_settings"section neatly introduces all the keys required for the inbox settings update. The labels, subtitles, and placeholders (e.g.{workspace}) are clear and align with the naming conventions used throughout the file. Please verify that these translations completely match the backend API and UI design expectations.packages/i18n/src/locales/it/translations.json (1)
2441-2473: Notification Settings (Italian): Comprehensive and consistent addition!The new
"notification_settings"block provides all the necessary keys and their corresponding subtitles for managing notification preferences (including inbox settings, advanced settings, task updates, comments, and various notifications). The use of placeholders like{workspace}and the overall terminology is consistent with other sections in the file. Please double-check that these strings are in full agreement with the UI and backend requirements.packages/i18n/src/locales/ua/translations.json (1)
2400-2432: Complete translation coverage for notification settings - good job!The new notification settings section provides comprehensive Ukrainian translations for all UI elements in the inbox settings feature. This includes headers, descriptions, and specific notification types, ensuring a consistent user experience for Ukrainian-speaking users.
web/core/components/inbox/settings/content-wrapper.tsx (1)
13-31: Well-structured responsive component with good mobile adaptation.The component correctly handles different screen sizes, showing the hamburger toggle only on mobile screens and adjusting padding responsively. The use of the
cnutility for class merging is a good practice.packages/i18n/src/locales/en/translations.json (1)
2275-2307: Complete English translations for the notification settings feature.All required translation keys for the inbox notification settings feature are properly implemented with clear and concise English text. The translations cover all UI elements including headers, descriptions, and specific notification types.
packages/i18n/src/locales/ja/translations.json (1)
2442-2474: Good implementation of notification settings localization in JapaneseThe newly added "notification_settings" section includes comprehensive translations for all required UI elements such as settings titles, descriptions, and notification categories. The structure is consistent with the rest of the file and provides complete coverage for the new notification preferences feature.
packages/i18n/src/locales/cs/translations.json (1)
2440-2472: Well-implemented Czech translation for notification settingsThe added "notification_settings" section maintains consistency with other language files while providing complete coverage of all notification preference UI elements. The translations are properly structured and include all necessary strings for the new inbox settings feature.
apiserver/plane/db/models/__init__.py (1)
47-53: Good refactoring of imports with additional notification modelsThe change properly imports the new
NotificationTransportChoicesandWorkspaceUserNotificationPreferencemodels needed for the workspace-specific notification preferences feature. The multi-line formatting improves readability, making the code more maintainable.packages/i18n/src/locales/vi-VN/translations.json (1)
2398-2429: The new Vietnamese translation keys for notification settings look good!The translation strings are well-organized, follow the same structure as other sections, and provide complete coverage for the new notification settings feature.
web/core/constants/fetch-keys.ts (2)
224-225: Good addition of the Slack connection status fetch key.The implementation follows the established pattern in the file, correctly using the workspace slug to create a unique fetch key identifier.
280-281: Properly implemented notification settings fetch key.This follows the standard pattern for fetch keys in this file, ensuring consistent formatting of the workspace slug.
packages/i18n/src/locales/pl/translations.json (1)
2401-2432: Polish translations for notification settings look well-implemented.All required translation keys are present and properly structured. The translations align with the feature's requirements and follow consistent formatting.
web/ce/components/inbox/settings/root.tsx (1)
1-56: LGTM! The component structure is clean and well-organized.The component correctly implements a notification settings UI with clear sections for task updates and comments. Good use of internationalization, grid layouts, and proper component composition.
packages/i18n/src/locales/id/translations.json (1)
2436-2468: LGTM! The translations are well-structured and comprehensive.The added translations for notification settings are well-organized and provide complete coverage for the new notification settings UI. The translations are consistent with the existing patterns in the file and include all the necessary keys used in the new components.
apiserver/plane/app/views/workspace/home.py (1)
62-92: LGTM! The notification preference functionality is well-implemented.The added code correctly implements the functionality to retrieve and create user notification preferences for different transport types. It follows the same pattern as the existing preference management code and ensures that users have all the necessary notification preference records.
web/ce/components/inbox/settings/update-setting-row.tsx (1)
1-40: LGTM! The component is well-structured and follows React best practices.This component effectively implements a row for notification settings with proper type definitions, internationalization support, and MobX integration via the observer HOC. The layout is clean and organized with good separation of concerns.
web/app/[workspaceSlug]/(projects)/notifications/settings/page.tsx (1)
1-46: Nice implementation of the notification settings page!The component is well-structured, following React and Next.js best practices. Good use of:
- MobX's observer pattern to react to observable state changes
- SWR for data fetching with proper loading state handling
- Translation hooks for internationalization
- Clean component composition with appropriate wrappers and headers
apiserver/plane/db/models/notification.py (1)
150-155: Consider default values consistency with existing modelsThe boolean fields in this model default to
Falsewhile the existingUserNotificationPreferencemodel defaults most fields toTrue. Verify this is intentional as it may lead to inconsistent default behaviors.web/core/components/inbox/settings/update-setting.tsx (2)
24-41: Good error handling with user feedbackThe component properly handles async operations with try/catch blocks and provides appropriate user feedback through toast notifications. Well implemented!
10-13: Well-typed props interfaceThe props are well-typed using TypeScript, which enhances code readability and provides better development experience with autocompletion and type checking.
web/core/store/root.store.ts (2)
25-25: Clean import addition for workspace notification settingsThe import statement follows the established pattern and correctly imports both the interface and implementation class.
64-64: Consistent store initialization patternThe new
workspaceNotificationSettingsproperty is properly declared and consistently initialized in both the constructor andresetOnSignOutmethod, following the established pattern for store management.Also applies to: 94-94, 128-128
apiserver/plane/app/views/notification/base.py (2)
12-12: Good import usage.
No issues found with adding the serializer import here.
21-23: Clean import definitions.
ImportingWorkspace,WorkspaceUserNotificationPreference, andNotificationTransportChoiceslooks correct and consistent with the usage below.packages/constants/src/notification.ts (1)
140-205: Constants and enums look adequately structured.
All enums and arrays for notification settings are logically organized and easy to read.apiserver/plane/bgtasks/notification_task.py (1)
210-271: Clean mention processing function.
Theprocess_mentionsfunction is well-structured and clearly separates new mentions, removed mentions, and comment-based mentions.web/core/store/notifications/workspace-notification-settings.store.ts (3)
51-68: The autorun implementation is well-structured.Good use of autorun to reactively update the store when the current user or workspace changes. The condition checks both for initialization and updates, ensuring the store stays in sync with the root store.
116-123: Good use of runInAction with lodash set.The combination of
runInActionwith lodash'ssetfunction is a good approach for updating nested state structures in MobX while ensuring reactivity. This pattern correctly updates the settings without causing unnecessary rerenders.
11-28: Well-defined interface with clear documentation.The interface is well-structured with clear sections for observables, computed functions, and actions. This makes the code more maintainable and provides good documentation for consumers of the store.
| # Notification preference get or create | ||
| workspace = Workspace.objects.get(slug=slug) | ||
| get_notification_preferences = ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant workspace fetch.
The workspace object is already fetched on line 21, so there's no need to fetch it again here. This redundant fetch can be removed to improve performance.
# Notification preference get or create
- workspace = Workspace.objects.get(slug=slug)
get_notification_preferences = (
WorkspaceUserNotificationPreference.objects.filter(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Notification preference get or create | |
| workspace = Workspace.objects.get(slug=slug) | |
| get_notification_preferences = ( | |
| # Notification preference get or create | |
| get_notification_preferences = ( | |
| WorkspaceUserNotificationPreference.objects.filter( |
| class WorkspaceUserNotificationPreference(BaseModel): | ||
| # user it is related to | ||
| user = models.ForeignKey( | ||
| settings.AUTH_USER_MODEL, | ||
| on_delete=models.CASCADE, | ||
| related_name="user_workspace_notification_preferences", | ||
| ) | ||
| # workspace if it is applicable | ||
| workspace = models.ForeignKey( | ||
| "db.Workspace", | ||
| on_delete=models.CASCADE, | ||
| related_name="workspace_user_notification_preferences", | ||
| ) | ||
| # project | ||
| project = models.ForeignKey( | ||
| "db.Project", | ||
| on_delete=models.CASCADE, | ||
| related_name="project_user_notification_preferences", | ||
| null=True, | ||
| ) | ||
|
|
||
| transport = models.CharField(max_length=50, default="EMAIL") | ||
|
|
||
| # task updates | ||
| property_change = models.BooleanField(default=False) | ||
| state_change = models.BooleanField(default=False) | ||
| priority = models.BooleanField(default=False) | ||
| assignee = models.BooleanField(default=False) | ||
| start_due_date = models.BooleanField(default=False) | ||
| # comments fields | ||
| comment = models.BooleanField(default=False) | ||
| mention = models.BooleanField(default=False) | ||
| comment_reactions = models.BooleanField(default=False) | ||
| class Meta: | ||
| unique_together = ["workspace", "user", "transport", "deleted_at"] | ||
| constraints = [ | ||
| models.UniqueConstraint( | ||
| fields=["workspace", "user", "transport"], | ||
| condition=models.Q(deleted_at__isnull=True), | ||
| name="notification_preferences_unique_workspace_user_transport_when_deleted_at_null", | ||
| ) | ||
| ] | ||
| verbose_name = "Workspace User Notification Preference" | ||
| verbose_name_plural = "Workspace User Notification Preferences" | ||
| db_table = "workspace_user_notification_preferences" | ||
| ordering = ("-created_at",) | ||
|
|
||
| def __str__(self): | ||
| """Return the user""" | ||
| return f"<{self.user}>" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Model definition looks good but should explicitly use choice field constraints
The WorkspaceUserNotificationPreference model is well-structured with appropriate relationships and fields. However, the transport field should explicitly use the NotificationTransportChoices you've defined.
- transport = models.CharField(max_length=50, default="EMAIL")
+ transport = models.CharField(
+ max_length=50,
+ choices=NotificationTransportChoices.choices,
+ default=NotificationTransportChoices.EMAIL
+ )This provides better data validation and makes the relationship between the field and the choices class explicit.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class WorkspaceUserNotificationPreference(BaseModel): | |
| # user it is related to | |
| user = models.ForeignKey( | |
| settings.AUTH_USER_MODEL, | |
| on_delete=models.CASCADE, | |
| related_name="user_workspace_notification_preferences", | |
| ) | |
| # workspace if it is applicable | |
| workspace = models.ForeignKey( | |
| "db.Workspace", | |
| on_delete=models.CASCADE, | |
| related_name="workspace_user_notification_preferences", | |
| ) | |
| # project | |
| project = models.ForeignKey( | |
| "db.Project", | |
| on_delete=models.CASCADE, | |
| related_name="project_user_notification_preferences", | |
| null=True, | |
| ) | |
| transport = models.CharField(max_length=50, default="EMAIL") | |
| # task updates | |
| property_change = models.BooleanField(default=False) | |
| state_change = models.BooleanField(default=False) | |
| priority = models.BooleanField(default=False) | |
| assignee = models.BooleanField(default=False) | |
| start_due_date = models.BooleanField(default=False) | |
| # comments fields | |
| comment = models.BooleanField(default=False) | |
| mention = models.BooleanField(default=False) | |
| comment_reactions = models.BooleanField(default=False) | |
| class Meta: | |
| unique_together = ["workspace", "user", "transport", "deleted_at"] | |
| constraints = [ | |
| models.UniqueConstraint( | |
| fields=["workspace", "user", "transport"], | |
| condition=models.Q(deleted_at__isnull=True), | |
| name="notification_preferences_unique_workspace_user_transport_when_deleted_at_null", | |
| ) | |
| ] | |
| verbose_name = "Workspace User Notification Preference" | |
| verbose_name_plural = "Workspace User Notification Preferences" | |
| db_table = "workspace_user_notification_preferences" | |
| ordering = ("-created_at",) | |
| def __str__(self): | |
| """Return the user""" | |
| return f"<{self.user}>" | |
| class WorkspaceUserNotificationPreference(BaseModel): | |
| # user it is related to | |
| user = models.ForeignKey( | |
| settings.AUTH_USER_MODEL, | |
| on_delete=models.CASCADE, | |
| related_name="user_workspace_notification_preferences", | |
| ) | |
| # workspace if it is applicable | |
| workspace = models.ForeignKey( | |
| "db.Workspace", | |
| on_delete=models.CASCADE, | |
| related_name="workspace_user_notification_preferences", | |
| ) | |
| # project | |
| project = models.ForeignKey( | |
| "db.Project", | |
| on_delete=models.CASCADE, | |
| related_name="project_user_notification_preferences", | |
| null=True, | |
| ) | |
| transport = models.CharField( | |
| max_length=50, | |
| choices=NotificationTransportChoices.choices, | |
| default=NotificationTransportChoices.EMAIL | |
| ) | |
| # task updates | |
| property_change = models.BooleanField(default=False) | |
| state_change = models.BooleanField(default=False) | |
| priority = models.BooleanField(default=False) | |
| assignee = models.BooleanField(default=False) | |
| start_due_date = models.BooleanField(default=False) | |
| # comments fields | |
| comment = models.BooleanField(default=False) | |
| mention = models.BooleanField(default=False) | |
| comment_reactions = models.BooleanField(default=False) | |
| class Meta: | |
| unique_together = ["workspace", "user", "transport", "deleted_at"] | |
| constraints = [ | |
| models.UniqueConstraint( | |
| fields=["workspace", "user", "transport"], | |
| condition=models.Q(deleted_at__isnull=True), | |
| name="notification_preferences_unique_workspace_user_transport_when_deleted_at_null", | |
| ) | |
| ] | |
| verbose_name = "Workspace User Notification Preference" | |
| verbose_name_plural = "Workspace User Notification Preferences" | |
| db_table = "workspace_user_notification_preferences" | |
| ordering = ("-created_at",) | |
| def __str__(self): | |
| """Return the user""" | |
| return f"<{self.user}>" |
| actor = User.objects.get(pk=actor_id) | ||
| for mention_id in comment_mentions: | ||
| if mention_id != actor_id: | ||
| in_app_preference = WorkspaceUserNotificationPreference.objects.filter( | ||
| user_id=mention_id, | ||
| workspace_id=issue_workspace_id, | ||
| transport=NotificationTransportChoices.IN_APP[0], | ||
| ).first() | ||
|
|
||
| if in_app_preference.mention: |
There was a problem hiding this comment.
Prevent potential NoneType error when checking in-app preferences.
If in_app_preference is None, calling in_app_preference.mention will raise an error. Consider verifying it first:
- if in_app_preference.mention:
+ if in_app_preference and in_app_preference.mention:
...
- if not in_app_preference.mention:
+ if not in_app_preference or not in_app_preference.mention:
continueAlso applies to: 371-378
| if email_preference.mention: | ||
| for issue_activity in issue_activities_created: |
There was a problem hiding this comment.
Avoid NoneType usage in email preference checks.
Similar to the in-app logic, do a null check to prevent AttributeError if email_preference is not found:
- if email_preference.mention:
+ if email_preference and email_preference.mention:
...
- if not email_preference.mention:
+ if not email_preference or not email_preference.mention:
continueAlso applies to: 503-504
| getNotificationSettingsForTransport = computedFn((transport: EWorkspaceNotificationTransport) => { | ||
| const workspaceSlug = this.store.workspaceRoot?.currentWorkspace?.slug; | ||
| if (!workspaceSlug || !transport) { | ||
| return; | ||
| } | ||
| const notificationSettingsForTransport = this.settings[workspaceSlug][transport] || undefined; | ||
| return notificationSettingsForTransport; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent potential undefined access error.
The code attempts to access this.settings[workspaceSlug][transport] without checking if this.settings[workspaceSlug] exists. This could lead to a runtime error if the settings for the workspace haven't been initialized yet.
getNotificationSettingsForTransport = computedFn((transport: EWorkspaceNotificationTransport) => {
const workspaceSlug = this.store.workspaceRoot?.currentWorkspace?.slug;
if (!workspaceSlug || !transport) {
return;
}
- const notificationSettingsForTransport = this.settings[workspaceSlug][transport] || undefined;
+ const workspaceSettings = this.settings[workspaceSlug];
+ const notificationSettingsForTransport = workspaceSettings ? workspaceSettings[transport] : undefined;
return notificationSettingsForTransport;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getNotificationSettingsForTransport = computedFn((transport: EWorkspaceNotificationTransport) => { | |
| const workspaceSlug = this.store.workspaceRoot?.currentWorkspace?.slug; | |
| if (!workspaceSlug || !transport) { | |
| return; | |
| } | |
| const notificationSettingsForTransport = this.settings[workspaceSlug][transport] || undefined; | |
| return notificationSettingsForTransport; | |
| }); | |
| getNotificationSettingsForTransport = computedFn((transport: EWorkspaceNotificationTransport) => { | |
| const workspaceSlug = this.store.workspaceRoot?.currentWorkspace?.slug; | |
| if (!workspaceSlug || !transport) { | |
| return; | |
| } | |
| const workspaceSettings = this.settings[workspaceSlug]; | |
| const notificationSettingsForTransport = workspaceSettings ? workspaceSettings[transport] : undefined; | |
| return notificationSettingsForTransport; | |
| }); |
Description
Inbox settings page UI and APIs
Type of Change
Screenshots and Media (if applicable)
Summary by CodeRabbit