chore: ln support for inbox constants #6432
Conversation
WalkthroughThis pull request introduces comprehensive changes to the inbox issue management system across multiple packages. The modifications include defining new enumerations, types, and constants for inbox issues, enhancing internationalization support, and reorganizing type imports. The changes span constants, translations, components, and services, focusing on improving the structure, localization, and type safety of inbox-related functionality. Changes
Sequence DiagramsequenceDiagram
participant UI as Inbox UI
participant Constants as Inbox Constants
participant I18n as Internationalization
participant Store as Inbox Store
UI->>Constants: Retrieve inbox issue statuses
Constants-->>UI: Return status definitions
UI->>I18n: Request translations
I18n-->>UI: Provide localized labels
UI->>Store: Fetch and manage inbox issues
Store-->>UI: Return inbox issue data
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
web/core/components/inbox/inbox-filter/applied-filters/status.tsx (1)
35-35: Enhance null safety for optional chaining.While the code works, the optional chaining on
optionDetail?.titlecould be more robust.Consider adding a fallback:
- <div className="text-xs truncate">{t(optionDetail?.title)}</div> + <div className="text-xs truncate"> + {optionDetail?.title ? t(optionDetail.title) : t('common.unknown_status')} + </div>web/core/components/inbox/inbox-filter/filters/status.tsx (1)
58-58: Maintain consistency with applied filters component.The translation implementation here should match the approach used in the applied filters component for consistency.
Consider applying the same null safety pattern:
- title={t(status.title)} + title={status.title ? t(status.title) : t('common.unknown_status')}packages/constants/src/inbox.ts (2)
1-53: Consider using an enum for status keys.The status keys are currently hardcoded strings. Consider using an enum to ensure type safety and prevent typos.
+export enum EInboxStatusKey { + PENDING = "pending", + DECLINED = "declined", + SNOOZED = "snoozed", + ACCEPTED = "accepted", + DUPLICATE = "duplicate" +} export const INBOX_STATUS: { - key: string; + key: EInboxStatusKey; title: string; description: () => string; textColor: (snoozeDatePassed: boolean) => string; bgColor: (snoozeDatePassed: boolean) => string; }[] = [ { - key: "pending", + key: EInboxStatusKey.PENDING, // ... rest of the code }, // ... rest of the statuses ];
57-70: Consider using an enum for order by keys.Similar to status keys, consider using an enum for order by options to ensure type safety.
+export enum EInboxIssueOrderByKey { + CREATED_AT = "issue__created_at", + UPDATED_AT = "issue__updated_at", + SEQUENCE_ID = "issue__sequence_id" +} export const INBOX_ISSUE_ORDER_BY_OPTIONS = [ { - key: "issue__created_at", + key: EInboxIssueOrderByKey.CREATED_AT, label: "inbox_issue.order_by.created_at", }, // ... rest of the options ];web/helpers/inbox.helper.ts (1)
78-84: Consider using type-safe icon mappings.The icon mappings could benefit from stronger type safety.
+type TInboxStatusIconMap = { + [K in keyof typeof INBOX_STATUS_KEYS]: LucideIcon; +}; -const INBOX_STATUS_ICONS = { +const INBOX_STATUS_ICONS: TInboxStatusIconMap = { pending: AlertTriangle, declined: XCircle, snoozed: Clock, accepted: CheckCircle2, duplicate: Copy, };packages/i18n/src/locales/en/translations.json (1)
352-357: Consider enhancing sort direction descriptions.While the current translations are correct, consider adding more context to help users understand the sorting direction (e.g., "A to Z" for ascending, "Z to A" for descending).
"common": { "sort": { - "asc": "Ascending", - "desc": "Descending" + "asc": "Ascending (A to Z)", + "desc": "Descending (Z to A)" } }packages/i18n/src/locales/es/translations.json (1)
352-357: Consider using more natural Spanish terms for sorting.While "Ascendente/Descendente" are correct, consider using more natural Spanish UI terms.
"common": { "sort": { - "asc": "Ascendente", - "desc": "Descendente" + "asc": "De menor a mayor", + "desc": "De mayor a menor" } }packages/i18n/src/locales/fr/translations.json (1)
352-357: Consider using more natural French terms for sorting.While "Croissant/Décroissant" are technically correct, consider using more natural French UI terms.
"common": { "sort": { - "asc": "Croissant", - "desc": "Décroissant" + "asc": "Du plus petit au plus grand", + "desc": "Du plus grand au plus petit" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/constants/src/inbox.ts(1 hunks)packages/constants/src/index.ts(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/ja/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)packages/types/src/inbox.d.ts(1 hunks)web/core/components/inbox/inbox-filter/applied-filters/status.tsx(2 hunks)web/core/components/inbox/inbox-filter/filters/status.tsx(3 hunks)web/core/components/inbox/inbox-filter/sorting/order-by.tsx(5 hunks)web/core/components/inbox/inbox-issue-status.tsx(3 hunks)web/core/components/stickies/layout/stickies-list.tsx(1 hunks)web/core/constants/empty-state.tsx(1 hunks)web/core/constants/inbox.tsx(0 hunks)web/helpers/inbox.helper.ts(2 hunks)
💤 Files with no reviewable changes (1)
- web/core/constants/inbox.tsx
✅ Files skipped from review due to trivial changes (3)
- web/core/constants/empty-state.tsx
- packages/types/src/inbox.d.ts
- web/core/components/stickies/layout/stickies-list.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
packages/constants/src/index.ts (1)
17-17: LGTM! Export addition follows established pattern.The new export for the inbox module is consistent with the existing export structure.
web/core/components/inbox/inbox-issue-status.tsx (1)
5-5: Verify translation key structure and fallbacks.The translation implementation looks good, but consider adding fallback handling for missing translations to ensure graceful degradation.
Add a fallback mechanism:
- const { t } = useTranslation(); + const { t } = useTranslation({ + fallbackLng: 'en', + returnNull: false, + });Also applies to: 8-8, 20-20
web/core/components/inbox/inbox-filter/sorting/order-by.tsx (1)
6-8: Great improvements to the component!The changes enhance the component by:
- Adding proper type safety through type casting
- Implementing internationalization support
- Using centralized constants for better maintainability
Also applies to: 19-19, 37-37, 52-52, 54-54, 63-63, 65-65
packages/i18n/src/locales/zh-CN/translations.json (1)
319-356: Well-structured translations for inbox functionality!The translations are comprehensive and properly organized, covering all necessary strings for inbox statuses, sources, and sorting options.
packages/i18n/src/locales/ja/translations.json (2)
320-351: LGTM: Inbox issue translations are well-structured and accurate.The Japanese translations for inbox issues are semantically correct and maintain proper hierarchy. The translations accurately convey the meaning of each status, source, and sorting option.
352-357: LGTM: Common sorting translations are consistent.The Japanese translations for common sorting options (昇順/降順) are accurate and commonly used terms in Japanese UIs.
packages/i18n/src/locales/en/translations.json (1)
320-351: LGTM: Base English translations are clear and well-structured.The English translations serve as a good reference for other languages, with clear and concise terms for each status, source, and sorting option.
packages/i18n/src/locales/es/translations.json (1)
320-351: LGTM: Spanish translations are accurate and well-structured.The Spanish translations for inbox issues maintain proper semantic meaning and use commonly accepted terms in Spanish UIs.
packages/i18n/src/locales/fr/translations.json (1)
320-351: LGTM: French translations are accurate and well-structured.The French translations for inbox issues maintain proper semantic meaning and use commonly accepted terms in French UIs.
| // derived values | ||
| const inboxIssueStatusDetail = INBOX_STATUS.find((s) => s.status === inboxIssue.status); | ||
|
|
||
| const isSnoozedDatePassed = inboxIssue.status === 0 && new Date(inboxIssue.snoozed_till ?? "") < new Date(); | ||
| if (!inboxIssueStatusDetail || isSnoozedDatePassed) return <></>; | ||
|
|
||
| const description = inboxIssueStatusDetail.description(new Date(inboxIssue.snoozed_till ?? "")); | ||
| const description = t(inboxIssueStatusDetail.description(new Date(inboxIssue.snoozed_till ?? ""))); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider separating dynamic content from translations.
The description translation includes a dynamic date value. This might cause issues with different language sentence structures.
Consider using translation interpolation:
- const description = t(inboxIssueStatusDetail.description(new Date(inboxIssue.snoozed_till ?? "")));
+ const formattedDate = new Date(inboxIssue.snoozed_till ?? "").toLocaleDateString();
+ const description = t(inboxIssueStatusDetail.descriptionKey, { date: formattedDate });
- {inboxIssue?.status === 0 && inboxIssue?.snoozed_till ? description : t(inboxIssueStatusDetail.title)}
+ {inboxIssue?.status === 0 && inboxIssue?.snoozed_till
+ ? t(inboxIssueStatusDetail.descriptionKey, { date: formattedDate })
+ : t(inboxIssueStatusDetail.title)
+ }Also applies to: 40-40
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/core/components/inbox/inbox-issue-status.tsx (1)
43-43: Consider extracting the conditional rendering logic.The ternary operation could be made more readable by extracting it into a separate function or variable.
+ const getStatusText = (status: number, snoozedTill: string | null, description: string, title: string) => { + return status === 0 && snoozedTill ? description : title; + }; - {inboxIssue?.status === 0 && inboxIssue?.snoozed_till ? description : t(inboxIssueStatusDetail.title)} + {getStatusText(inboxIssue?.status, inboxIssue?.snoozed_till, description, t(inboxIssueStatusDetail.title))}web/helpers/inbox.helper.ts (1)
86-92: Consider using an enum map for type safety.The status keys mapping could be more type-safe by using a mapped type from the enum.
- const INBOX_STATUS_KEYS = { - pending: EInboxIssueStatus.PENDING, - declined: EInboxIssueStatus.DECLINED, - snoozed: EInboxIssueStatus.SNOOZED, - accepted: EInboxIssueStatus.ACCEPTED, - duplicate: EInboxIssueStatus.DUPLICATE, - }; + const INBOX_STATUS_KEYS: Record<keyof typeof INBOX_STATUS_ICONS, EInboxIssueStatus> = { + pending: EInboxIssueStatus.PENDING, + declined: EInboxIssueStatus.DECLINED, + snoozed: EInboxIssueStatus.SNOOZED, + accepted: EInboxIssueStatus.ACCEPTED, + duplicate: EInboxIssueStatus.DUPLICATE, + };packages/i18n/src/locales/en/translations.json (1)
320-357: Well implemented pluralization for English translations.The English translations correctly handle pluralization for the "days to go" message using the ICU message format.
Consider adding a zero case for better user experience:
- "description": "{days, plural, one{# day} other{# days}} to go" + "description": "{days, plural, =0{Today} one{# day} other{# days}} to go"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
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/ja/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)web/core/components/inbox/inbox-issue-status.tsx(3 hunks)web/helpers/inbox.helper.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (9)
web/core/components/inbox/inbox-issue-status.tsx (2)
5-5: LGTM! Good refactoring of imports.The reorganization of imports improves code organization by:
- Moving INBOX_STATUS to a dedicated helper file
- Adding proper i18n support
- Including a specialized date-time helper
Also applies to: 7-7, 9-9
28-30: Well implemented translation with interpolation.The translation implementation correctly handles dynamic content by using interpolation for the days value, which is better than string concatenation.
web/helpers/inbox.helper.ts (2)
78-84: LGTM! Well-organized icon mappings.The icon mappings are well-structured and provide a centralized way to manage icons for different inbox statuses.
94-106: LGTM! Well-typed status configuration.The INBOX_STATUS type definition and mapping provide good type safety and maintainability.
packages/i18n/src/locales/zh-CN/translations.json (1)
319-356: LGTM! Well-structured Chinese translations.The translations are well-organized and maintain consistency with other language files.
packages/i18n/src/locales/ja/translations.json (1)
319-357: LGTM! Well-structured Japanese translations.The translations are accurate and maintain consistency with other language files.
packages/i18n/src/locales/es/translations.json (1)
319-357: Well-structured translations with proper localization practices!The Spanish translations for inbox functionality are well-implemented with:
- Proper handling of pluralization in the "snoozed" status description
- Consistent terminology throughout the translations
- Natural and grammatically correct Spanish language usage
packages/i18n/src/locales/fr/translations.json (2)
319-357: Well-implemented French translations with proper localization practices!The French translations for inbox functionality are well-implemented with:
- Proper handling of pluralization in the "snoozed" status description
- Consistent terminology throughout the translations
- Natural and grammatically correct French language usage
319-357: Excellent implementation of internationalization across both files!The implementation demonstrates high-quality localization practices across both Spanish and French translations:
- Consistent structure and naming conventions between language files
- Proper handling of pluralization rules for each language
- Natural language usage specific to each locale
- Complete coverage of all required translations
There was a problem hiding this comment.
Color is not getting applied to this icon
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
packages/types/src/inbox.d.ts (2)
Line range hint
13-13: Import missing forTInboxIssueStatusThe type
TInboxIssueStatusis referenced inTInboxIssueFilterbut is neither defined in this file nor imported. Please importTInboxIssueStatusfrom its new location to ensure type correctness.Apply this diff to fix the issue:
+import { TInboxIssueStatus } from "@plane/constants/src/inbox";
Line range hint
41-41: Import missing forTInboxIssueThe type
TInboxIssueis used inTInboxIssueWithPaginationbut is not imported or defined in this file. Ensure thatTInboxIssueis properly imported from its new location.Apply this diff to fix the issue:
+import { TInboxIssue } from "@plane/constants/src/inbox";
🧹 Nitpick comments (3)
packages/constants/src/inbox.ts (2)
41-41: Simplify thedescriptionfunctions by removing unnecessary template literalsThe
descriptionfunctions are returning string literals without any variable interpolation. Replacing the template literals with regular string literals improves readability.Apply this diff to simplify the code:
- description: () => `inbox_issue.status.pending.description`, + description: () => 'inbox_issue.status.pending.description',Repeat this change for the
descriptionfunctions in all statuses.Also applies to: 51-51, 61-61, 71-71, 81-81
22-22: Use optional property syntax for properties that can be undefinedIn TypeScript, it's idiomatic to declare properties that may be absent using the optional property syntax (
?). This enhances code clarity and type safety.Apply this diff to update the type definitions:
- duplicate_to: string | undefined; + duplicate_to?: string; - duplicate_issue_detail: TInboxDuplicateIssueDetails | undefined; + duplicate_issue_detail?: TInboxDuplicateIssueDetails;Also applies to: 26-26
web/core/components/inbox/inbox-issue-status.tsx (1)
42-42: Consider improving readability.The conditional rendering logic could be more readable. Consider extracting the condition and message selection into a separate function.
+ const getStatusMessage = () => { + if (inboxIssue?.status === 0 && inboxIssue?.snoozed_till) { + return description; + } + return t(inboxIssueStatusDetail.title); + }; <div className="flex items-center gap-1"> <InboxStatusIcon type={inboxIssue?.status} size={iconSize} className="flex-shrink-0" /> <div className="font-medium text-xs whitespace-nowrap"> - {inboxIssue?.status === 0 && inboxIssue?.snoozed_till ? description : t(inboxIssueStatusDetail.title)} + {getStatusMessage()} </div> </div>Also applies to: 44-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/constants/src/inbox.ts(1 hunks)packages/types/src/inbox.d.ts(1 hunks)web/core/components/inbox/inbox-filter/applied-filters/status.tsx(2 hunks)web/core/components/inbox/inbox-filter/filters/status.tsx(3 hunks)web/core/components/inbox/inbox-issue-status.tsx(3 hunks)web/core/components/inbox/inbox-status-icon.tsx(1 hunks)web/core/components/inbox/sidebar/root.tsx(1 hunks)web/core/services/inbox/inbox-issue.service.ts(1 hunks)web/core/store/inbox/inbox-issue.store.ts(1 hunks)web/core/store/inbox/project-inbox.store.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- web/core/components/inbox/sidebar/root.tsx
- web/core/store/inbox/project-inbox.store.ts
- web/core/services/inbox/inbox-issue.service.ts
- web/core/store/inbox/inbox-issue.store.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- web/core/components/inbox/inbox-filter/applied-filters/status.tsx
- web/core/components/inbox/inbox-filter/filters/status.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
web/core/components/inbox/inbox-issue-status.tsx (3)
5-6: LGTM! Well-organized imports.The imports are properly structured, with clear separation between constants, helpers, and components. The migration to
@plane/constantsand addition of i18n support aligns well with the PR objectives.Also applies to: 8-8, 11-11
21-22: LGTM! Proper translation hook setup.The translation hook is correctly initialized and follows React best practices.
29-31: LGTM! Well-implemented translation with dynamic content.The description handling correctly uses translation interpolation with named parameters and separates date calculation logic into a helper function.
packages/types/src/inbox.d.ts (1)
24-24: LGTM: Simplified union type definitionThe consolidation of
TInboxIssueSortingOrderByKeysinto a single line enhances code readability and maintains consistency.
| size, | ||
| className, | ||
| }: { | ||
| type: TInboxIssueStatus; |
There was a problem hiding this comment.
Ensure consistency between parameter definition and null check
The type parameter is declared as required (type: TInboxIssueStatus), but you're checking if it's undefined. To maintain type safety, consider making type optional or remove the undefined check if type should always be provided.
Apply this diff to correct the inconsistency:
- type: TInboxIssueStatus;
+ type?: TInboxIssueStatus;
...
- if (type === undefined) return null;
+ if (!type) return null;Alternatively, if type is always expected to have a value, you can remove the undefined check:
- if (type === undefined) return null;Also applies to: 21-21
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores