Skip to content

fix(content-sharing): USM Expiration Permission For Individual Users#4526

Merged
mergify[bot] merged 5 commits into
masterfrom
fix-usm-expiration-permission-for-individual-users
Apr 30, 2026
Merged

fix(content-sharing): USM Expiration Permission For Individual Users#4526
mergify[bot] merged 5 commits into
masterfrom
fix-usm-expiration-permission-for-individual-users

Conversation

@reneshen0328
Copy link
Copy Markdown
Contributor

@reneshen0328 reneshen0328 commented Apr 29, 2026

What

  • In ContentSharingV2.tsx, after fetching the current user, if enterprise is null we set canChangeExpiration: false on the shared link settings state. This prevents free users from seeing an enabled expiration toggle in Link Settings.
  • This matches the legacy ContentSharing behavior defined within SharingModal.js

Summary by CodeRabbit

  • New Features

    • Shared link expiration controls now respect enterprise affiliation: enterprise users can enable/change expiration; individual users see the option disabled.
  • Behavior

    • The Link Settings panel reflects this restriction immediately when opened and stays disabled for non-enterprise users across updates.
  • Tests

    • Added tests validating UI behavior for enterprise and non-enterprise users and updated test fixtures accordingly.

@reneshen0328 reneshen0328 requested review from a team as code owners April 29, 2026 23:11
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds isEnterpriseUser state in ContentSharingV2 derived from fetchCurrentUser's enterprise field, resets it when api changes, and on sharedLink updates forces sharedLink.settings.canChangeExpiration = false for non-enterprise users.

Changes

Cohort / File(s) Summary
Component Logic
src/elements/content-sharing/ContentSharingV2.tsx
Introduce isEnterpriseUser state initialized from fetchCurrentUser's enterprise; reset when api changes; on sharedLink updates, set sharedLink.settings.canChangeExpiration = false for non-enterprise users.
Tests
src/elements/content-sharing/__tests__/ContentSharingV2.test.tsx
Add fireEvent usage to open Link Settings; assert Link expiration toggle is enabled for enterprise mock and disabled when mock user has enterprise: null.
Mocks
src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js
Extend DEFAULT_USER_API_RESPONSE to include an enterprise object (id, name).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ready-to-merge

Suggested reviewers

  • tjuanitas
  • jfox-box

Poem

🐇 I fetched the user and looked for enterprise light,
If none I found, I turned expiration off at night.
A gentle hop, a tiny tweak,
Links sleep soundly, mild and meek,
A rabbit's nudge to keep settings right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: implementing USM expiration permission restrictions for individual (non-enterprise) users in the content-sharing component.
Description check ✅ Passed The description clearly explains what was changed and why, though it lacks the typical PR template structure with sections like Motivation, Testing, or other context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-usm-expiration-permission-for-individual-users

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 183-205: getUserSuccess only clamps canChangeExpiration during
initial user fetch, so later sharedLink updates can re-enable expiration;
persist the enterprise info in state (e.g., include enterprise on setCurrentUser
in getUserSuccess) and add a useEffect that watches sharedLink and the stored
enterprise and, when enterprise?.name is falsy, always call setSharedLink to set
settings.canChangeExpiration = false (safely returning prevSharedLink if
missing) so the flag is clamped on every sharedLink change as well as on initial
load.
🪄 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: 65ff497a-a6b2-4b52-be7b-a6c39db5162a

📥 Commits

Reviewing files that changed from the base of the PR and between 763dce3 and a6b9070.

📒 Files selected for processing (3)
  • src/elements/content-sharing/ContentSharingV2.tsx
  • src/elements/content-sharing/__tests__/ContentSharingV2.test.tsx
  • src/elements/content-sharing/utils/__mocks__/ContentSharingV2Mocks.js

Comment thread src/elements/content-sharing/ContentSharingV2.tsx Outdated
@reneshen0328 reneshen0328 force-pushed the fix-usm-expiration-permission-for-individual-users branch from 7fe70e7 to d60c714 Compare April 30, 2026 00:45
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/elements/content-sharing/ContentSharingV2.tsx (1)

221-237: 💤 Low value

Addresses past review concern, but consider tightening the dependency array.

This useEffect correctly clamps canChangeExpiration on every sharedLink update for non-enterprise users, addressing the earlier feedback. However, since sharedLink is in the dependency array and we mutate it inside, this will cause one extra render cycle each time sharedLink changes with canChangeExpiration: true.

The current approach works (no infinite loop due to the early return), but you could avoid the extra render by moving this logic into handleGetItemSuccess and the sharing service callbacks that update sharedLink, rather than reacting to state changes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/elements/content-sharing/ContentSharingV2.tsx` around lines 221 - 237,
The effect currently watches sharedLink and flips canChangeExpiration via
setSharedLink inside the useEffect, causing an extra render whenever sharedLink
arrives with canChangeExpiration:true; move the clamping logic out of the
useEffect and into the code paths that first set sharedLink instead (e.g. inside
handleGetItemSuccess and the sharing service callbacks that produce/update
sharedLink) so you normalize the sharedLink before calling setSharedLink (or
before dispatching the update) instead of reacting to it in useEffect; update
handleGetItemSuccess and the sharing service update handlers to ensure
sharedLink.settings.canChangeExpiration is set to false for non-enterprise users
prior to state update, and then remove sharedLink from the useEffect dependency
to avoid the redundant render.
🤖 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 193-194: The boolean naming is inverted: isEnterpriseUser is set
true when no enterprise exists; rename the state and setter to reflect actual
semantics (e.g., isIndividualUser and setIsIndividualUser) and update all usages
accordingly (replace isEnterpriseUser/setIsEnterpriseUser with
isIndividualUser/setIsIndividualUser and adjust related checks such as the
conditional that currently reads if (!isEnterpriseUser || ...) to use if
(!isIndividualUser || ...)); alternatively, if you prefer to keep the current
name, invert the assignment logic in the initialization and anywhere you set the
flag (ensure setIsEnterpriseUser(false) is called where the user is not
enterprise) so the variable meaning matches its value.

---

Nitpick comments:
In `@src/elements/content-sharing/ContentSharingV2.tsx`:
- Around line 221-237: The effect currently watches sharedLink and flips
canChangeExpiration via setSharedLink inside the useEffect, causing an extra
render whenever sharedLink arrives with canChangeExpiration:true; move the
clamping logic out of the useEffect and into the code paths that first set
sharedLink instead (e.g. inside handleGetItemSuccess and the sharing service
callbacks that produce/update sharedLink) so you normalize the sharedLink before
calling setSharedLink (or before dispatching the update) instead of reacting to
it in useEffect; update handleGetItemSuccess and the sharing service update
handlers to ensure sharedLink.settings.canChangeExpiration is set to false for
non-enterprise users prior to state update, and then remove sharedLink from the
useEffect dependency to avoid the redundant render.
🪄 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: fe698bd7-2870-456b-bcce-585d55f4ac35

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe70e7 and d60c714.

📒 Files selected for processing (1)
  • src/elements/content-sharing/ContentSharingV2.tsx

Comment thread src/elements/content-sharing/ContentSharingV2.tsx Outdated
@reneshen0328 reneshen0328 force-pushed the fix-usm-expiration-permission-for-individual-users branch 2 times, most recently from 4b5dc68 to cb6aebd Compare April 30, 2026 01:12
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 185-187: The handler getUserSuccess currently destructures
userData (const { enterprise, hostname, id } = userData) but fetchCurrentUser
can return null; update getUserSuccess to guard for a null/undefined userData
before destructuring (e.g., return or setCurrentUser(null)/fallback when
userData is falsy) and apply the same null-check protection to the similar logic
around lines 198-199; reference the getUserSuccess function and any other
callbacks that consume fetchCurrentUser results to ensure no direct
destructuring happens without a preceding truthy check.
- Around line 212-216: The updater passed to setSharedLink can assume
prevSharedLink exists but may be null if a reset (setSharedLink(null))
interleaves; update the setSharedLink(prevSharedLink => ...) callback in
ContentSharingV2 to defensively handle prevSharedLink === null by returning an
appropriate value (e.g., null or a new object) instead of accessing
prevSharedLink.settings, ensuring you guard reads of prevSharedLink.settings and
prevSharedLink before spreading or modifying fields like settings and
canChangeExpiration.
🪄 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: 8e20469d-fd57-442e-adf9-ba08581b9c10

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5dc68 and cb6aebd.

📒 Files selected for processing (1)
  • src/elements/content-sharing/ContentSharingV2.tsx

Comment thread src/elements/content-sharing/ContentSharingV2.tsx
Comment thread src/elements/content-sharing/ContentSharingV2.tsx Outdated
@reneshen0328 reneshen0328 force-pushed the fix-usm-expiration-permission-for-individual-users branch from 9e5a2eb to 8f6aa0b Compare April 30, 2026 18:29
Comment thread src/elements/content-sharing/__tests__/ContentSharingV2.test.tsx
Comment thread src/elements/content-sharing/ContentSharingV2.tsx
Comment thread src/elements/content-sharing/ContentSharingV2.tsx
Comment thread src/elements/content-sharing/ContentSharingV2.tsx
@mergify mergify Bot added the queued label Apr 30, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 30, 2026

Merge Queue Status

  • Entered queue2026-04-30 20:46 UTC · Rule: Automatic strict merge
  • Checks passed · in-place
  • Merged2026-04-30 20:56 UTC · at ddae9a0f21e7d2fc6462d1c899682b8e4215de4f · squash

This pull request spent 10 minutes 3 seconds in the queue, including 9 minutes 50 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 48b736f into master Apr 30, 2026
11 checks passed
@mergify mergify Bot deleted the fix-usm-expiration-permission-for-individual-users branch April 30, 2026 20:56
@mergify mergify Bot removed the queued label Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants