Skip to content

fix: correct userId corruption when moving shared content to a folder (#2670)#2926

Open
cqnykamp wants to merge 1 commit into
mainfrom
claude/modest-franklin-ToRyQ
Open

fix: correct userId corruption when moving shared content to a folder (#2670)#2926
cqnykamp wants to merge 1 commit into
mainfrom
claude/modest-franklin-ToRyQ

Conversation

@cqnykamp

Copy link
Copy Markdown
Contributor

Summary

Fixes #2670 — "Moving document to another folder and then clicking Open Folder causes error"

Root Cause

In MoveCopyContent.tsx, the shareAlert computation contained a for...of loop that used the component's userId prop as the loop variable (without declaring a new const/let variable):

for (userId of sourceSharedWith) {  // ← mutates the `userId` parameter!
  count.set(userId, (count.get(userId) || 0) + 1);
}

When moving content that was explicitly shared with other users, this loop overwrote the userId parameter with the last shared-user's ID. The destination URL was then computed as:

destinationUrl = `/activities/${userId}/${activeView.parent.id}`;
//                              ^^^^^^ wrong user ID!

Navigating to /activities/{wrongUserId}/{folderId} caused the API to either return {notMe: true} (redirecting to Shared Activities) or a 404 if the folder wasn't publicly accessible — showing the error page.

A secondary bug: the ActiveView.parent.sharedWith TypeScript type was declared as string[], but getMoveCopyContentData returned the full contentShares relation (array of objects). This meant the shareAlert check count.get(user) was comparing a string map key against a whole object, always returning undefined and incorrectly triggering share alerts for any shared destination folder.

Changes

apps/app/src/popups/MoveCopyContent.tsx

  • Change for (userId of sourceSharedWith)for (const sharedUserId of sourceSharedWith) to stop corrupting the userId prop
  • Update ActiveView.parent.sharedWith type from string[] to { userId: string }[]
  • Fix shareAlert comparison to use share.userId instead of the raw object

apps/api/src/query/copy_move.ts

  • In getMoveCopyContentData, narrow sharedWith: true to sharedWith: { select: { userId: true } } so the response only includes the userId field, matching the updated frontend type

packages/e2e-tests/e2e/Content Lists/move.cy.ts

  • Add E2E test: "Move document into folder and go to destination folder" — the scenario described in the issue was not previously covered by any test

apps/app/src/popups/MoveCopyContent.cy.tsx

  • Add component test: "Go to folder uses correct userId even when content is shared with others" — directly verifies the userId prop is not corrupted by the loop when shared content is moved to a shared-with folder

Test plan

  • New E2E test passes: "Move document into folder and go to destination folder"
  • New component test passes: "Go to folder uses correct userId even when content is shared with others"
  • Existing move tests still pass (no regressions)
  • npm run format && npm run lint clean

https://claude.ai/code/session_015zUyV9xisamvNrjBnCbQC3


Generated by Claude Code

…content to a folder

The for...of loop `for (userId of sourceSharedWith)` was mutating the
`userId` function parameter instead of using a new loop variable. When
moving content that was shared with other users, this caused `userId` to
be overwritten with the last shared user's ID. The resulting destination
URL `/activities/${userId}/${folderId}` would point to the wrong user,
causing a 404 error when clicking "Go to folder" after the move.

Also fixes the related sharedWith type mismatch:
- `getMoveCopyContentData` now returns `sharedWith: {userId: string}[]`
  instead of the full contentShares relation (which includes extra fields
  not needed here)
- `ActiveView.parent.sharedWith` type updated from `string[]` to
  `{userId: string}[]` to match the actual API response
- The shareAlert comparison now correctly uses `share.userId` instead of
  comparing the raw share object against a string map key

Adds E2E test for moving a document to a folder and clicking "Go to folder"
(previously untested), and a component test verifying the destination URL
uses the correct userId when content and destination have shared users.

Closes #2670

https://claude.ai/code/session_015zUyV9xisamvNrjBnCbQC3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Moving document to another folder and then clicking Open Folder causes error

2 participants