feat(content-sharing): Create sharing service sendInvitations #4344
Conversation
WalkthroughThis PR adds collaborator-aware invitation sending to content sharing: expands useSharingService inputs/return to include invitation sending and collaborator state, updates avatar-fetch and owner handling, renames serverURL→serverUrl across sharing utilities, and introduces collaborator conversion helpers and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Comp as ContentSharingV2
participant Hook as useSharingService
participant Inv as useInvites
participant API as Box API
participant Utils as convertCollab/convertCollabsRequest
Comp->>Hook: mount(props: avatarURLMap, collaborators, currentUserId, setCollaborators, sharingServiceProps)
Hook->>Inv: build sendInvitations(transformRequest -> convertCollabsRequest)
note right of Inv #E8F5E9: sendInvitations available on sharingService
Comp->>Hook: user triggers sendInvitations(collabRequest)
Hook->>Inv: invoke sendInvitations(collabRequest)
alt collaborators present (V2)
Inv->>Utils: convertCollabsRequest(collabRequest, existingCollaborators)
Utils-->>Inv: { users, groups }
Inv->>API: POST invites for users/groups
API-->>Inv: success (new collaborator entries)
Inv->>Hook: handleSuccess(newEntries)
Hook->>Utils: convertCollab(newEntry, context: owner, avatarURLMap, currentUserId)
Utils-->>Hook: converted collaborator
Hook->>Comp: setCollaborators(append converted collaborator)
else collaborators missing
Inv-->>Hook: early return / no-op
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, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (4)
src/elements/content-sharing/constants.js (1)
85-86: Consider reusing existing constants to avoid duplication.These constants are identical to those in
src/features/unified-share-modal/constants.js(lines 22-23). Consider importing them from the existing location or extracting them to a shared constants file to maintain a single source of truth.Apply this diff to import the existing constants:
+import { COLLAB_USER_TYPE, COLLAB_GROUP_TYPE } from '../../features/unified-share-modal/constants'; + export const ANYONE_WITH_LINK = 'peopleWithTheLink'; export const ANYONE_IN_COMPANY = 'peopleInYourCompany'; export const PEOPLE_IN_ITEM = 'peopleInThisItem'; - -export const COLLAB_USER_TYPE = 'user'; -export const COLLAB_GROUP_TYPE = 'group'; + +export { COLLAB_USER_TYPE, COLLAB_GROUP_TYPE };src/elements/content-sharing/types.js (1)
128-128: Specify a more precise type for the collaborators array.The generic
Arraytype provides minimal type safety. Consider specifying the element type to improve type checking and developer experience.Apply this diff to add a more specific type:
export type UseInvitesOptions = ContentSharingHooksOptions & { - collaborators?: Array, + collaborators?: Array<{ id: string, type: string, email: string, role: string }>, transformRequest: InviteCollaboratorsRequest => ContentSharingCollaborationsRequest, };Alternatively, if a Collaborator type already exists elsewhere, reference it:
+import type { Collaborator } from '@box/unified-share-modal'; + export type UseInvitesOptions = ContentSharingHooksOptions & { - collaborators?: Array, + collaborators?: Array<Collaborator>, transformRequest: InviteCollaboratorsRequest => ContentSharingCollaborationsRequest, };src/elements/content-sharing/ContentSharingV2.tsx (1)
162-162: Consider optimizing the owner dependency to prevent unnecessary re-fetches.Including the
ownerobject directly in the dependency array will cause the effect to re-run wheneversetOwneris called, even if the owner data values haven't changed. This happens because React compares object references, not their contents.Consider one of these approaches:
- Include only the primitive values in the dependency array:
[api, avatarURLMap, collaboratorsData, itemID, owner.id]- Use
useMemoto memoize the owner object based on its properties- Store owner as primitive values in separate state variables
Option 1 is the simplest fix:
- }, [api, avatarURLMap, collaboratorsData, itemID, owner]); + }, [api, avatarURLMap, collaboratorsData, itemID, owner.id]);src/elements/content-sharing/hooks/useSharingService.ts (1)
76-90: Consider making the email domain extraction more robust.The current implementation extracts the owner email domain with a regex test, but could handle edge cases more explicitly.
Consider this slightly more defensive approach:
- const handleSuccess = response => { - const { id: ownerId, login: ownerEmail } = response.created_by; - const ownerEmailDomain = ownerEmail && /@/.test(ownerEmail) ? ownerEmail.split('@')[1] : null; + const handleSuccess = response => { + const { id: ownerId, login: ownerEmail } = response.created_by; + const ownerEmailDomain = ownerEmail?.includes('@') ? ownerEmail.split('@')[1] : null; setCollaborators(prevList => {This uses optional chaining and
includes()for slightly cleaner null handling, though the current implementation is also correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/elements/content-sharing/ContentSharingV2.tsx(3 hunks)src/elements/content-sharing/SharingNotification.js(2 hunks)src/elements/content-sharing/__tests__/useInvites.test.js(6 hunks)src/elements/content-sharing/constants.js(1 hunks)src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts(12 hunks)src/elements/content-sharing/hooks/useInvites.js(3 hunks)src/elements/content-sharing/hooks/useSharingService.ts(3 hunks)src/elements/content-sharing/types.js(1 hunks)src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts(2 hunks)src/elements/content-sharing/utils/__tests__/convertItemResponse.test.ts(1 hunks)src/elements/content-sharing/utils/convertCollaborators.ts(2 hunks)src/elements/content-sharing/utils/convertItemResponse.ts(1 hunks)src/elements/content-sharing/utils/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/elements/content-sharing/constants.js (1)
src/features/unified-share-modal/constants.js (2)
COLLAB_USER_TYPE(24-24)COLLAB_GROUP_TYPE(23-23)
src/elements/content-sharing/utils/__tests__/convertItemResponse.test.ts (1)
src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js (4)
mockOwnerEmail(34-34)mockOwnerEmail(34-34)mockOwnerId(36-36)mockOwnerId(36-36)
src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts (1)
src/elements/content-sharing/utils/convertCollaborators.ts (1)
convertCollabsRequest(84-122)
src/elements/content-sharing/ContentSharingV2.tsx (1)
src/elements/content-sharing/apis/fetchAvatars.ts (1)
fetchAvatars(3-22)
src/elements/content-sharing/SharingNotification.js (4)
src/elements/content-sharing/ContentSharing.js (1)
api(89-89)src/features/unified-share-modal/utils/convertData.js (2)
USM_TO_API_ACCESS_LEVEL_MAP(89-93)USM_TO_API_ACCESS_LEVEL_MAP(89-93)src/components/notification/Notification.js (2)
TYPE_ERROR(34-34)TYPE_INFO(32-32)src/elements/content-sharing/utils/index.ts (2)
convertSharedLinkPermissions(8-8)convertSharedLinkSettings(8-8)
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (2)
src/elements/content-sharing/hooks/useSharingService.ts (1)
useSharingService(8-99)src/elements/content-sharing/utils/convertCollaborators.ts (2)
convertCollab(17-52)convertCollabsRequest(84-122)
src/elements/content-sharing/hooks/useInvites.js (2)
src/elements/content-sharing/SharingModal.js (1)
sendInvites(94-94)src/features/unified-share-modal/utils/convertData.js (1)
collaborators(435-435)
src/elements/content-sharing/utils/convertCollaborators.ts (1)
src/elements/content-sharing/constants.js (4)
COLLAB_USER_TYPE(85-85)COLLAB_USER_TYPE(85-85)COLLAB_GROUP_TYPE(86-86)COLLAB_GROUP_TYPE(86-86)
src/elements/content-sharing/hooks/useSharingService.ts (1)
src/elements/content-sharing/utils/convertCollaborators.ts (2)
convertCollab(17-52)convertCollabsRequest(84-122)
🪛 Biome (2.1.2)
src/elements/content-sharing/hooks/useInvites.js
[error] 49-49: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 50-50: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (25)
src/elements/content-sharing/__tests__/useInvites.test.js (1)
7-7: LGTM!The test additions appropriately cover the new collaborators parameter, including the edge case where collaborators is not provided. The mock data is minimal yet sufficient for testing purposes.
Also applies to: 123-135
src/elements/content-sharing/utils/__tests__/convertItemResponse.test.ts (1)
47-48: LGTM!The test expectations correctly validate that
ownerEmailandownerIdare populated in thesharingServiceobject from the owner data.src/elements/content-sharing/utils/index.ts (1)
1-1: LGTM!The expanded exports appropriately expose the new conversion utilities for collaborator requests alongside the existing response converter.
src/elements/content-sharing/utils/convertCollaborators.ts (1)
78-81: LGTM!The change from mutating
entrieswithunshiftto using the spread operator creates a more functional, immutable approach that's easier to reason about.src/elements/content-sharing/utils/__tests__/convertCollaborators.test.ts (1)
258-320: LGTM!The test suite comprehensively validates
convertCollabsRequestfunctionality, including proper separation of users and groups, filtering of existing collaborators, and handling of empty inputs.src/elements/content-sharing/utils/convertItemResponse.ts (1)
106-107: No changes needed—null safety is guaranteed by type definitions.The
owned_byfield inContentSharingItemAPIResponseis defined as a required property with requiredidandloginfields (both non-optional strings). SinceownedByis destructured fromitemAPIDataof typeContentSharingItemAPIResponse, TypeScript/Flow guarantees bothownedBy.loginandownedBy.idexist and are strings. The code at lines 106-107 is safe as-is.src/elements/content-sharing/ContentSharingV2.tsx (3)
56-64: LGTM!The hook now correctly receives the necessary parameters for supporting invitations including
avatarURLMap,collaborators,currentUserId, andsetCollaborators.
118-121: LGTM!The serverURL is now correctly managed through
sharingServicePropsinstead of being set directly onsharedLink, which aligns with the architectural changes in this PR.
148-159: LGTM!The owner is correctly included in the avatar fetch by creating an
ownerEntrythat matches the expected collaborator structure. This ensures the owner's avatar is fetched alongside regular collaborators.src/elements/content-sharing/SharingNotification.js (2)
235-235: LGTM!The empty
collaboratorsarray is correctly passed touseInvites, aligning with the updated hook API that now requires the collaborators parameter. An empty array is the appropriate initial value in this context.
173-203: Formatting update looks good.The restructuring of the
useSharedLinkcall improves readability without changing the logic.src/elements/content-sharing/hooks/useInvites.js (4)
21-21: LGTM!The
collaboratorsparameter is correctly extracted from options to support the updated invitation flow.
30-30: LGTM!The guard correctly prevents the hook from initializing when
collaboratorsis null or undefined, ensuring the hook only operates when collaborator data is available.
49-58: LGTM!The refactoring to an arrow function maintains the same behavior while improving code clarity.
65-65: LGTM!Adding
collaboratorsto the dependency array ensures the effect re-runs when the collaborators list changes, which is the correct behavior.src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (5)
5-12: LGTM!The imports and mocks are correctly updated to support testing the new invitation functionality.
25-32: LGTM!The mock for
sendInvitationsand the structuredsharingServicePropswithserverURLcorrectly reflect the new hook API.
61-69: LGTM!The hook is now called with all the required parameters for the invitation flow including
avatarURLMap,collaborators,currentUserId, andsetCollaborators.
84-86: LGTM!Mock implementations for
useInvites,convertCollab, andconvertCollabsRequestare correctly set up to support the test scenarios.
232-295: LGTM!The new test suite for
sendInvitationsprovides comprehensive coverage:
- Verifies
useInvitesis called with correct parameters- Tests the success callback flow including
convertCollabinvocation- Validates
transformRequestusesconvertCollabsRequestwith collaboratorsThe test scenarios are well-structured and align with the implementation.
src/elements/content-sharing/hooks/useSharingService.ts (5)
4-6: LGTM!The imports correctly include the utilities needed for the invitation flow:
convertCollab,convertCollabsRequest, anduseInvites.
10-18: LGTM!The hook signature is correctly expanded to accept the parameters needed for invitations:
avatarURLMap,collaborators,currentUserId, andsetCollaborators.
47-51: LGTM!The permissions are now correctly structured as an object with
can_set_share_accessandcan_shareproperties, andserverURLis properly sourced fromsharingServicePropsinstead ofsharedLink.
92-96: LGTM!The
useInviteshook is correctly invoked with the proper parameters:
collaboratorsenables the hook to initializehandleSuccessprocesses the response and updates the collaborator listtransformRequestusesconvertCollabsRequestto transform the invitation request data
98-98: LGTM!The return statement correctly exposes
sendInvitationsas part of thesharingServiceobject. WhensharingServiceis null (no itemApiInstance), the spread operation is safely ignored andsendInvitationsis still available.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/elements/content-sharing/hooks/useInvites.js (2)
50-59: Reset loading state and tolerate partial failures.Currently
setIsLoading(true)is set per request and never reset;Promise.allalso fails fast. PreferallSettledand a singlefinallyto clear loading after all requests finish.- return Promise.all(promises); + try { + const results = await Promise.allSettled(promises); + return results; + } finally { + setIsLoading(false); + }If you need only fulfilled values, filter by
status === 'fulfilled'.
21-31: Guard depends oncollaboratorspresence; default to empty array.If
isContentSharingV2Enabledis true andcollaboratorsisundefined, initialization is skipped. Defaultcollaborators = []to avoid gating on presence.- const { - collaborators, + const { + collaborators = [], handleSuccess = noop, handleError = noop, isContentSharingV2Enabled, setIsLoading = noop, transformRequest, transformResponse = arg => arg, } = options; @@ - if (sendInvites || (isContentSharingV2Enabled && !collaborators)) return; + if (sendInvites) return;Also, Biome flags Flow annotations here; exclude this file from Biome or configure it to ignore Flow syntax.
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (2)
232-296: Good coverage forsendInvitationswiring and conversions.The tests validate parameters, success mapping via
convertCollab, andconvertCollabsRequestusage. Looks solid.Consider adding:
- A test asserting returned
sendInvitationsawaits all invite calls (i.e., does not return nested arrays).- A test for behavior when
collaboratorsisundefinedto ensure the hook still initializes.- If you add loading toggling, assert
setIsLoadingis cleared after completion.
96-118: Ensure production code safely spreads possibly nullsharingService.Tests expect
{ sendInvitations }even when the underlying service is null. Confirm implementation guards the spread (e.g.,{ ...(sharingService ?? {}), sendInvitations }) to avoid runtime errors in environments without Babel’s null-safe helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/elements/content-sharing/SharingNotification.js(1 hunks)src/elements/content-sharing/__tests__/useInvites.test.js(2 hunks)src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts(12 hunks)src/elements/content-sharing/hooks/useInvites.js(2 hunks)src/elements/content-sharing/hooks/useSharingService.ts(3 hunks)src/elements/content-sharing/types.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/elements/content-sharing/tests/useInvites.test.js
- src/elements/content-sharing/SharingNotification.js
- src/elements/content-sharing/hooks/useSharingService.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (3)
src/elements/content-sharing/hooks/useSharingService.ts (1)
useSharingService(8-100)src/elements/content-sharing/utils/convertCollaborators.ts (2)
convertCollab(17-52)convertCollabsRequest(84-122)src/elements/content-sharing/utils/index.ts (2)
convertCollab(1-1)convertCollabsRequest(1-1)
src/elements/content-sharing/hooks/useInvites.js (2)
src/elements/content-sharing/hooks/useContacts.js (1)
noop(20-27)src/features/unified-share-modal/utils/convertData.js (1)
collaborators(435-435)
src/elements/content-sharing/types.js (1)
src/features/presence/PresenceAvatarList.tsx (1)
Collaborator(10-18)
🪛 Biome (2.1.2)
src/elements/content-sharing/hooks/useInvites.js
[error] 50-50: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 51-51: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
src/elements/content-sharing/types.js
[error] 2-2: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (3)
src/elements/content-sharing/types.js (1)
2-2: Biome false positive on Flowimport type.This file is Flow-typed (
// @flow). Biome flagsimport typeas TS-only. Exclude Flow files from Biome or configure it to ignore Flow syntax for.jswith@flow.You can verify by updating Biome config ignore patterns (e.g.,
**/src/elements/content-sharing/**/*.js) or swapping to TypeScript in a follow-up if migration is planned.src/elements/content-sharing/hooks/useInvites.js (1)
50-51: Biome false positive on Flow type annotations.Lines contain Flow type annotations (
SendInvitesFnType,InviteCollaboratorsRequest). Biome treats them as TS-only. Exclude Flow files from Biome or add per-file ignore.src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (1)
125-147: Assertion updates forpermissionsandserverURLlook correct.Assertions extract
can_set_share_access,can_share, andserverURLand verify they are passed tocreateSharingServiceoptions. Matches the intended API surface.Also applies to: 181-203
reneshen0328
force-pushed
the
create-sendInvitations
branch
from
77d45b2 to
6477132
Compare
What
Before this PR
After this PR
Testing:
Summary by CodeRabbit
New Features
Bug Fixes
Tests