feat(content-sharing): Sharing service updateSharedLink #4336
Conversation
WalkthroughIntroduces sharedLink/serverURL handling and sharingServiceProps state; refactors hooks and createSharingService to accept config objects; moves shared-link conversion logic into utils and adds updateSharedLink, new types for shared-link settings, and related tests and converters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ContentSharingV2
participant useSharingService
participant SharingService
participant ItemAPI as Item API
User->>ContentSharingV2: change shared link settings / permission
ContentSharingV2->>useSharingService: init({ api, itemId, itemType, sharedLink, sharingServiceProps, ... })
useSharingService->>SharingService: createSharingService({ itemApiInstance, onSuccess, options })
rect rgba(200,230,255,0.25)
note right of SharingService: Permission update flow
ContentSharingV2->>SharingService: changeSharedLinkPermission(level)
SharingService->>ItemAPI: updatePermission({ id, permissions })
ItemAPI-->>SharingService: success
SharingService-->>useSharingService: onSuccess(itemData)
useSharingService-->>ContentSharingV2: setItem / setSharedLink
end
rect rgba(200,255,200,0.18)
note right of SharingService: Shared-link settings update flow (new)
ContentSharingV2->>SharingService: updateSharedLink(newSettings)
SharingService->>SharingService: convertSharedLinkSettings(newSettings, access, isDownloadAvailable, serverURL)
SharingService->>ItemAPI: updateSharedLink(transformedSettings, UPDATE_PARAMS)
ItemAPI-->>SharingService: success
SharingService-->>useSharingService: onSuccess(itemData)
useSharingService-->>ContentSharingV2: setItem / setSharedLink
end
sequenceDiagram
autonumber
participant API as Items API
participant ContentSharingV2
participant useSharingService
API-->>ContentSharingV2: getItemSuccess({ item, sharingServicePropsFromApi, hostname })
ContentSharingV2->>ContentSharingV2: setSharingServiceProps(sharingServicePropsFromApi)
ContentSharingV2->>ContentSharingV2: serverURL = hostname + "v/"
ContentSharingV2->>useSharingService: pass sharedLink (with serverURL) and sharingServiceProps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/elements/content-sharing/types.js (1)
1-185: File extension and syntax mismatch.This file uses a
.jsextension but contains TypeScript-only syntax (import type,interface). Additionally, Line 179 uses Flow's nullable syntax (?DateValue) instead of TypeScript's optional syntax (DateValue | null).Rename the file to
.tsand fix the nullable syntax:-// @flow -import type { CollaborationRole, DateValue, Item, SharedLink } from '@box/unified-share-modal'; +import type { CollaborationRole, DateValue, Item, SharedLink } from '@box/unified-share-modal';export interface SharedLinkSettings { - expiration: ?DateValue; + expiration: DateValue | null; isDownloadEnabled: boolean; isExpirationEnabled: boolean; isPasswordEnabled: boolean; password: string; vanityName: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/elements/content-sharing/ContentSharingV2.tsx(3 hunks)src/elements/content-sharing/__tests__/ContentSharingV2.test.tsx(1 hunks)src/elements/content-sharing/__tests__/sharingService.test.ts(3 hunks)src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts(5 hunks)src/elements/content-sharing/hooks/useSharingService.ts(3 hunks)src/elements/content-sharing/sharingService.ts(1 hunks)src/elements/content-sharing/types.js(2 hunks)src/elements/content-sharing/utils/__tests__/convertSharingServiceData.test.ts(1 hunks)src/elements/content-sharing/utils/convertItemResponse.ts(1 hunks)src/elements/content-sharing/utils/convertSharingServiceData.ts(1 hunks)src/elements/content-sharing/utils/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/elements/content-sharing/ContentSharingV2.tsx (2)
src/elements/content-sharing/hooks/useSharingService.ts (1)
useSharingService(7-56)src/elements/content-sharing/utils/convertItemResponse.ts (1)
convertItemResponse(8-109)
src/elements/content-sharing/utils/__tests__/convertSharingServiceData.test.ts (1)
src/elements/content-sharing/utils/convertSharingServiceData.ts (2)
convertSharedLinkPermissions(15-24)convertSharedLinkSettings(34-79)
src/elements/content-sharing/sharingService.ts (1)
src/elements/content-sharing/utils/convertSharingServiceData.ts (2)
convertSharedLinkPermissions(15-24)convertSharedLinkSettings(34-79)
src/elements/content-sharing/hooks/useSharingService.ts (1)
src/elements/content-sharing/sharingService.ts (1)
createSharingService(27-56)
src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (3)
src/elements/content-sharing/hooks/useSharingService.ts (1)
useSharingService(7-56)src/elements/content-sharing/sharingService.ts (1)
createSharingService(27-56)src/elements/content-sharing/utils/convertItemResponse.ts (1)
convertItemResponse(8-109)
src/elements/content-sharing/__tests__/sharingService.test.ts (2)
src/elements/content-sharing/utils/convertSharingServiceData.ts (2)
convertSharedLinkPermissions(15-24)convertSharedLinkSettings(34-79)src/elements/content-sharing/sharingService.ts (1)
createSharingService(27-56)
🪛 Biome (2.1.2)
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)
[error] 179-179: Expected a type but instead found '?'.
Expected a type here.
(parse)
[error] 179-179: Expected a property, or a signature but instead found ';'.
Expected a property, or a signature here.
(parse)
[error] 184-185: Expected a statement but instead found '}'.
Expected a statement here.
(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: lint_test_build
- GitHub Check: Summary
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
src/elements/content-sharing/utils/convertItemResponse.ts (1)
68-68: LGTM! Expiration timestamp conversion is correct.The conversion from ISO string to milliseconds using
new Date(expirationTimestamp).getTime()is appropriate and aligns with the new shared link data flow.src/elements/content-sharing/__tests__/ContentSharingV2.test.tsx (1)
161-161: LGTM! Test mock updated correctly.The addition of
updateSharedLinkto the mock sharingService aligns with the new capability being added.src/elements/content-sharing/utils/index.ts (1)
2-2: LGTM! Public API expanded correctly.Exposing
convertSharedLinkPermissionsandconvertSharedLinkSettingsfrom the utils layer enables reuse across modules and aligns with the refactoring to use config objects.src/elements/content-sharing/ContentSharingV2.tsx (4)
46-46: LGTM! State added for sharing service props.The new
sharingServicePropsstate stores permissions data needed by the sharing service, enabling proper configuration of shared link operations.
53-62: LGTM! Hook invocation refactored to use config object.The change from passing multiple arguments to a single configuration object improves maintainability and aligns with the updated
useSharingServicesignature.
71-76: LGTM! Sharing service props extracted and stored.The extraction of
sharingServicePropsFromApifrom the converted item response and storage viasetSharingServicePropscorrectly wires up the new data flow.
107-112: Verify serverURL construction logic Ensurehostnametrailing‐slash handling aligns with Box’s vanity URL format so that${hostname}v/yields a valid path (e.g.https://acme.box.com/v/).src/elements/content-sharing/hooks/__tests__/useSharingService.test.ts (3)
29-44: LGTM! Mock data expanded correctly.The mock data now includes
sharingServiceProps,sharedLink.serverURL, andsharedLink.settings, which align with the updated hook requirements and data flow.
49-63: LGTM! Test helper improves maintainability.The
renderHookWithPropshelper function centralizes default props and allows overrides, making tests more readable and easier to maintain.
116-127: LGTM! Test assertions updated correctly.The test expectations now verify the new
optionsstructure includingaccess,isDownloadAvailable,permissions(from sharingServiceProps), andserverURL, correctly reflecting the updated hook implementation.src/elements/content-sharing/utils/__tests__/convertSharingServiceData.test.ts (1)
1-194: LGTM! Comprehensive test coverage for conversion utilities.The test suite thoroughly covers both
convertSharedLinkPermissionsandconvertSharedLinkSettings, including edge cases for expiration, vanity URLs, permissions, and passwords across different access levels.src/elements/content-sharing/hooks/useSharingService.ts (4)
7-16: LGTM! Hook signature refactored to config object.The change from multiple parameters to a single configuration object improves the API ergonomics and makes it easier to extend with additional options in the future.
18-31: LGTM! Guard condition extended correctly.The guard now checks both
itemandsharedLink, ensuring the API instance is only created when both are available. The dependency array is correctly updated to includesharedLink.
38-44: LGTM! Options object constructed correctly.The options object includes all necessary fields (
id,access,permissions,serverURL,isDownloadAvailable) derived from the appropriate sources, enabling proper shared link operations.
52-53: LGTM! Service creation and dependencies updated correctly.The
createSharingServicecall passes the newoptionsstructure, and the dependency array is properly updated to includesharingServicePropsandsharedLink, ensuring the service is recreated when these values change.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/elements/content-sharing/utils/convertSharingServiceData.ts (2)
22-24: Fix preview flag when download permission is selected.Setting
permissionLevelto download currently forcescan_previewtofalse, which strips preview access in the API payload. Download permission should always imply preview access.Apply this diff to ensure preview remains enabled for download permission:
return { [PERMISSION_CAN_DOWNLOAD]: permissionLevel === PERMISSION_CAN_DOWNLOAD, - [PERMISSION_CAN_PREVIEW]: permissionLevel === PERMISSION_CAN_PREVIEW, + [PERMISSION_CAN_PREVIEW]: + permissionLevel === PERMISSION_CAN_DOWNLOAD || permissionLevel === PERMISSION_CAN_PREVIEW, };
53-56: Preserve preview access when downloads are enabled.Initializing
can_previewwith!isDownloadEnabledrevokes preview access when downloads are turned on. Preview should remaintrueregardless of the download toggle, so only thecan_downloadflag should vary.Apply this diff to fix the logic:
-const permissions: ConvertSharedLinkSettingsReturnType['permissions'] = { can_preview: !isDownloadEnabled }; +const permissions: ConvertSharedLinkSettingsReturnType['permissions'] = { can_preview: true }; if (isDownloadAvailable) { permissions.can_download = isDownloadEnabled; }
🧹 Nitpick comments (1)
src/elements/content-sharing/utils/convertSharingServiceData.ts (1)
48-48: Consider validating the vanity_url construction.Concatenating
serverURLandvanityNamewithout checking for a separator (e.g., ensuringserverURLends with/) could produce malformed URLs. If the format is guaranteed by upstream code, document that assumption; otherwise, add validation or normalization.Example defensive approach:
-vanity_url: serverURL && vanityName ? `${serverURL}${vanityName}` : '', +vanity_url: serverURL && vanityName + ? `${serverURL.replace(/\/$/, '')}/${vanityName.replace(/^\//, '')}` + : '',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/elements/content-sharing/utils/__tests__/convertSharingServiceData.test.ts(1 hunks)src/elements/content-sharing/utils/convertItemResponse.ts(1 hunks)src/elements/content-sharing/utils/convertSharingServiceData.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-sharing/utils/convertItemResponse.ts
- src/elements/content-sharing/utils/tests/convertSharingServiceData.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/elements/content-sharing/utils/convertSharingServiceData.ts (1)
src/utils/datetime.ts (1)
convertISOStringToUTCDate(244-244)
⏰ 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
🔇 Additional comments (2)
src/elements/content-sharing/utils/convertSharingServiceData.ts (2)
74-80: LGTM! Password handling logic is correct.The conditional logic correctly handles all three cases:
- Setting a new password when
isPasswordEnabled && !!password- Removing an existing password when
!isPasswordEnabled(setsnull)- Preserving an existing password when
isPasswordEnabledbutpasswordis empty (omits the field)This aligns with the detailed comment and API expectations.
46-46: Do not remove convertISOStringToUTCDate: it applies the local timezone offset that.toISOString()alone doesn’t, so the full chain is required to normalizeunshared_atcorrectly to UTC.Likely an incorrect or invalid review comment.
What
Before this PR: cannot update shared link settings
After this PR: updateSharedLink API call (created and used by existing contentSharingV1) is called, the updated shared link settings are passed in successfully, and the UI reflex the item settings is updated properly.
Testing:
Summary by CodeRabbit