feat(content-sharing): add email shared link#4488
Merged
mergify[bot] merged 4 commits intomasterfrom Mar 26, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CSV2 as ContentSharingV2
participant Hook as useSharingService
participant Service as SharingService
participant Notif as Notifications
User->>CSV2: trigger sendSharedLink / sendInvitations
CSV2->>Hook: call sendSharedLink / sendInvitations (forwards onSendSharedLink)
Hook->>Service: invoke sendSharedLink / sendInvitations
Service-->>Hook: response (success/error)
Hook->>Notif: compose & publish messages
Notif-->>User: display success/error notifications
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
tjuanitas
commented
Mar 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/elements/content-sharing/constants.js (1)
90-110: Consider deriving the reverse role map to avoid future drift.Maintaining both maps manually risks mismatches over time.
Refactor suggestion
export const API_TO_USM_COLLAB_ROLE_MAP = { @@ }; -export const USM_TO_API_COLLAB_ROLE_MAP = { - co_owner: 'co-owner', - editor: 'editor', - owner: 'owner', - previewer: 'previewer', - previewer_uploader: 'previewer uploader', - uploader: 'uploader', - viewer: 'viewer', - viewer_uploader: 'viewer uploader', -}; +export const USM_TO_API_COLLAB_ROLE_MAP = Object.fromEntries( + Object.entries(API_TO_USM_COLLAB_ROLE_MAP).map(([apiRole, usmRole]) => [usmRole, apiRole]), +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sharing/constants.js` around lines 90 - 110, The two role maps (API_TO_USM_COLLAB_ROLE_MAP and USM_TO_API_COLLAB_ROLE_MAP) are duplicated and can drift; replace the manual USM_TO_API_COLLAB_ROLE_MAP with a programmatically derived inverse of API_TO_USM_COLLAB_ROLE_MAP (e.g., use Object.entries(API_TO_USM_COLLAB_ROLE_MAP).reduce to swap key/values), validate there are no duplicate values before inverting, and export the derived constant in place of the hard-coded reverse map so future changes to API_TO_USM_COLLAB_ROLE_MAP automatically propagate.src/elements/content-sharing/utils/convertCollaborators.ts (1)
89-105: Normalize collaborator/contact ids to the same type for reliable dedupe.Current Set membership checks compare
collab.userIdandcontact.idwithout normalization, which can miss matches across number/string boundaries.Based on learnings: In `src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js`, `mockCurrentUserID` is intentionally numeric while converted user ids are strings in later transformation stages.Refactor suggestion
if (currentCollabs) { currentCollabs.forEach(collab => { - currentCollabIds.add(collab.userId); + currentCollabIds.add(String(collab.userId)); }); } @@ - if (currentCollabIds.has(contact.id)) { + if (currentCollabIds.has(String(contact.id))) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sharing/utils/convertCollaborators.ts` around lines 89 - 105, The dedupe fails because Set membership compares raw collab.userId and contact.id with different types; normalize IDs to the same type (use String(...)) when inserting into currentCollabIds and when checking contacts. Update the block that builds currentCollabIds (add String(collab.userId)) and change the contacts loop membership check to use String(contact.id) (and use the normalized id when pushing into users/groups) so numeric mock IDs match string IDs later in the pipeline.src/elements/content-sharing/hooks/useSharingService.ts (1)
113-124: Return early when there are no contacts.
contacts?.lengthis only checked after awaitinghandleSendInvitations(data), so an empty submit still walks the invite pipeline. Move that guard up to keep the no-recipient path a true no-op.Suggested change
const sendInvitations = React.useCallback( async data => { if (!handleSendInvitations) { return null; } - const response = await handleSendInvitations(data); - const { contacts } = data; + if (!contacts?.length) { + return null; + } - if (!response || !contacts?.length) { + const response = await handleSendInvitations(data); + + if (!response) { return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sharing/hooks/useSharingService.ts` around lines 113 - 124, The sendInvitations callback currently awaits handleSendInvitations(data) before checking contacts, so empty submissions still invoke the invite pipeline; before calling handleSendInvitations(data) in the sendInvitations function, add an early return when data.contacts is falsy or has zero length (e.g., check contacts or contacts?.length) to short-circuit and return null; keep the existing response handling code (including the response variable and subsequent logic) only after this contacts guard so handleSendInvitations is not called for no-recipient submissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/elements/content-sharing/ContentSharingV2.tsx`:
- Around line 74-79: The config object in ContentSharingV2 incorrectly allows
usmConfig to override the intended sharedLinkEmail based on onSendSharedLink;
update the creation of config (the React.useMemo block for config) so that
sharedLinkEmail is set after spreading usmConfig (or explicitly override
usmConfig.sharedLinkEmail) — ensure sharedLinkEmail is computed as
!!onSendSharedLink and placed last in the object literal to take precedence over
any value from usmConfig.
In `@src/elements/content-sharing/hooks/useSharingService.ts`:
- Around line 151-156: The sharingService memo currently always includes a
sendSharedLink key even when onSendSharedLink is undefined; change the factory
to only add sendSharedLink when onSendSharedLink is provided (e.g., build the
object with sendInvitations and ...sharedLinkService, then conditionally assign
obj.sendSharedLink = onSendSharedLink or conditionally spread { sendSharedLink:
onSendSharedLink } only when onSendSharedLink is truthy) so feature detection
sees the key omitted when no callback exists; update the React.useMemo that
returns sharingService to perform this conditional inclusion for
onSendSharedLink.
In `@src/elements/content-sharing/utils/convertCollaborators.ts`:
- Around line 96-99: The mapped role from USM_TO_API_COLLAB_ROLE_MAP can be
undefined for unknown usmRole values; update convertCollaborators (around the
request handling where const { contacts, role: usmRole } = request and const
role = USM_TO_API_COLLAB_ROLE_MAP[usmRole]) to validate the mapping before
building user/group collaboration payloads: check that role is a defined value
and if not either skip that request, set a safe default role, or throw/return an
explicit error; apply the same validation to the logic that builds the user and
group request payloads (the blocks referenced around lines 108-123) so no
outgoing payloads contain role: undefined.
- Around line 53-54: The code currently assigns role:
API_TO_USM_COLLAB_ROLE_MAP[role] which can be undefined for unknown API roles;
change convertCollaborators to first lookup const mappedRole =
API_TO_USM_COLLAB_ROLE_MAP[role] and handle the unmapped case before returning a
Collaborator (e.g., throw a clear error, skip the collaborator, or assign a safe
default like 'viewer'), then use mappedRole for the returned object's role field
so no undefined role is produced.
In `@src/elements/content-sharing/utils/convertItemResponse.ts`:
- Around line 92-95: Filter or reject invitee roles that don't have a mapping in
API_TO_USM_COLLAB_ROLE_MAP before building collaborationRoles: validate
allowed_invitee_roles entries (e.g., in the code path that constructs
collaborationRoles) and skip any role where API_TO_USM_COLLAB_ROLE_MAP[role] is
undefined (or throw a descriptive error), then map the remaining roles to
objects with id set to the mapped value and isDefault computed with
INVITEE_ROLE_EDITOR so CollaborationRole.id is never undefined.
---
Nitpick comments:
In `@src/elements/content-sharing/constants.js`:
- Around line 90-110: The two role maps (API_TO_USM_COLLAB_ROLE_MAP and
USM_TO_API_COLLAB_ROLE_MAP) are duplicated and can drift; replace the manual
USM_TO_API_COLLAB_ROLE_MAP with a programmatically derived inverse of
API_TO_USM_COLLAB_ROLE_MAP (e.g., use
Object.entries(API_TO_USM_COLLAB_ROLE_MAP).reduce to swap key/values), validate
there are no duplicate values before inverting, and export the derived constant
in place of the hard-coded reverse map so future changes to
API_TO_USM_COLLAB_ROLE_MAP automatically propagate.
In `@src/elements/content-sharing/hooks/useSharingService.ts`:
- Around line 113-124: The sendInvitations callback currently awaits
handleSendInvitations(data) before checking contacts, so empty submissions still
invoke the invite pipeline; before calling handleSendInvitations(data) in the
sendInvitations function, add an early return when data.contacts is falsy or has
zero length (e.g., check contacts or contacts?.length) to short-circuit and
return null; keep the existing response handling code (including the response
variable and subsequent logic) only after this contacts guard so
handleSendInvitations is not called for no-recipient submissions.
In `@src/elements/content-sharing/utils/convertCollaborators.ts`:
- Around line 89-105: The dedupe fails because Set membership compares raw
collab.userId and contact.id with different types; normalize IDs to the same
type (use String(...)) when inserting into currentCollabIds and when checking
contacts. Update the block that builds currentCollabIds (add
String(collab.userId)) and change the contacts loop membership check to use
String(contact.id) (and use the normalized id when pushing into users/groups) so
numeric mock IDs match string IDs later in the pipeline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13e5bb8e-8584-4f2e-baa4-7c6a71375e33
📒 Files selected for processing (9)
src/elements/content-sharing/ContentSharing.jssrc/elements/content-sharing/ContentSharingV2.tsxsrc/elements/content-sharing/constants.jssrc/elements/content-sharing/hooks/__tests__/useSharingService.test.tssrc/elements/content-sharing/hooks/useInvites.jssrc/elements/content-sharing/hooks/useSharingService.tssrc/elements/content-sharing/utils/__tests__/convertCollaborators.test.tssrc/elements/content-sharing/utils/convertCollaborators.tssrc/elements/content-sharing/utils/convertItemResponse.ts
reneshen0328
approved these changes
Mar 26, 2026
jfox-box
reviewed
Mar 26, 2026
jfox-box
reviewed
Mar 26, 2026
Contributor
Merge Queue Status
This pull request spent 10 minutes 51 seconds in the queue, including 10 minutes 40 seconds running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes