feat(content-sharing): Render notification for sendInvitations#4347
feat(content-sharing): Render notification for sendInvitations#4347mergify[bot] merged 4 commits intomasterfrom
Conversation
WalkthroughAdds localized invitation notifications, threads avatar URL mapping into collaborator conversion, updates collaborator-type handling for non-group contacts, and expands tests to assert formatted notification messages; includes a minor formatting change in contact avatar utility. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Component
participant Hook as useSharingService
participant Invites as useInvites
participant Intl as formatMessage
App->>Hook: handleSendInvitations(contacts)
Hook->>Invites: sendInvites(...)
Invites-->>Hook: result {succeeded[], failed[]} / null
alt result is null
Hook-->>App: null
else
alt all succeeded
Hook->>Intl: format(sendInvitationsSuccess, {count})
Intl-->>Hook: localized success text
Hook-->>App: { success: { text } }
else some failed
Hook->>Intl: format(sendInvitationsError, {countFailed})
Intl-->>Hook: localized error text
Hook->>Intl: format(sendInvitationsSuccess, {countSucceeded})
Intl-->>Hook: localized success text
Hook-->>App: { error: { text }, success: { text } }
else all failed
Hook->>Intl: format(sendInvitationsError, {count})
Intl-->>Hook: localized error text
Hook-->>App: { error: { text } }
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)src/elements/content-sharing/hooks/useSharingService.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (3)
239-257: Good coverage for useInvites wiring.Asserting
isContentSharingV2Enabled: true,handleSuccess, andtransformRequestis sufficient. Consider also asserting the exactitemTypefor folders in the folder block for symmetry.
311-331: All-success path covered; consider asserting single message.Add
expect(sendInvitationsResult.messages).toHaveLength(1);to guard against accidental extra notifications.
100-116: Rename test titles for accuracy.They assert that API instances aren’t created and that a “sendInvitations-only” sharingService is returned, not that sharingService is null. Rename for clarity (no behavior change).
src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts (1)
308-345: Missing-type contacts treated as users.Test correctly validates that non-group contacts without
typeare added as users. Ensure server-side validations still prevent unintended external invites where policy disallows them.If policies exist, add a test ensuring external domains are rejected at a higher layer.
src/elements/content-sharing/messages.js (1)
62-74: Normalize ICU plural message formatting.Minor: remove extra spaces after “{” and before “}” for consistency with our ICU style.
- defaultMessage: - '{ count, plural, one { An invitation to join this item was not sent.} other { {count} invitations to join this item were not sent.}}', + defaultMessage: + '{count, plural, one {An invitation to join this item was not sent.} other {{count} invitations to join this item were not sent.}}',src/elements/content-sharing/utils/convertCollaborators.ts (2)
9-15: Fix type-safety and equality checks in ConvertCollabProps.
ownerEmailDomaincan be null;currentUserIdmay be string or number; strict===can miscompare. Update types and compare as strings.Apply this diff:
export interface ConvertCollabProps { - collab: Collaboration; - currentUserId: string; + collab: Collaboration; + currentUserId: string | number; isCurrentUserOwner: boolean; - ownerEmailDomain: string; + ownerEmailDomain: string | null; avatarUrlMap?: AvatarURLMap; } @@ - const isCurrentUser = collabId === currentUserId; + const isCurrentUser = String(collabId) === String(currentUserId);Consider aligning upstream types for
currentUserIdandaccessible_by.idto the same primitive to avoid future coercions.Also applies to: 17-23, 33-37
54-59: Broaden owner id typing to match actual data.Owner ids can be numeric. Loosen the type to avoid down-stream casts.
-export const convertCollabsResponse = ( +export const convertCollabsResponse = ( collabsApiData: Collaborations, - currentUserId: string, - owner: { id: string; email: string; name: string }, + currentUserId: string | number, + owner: { id: string | number; email: string; name: string }, avatarUrlMap?: AvatarURLMap, ): Collaborator[] => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
i18n/en-US.propertiesis excluded by!i18n/**
📒 Files selected for processing (6)
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts(3 hunks)src/elements/content-sharing/hooks/useContactService.ts(1 hunks)src/elements/content-sharing/hooks/useSharingService.ts(3 hunks)src/elements/content-sharing/messages.js(1 hunks)src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts(6 hunks)src/elements/content-sharing/utils/convertCollaborators.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/elements/content-sharing/hooks/useSharingService.ts (3)
src/elements/content-sharing/SharingNotification.js (1)
newCollab(240-244)src/elements/content-sharing/utils/convertCollaborators.ts (1)
convertCollabsRequest(84-120)src/elements/content-sharing/messages.js (1)
messages(3-80)
src/elements/content-sharing/utils/convertCollaborators.ts (1)
src/elements/content-sharing/constants.js (2)
COLLAB_USER_TYPE(85-85)COLLAB_USER_TYPE(85-85)
src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts (2)
src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js (4)
mockAvatarUrlMap(30-32)mockAvatarUrlMap(30-32)mockOwnerId(36-36)mockOwnerId(36-36)src/elements/content-sharing/utils/convertCollaborators.ts (2)
convertCollab(17-52)convertCollabsRequest(84-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint_test_build
- GitHub Check: Summary
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
src/elements/content-sharing/hooks/useContactService.ts (1)
28-30: LGTM! Good formatting practice.Adding braces to the single-line conditional improves maintainability and follows best practices, preventing potential bugs during future modifications.
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (2)
13-19: Intl mock looks good; reset behavior is correct.
jest.clearAllMocks()resetsmockFormatMessagecalls between tests. No changes needed.
332-357: Partial-success ordering verified.Nice use of
toHaveBeenNthCalledWithto lock message order. LGTM.src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts (1)
67-73: Avatar URL map propagation verified.Passing
avatarUrlMapintoconvertCollabacross cases is correct and aligns with implementation. LGTM.Also applies to: 92-99, 104-112, 116-123, 140-148, 154-164, 173-181
src/elements/content-sharing/hooks/useSharingService.ts (1)
25-26: Intl + avatarUrlMap + useInvites wiring is sound.Threading
avatarUrlMap, addingformatMessage, and passingtransformRequestare correct. LGTM.Also applies to: 86-91, 97-103
src/elements/content-sharing/utils/convertCollaborators.ts (1)
108-116: Non-group → user fallback is correct.This addresses missing
typeon contacts and supports free-typed emails. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
i18n/en-US.propertiesis excluded by!i18n/**
📒 Files selected for processing (2)
src/elements/content-sharing/hooks/useSharingService.ts(3 hunks)src/elements/content-sharing/messages.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/elements/content-sharing/hooks/useSharingService.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint_test_build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
3 participants
What
Testing
Summary by CodeRabbit
New Features
Tests
Refactor