fix: open recipient shared-drive files from recents via the proxy#3948
Conversation
A file carrying a driveId is a proxied shared-drive file, but computeFileType only classified it as such when dir_id was the shared-drives container, which is never true for a real file nested in a drive. Those files fell through to the local /files/:id route and 404'd for the recipient. Key on driveId (minus the owner's own file-root root) and require dir_id so the route can be built.
rezk2ll
requested review from
JF-Cozy,
doubleface,
lethemanh and
zatteo
as code owners
WalkthroughThe PR fixes routing for shared-drive recipient files nested inside folders. In Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Our agent can fix these. Install it.
Gates Passed
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
BundleMonUnchanged files (20)
Total files change +1B 0% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/tests/z-shared-drive-open-file.spec.ts`:
- Around line 65-67: The network assertion in the bobPage.waitForResponse call
only validates the URL pattern but does not verify the HTTP method, allowing
non-POST requests to the same path to satisfy the wait condition and potentially
mask regressions. Update the response matcher to additionally check that the
request method is POST by examining the request object alongside the URL
pattern, ensuring both conditions are met before the wait resolves.
🪄 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: aaa9835e-4f02-4708-ac49-b863b9854179
📒 Files selected for processing (4)
e2e/tests/z-shared-drive-open-file.spec.tssrc/modules/navigation/hooks/helpers.spec.jssrc/modules/navigation/hooks/helpers.tssrc/modules/navigation/hooks/useFileLink.tsx
| bobPage.waitForResponse( | ||
| res => /\/sharings\/drives\/[^/]+\/downloads/.test(res.url()), | ||
| { timeout: 20_000 } |
There was a problem hiding this comment.
Tighten the network assertion to the expected POST download call.
The response matcher is URL-only right now, so a non-POST hit to the same path could satisfy the wait and mask a regression.
Suggested patch
- const [proxiedDownload] = await Promise.all([
- bobPage.waitForResponse(
- res => /\/sharings\/drives\/[^/]+\/downloads/.test(res.url()),
- { timeout: 20_000 }
- ),
+ const [proxiedDownload] = await Promise.all([
+ bobPage.waitForResponse(
+ res =>
+ res.request().method() === 'POST' &&
+ /\/sharings\/drives\/[^/]+\/downloads$/.test(
+ new URL(res.url()).pathname
+ ),
+ { timeout: 20_000 }
+ ),
bobDrive.row(FILE_NAME).open()
])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bobPage.waitForResponse( | |
| res => /\/sharings\/drives\/[^/]+\/downloads/.test(res.url()), | |
| { timeout: 20_000 } | |
| bobPage.waitForResponse( | |
| res => | |
| res.request().method() === 'POST' && | |
| /\/sharings\/drives\/[^/]+\/downloads$/.test( | |
| new URL(res.url()).pathname | |
| ), | |
| { timeout: 20_000 } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/tests/z-shared-drive-open-file.spec.ts` around lines 65 - 67, The network
assertion in the bobPage.waitForResponse call only validates the URL pattern but
does not verify the HTTP method, allowing non-POST requests to the same path to
satisfy the wait condition and potentially mask regressions. Update the response
matcher to additionally check that the request method is POST by examining the
request object alongside the URL pattern, ensuring both conditions are met
before the wait resolves.
3 participants
Problem
Opening a shared-drive file a recipient sees outside the drive view (Recents, Search) tried the local
/files/:idroute and 404'd, because the file lives on the owner's instance and is only reachable through the/sharings/drivesproxy.computeFileTypeonly treated a file as a shared-drive file when its parent was the shared-drives container, which never holds for a real file nested inside a drive.Solution
Classify any file carrying a
driveIdas a shared-drive file (excluding the owner's own file-root sharing root, which lives locally), so it routes to/shareddrive/:driveId/:dir/file/:id.dir_idis now required for that classification so a doc missing it falls back to the local route instead of throwing, and the folder-path prefix used by Favorites no longer mangles the already-absolute shared-drive path. Adds an e2e test covering a recipient opening a shared-drive file through the proxy.Summary by CodeRabbit
Bug Fixes
Tests