fix(cozy-sharing): Allow sharing shared drive folder root by email#3023
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough
Possibly related PRs
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 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.jsx (1)
38-44:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate
getSharingByIddeclaration will cause a parse error.
getSharingByIdis destructured twice at lines 38 and 40, which is invalid JavaScript syntax and will fail to compile.Proposed fix: remove the duplicate declaration
const { share, getSharingLink, getFederatedShareLink, getDocumentPermissions, getOwner, getSharingById, getRecipients, - getSharingById, hasSharedChild, hasSharedParent, revoke } = useSharingContext()🤖 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 `@packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.jsx` around lines 38 - 44, The destructuring from useSharingContext() in FederatedFolderModal.jsx mistakenly includes getSharingById twice which causes a parse error; remove the duplicate getSharingById from the destructuring list (keep a single getSharingById alongside getRecipients, hasSharedChild, hasSharedParent, revoke) so the destructure expression in the useSharingContext() assignment is valid.Source: Linters/SAST tools
packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.spec.jsx (1)
22-33:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
getSharingByIdis not wired into theuseSharingContextmock.
mockGetSharingByIdis defined at line 16 and initialized inbeforeEach, but it's missing from the mock return value. The new test at lines 168-192 will fail because the component will receiveundefinedforgetSharingById.Proposed fix: add getSharingById to the mock
jest.mock('../../hooks/useSharingContext', () => ({ useSharingContext: () => ({ share: mockShare, revoke: mockRevoke, getSharingLink: mockGetSharingLink, getFederatedShareLink: mockGetFederatedShareLink, getDocumentPermissions: jest.fn().mockReturnValue([]), + getOwner: jest.fn(), + getSharingById: mockGetSharingById, getRecipients: mockGetRecipients, hasSharedChild: mockHasSharedChild, hasSharedParent: mockHasSharedParent }) }))🤖 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 `@packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.spec.jsx` around lines 22 - 33, The useSharingContext mock is missing getSharingById so the component receives undefined; update the jest.mock return to include getSharingById: mockGetSharingById (the mock defined at line 16 and initialized in beforeEach) so components calling getSharingById get the mocked implementation; modify the object returned by useSharingContext (in the mocked module) to add a getSharingById property pointing to mockGetSharingById.
🤖 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.
Outside diff comments:
In
`@packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.jsx`:
- Around line 38-44: The destructuring from useSharingContext() in
FederatedFolderModal.jsx mistakenly includes getSharingById twice which causes a
parse error; remove the duplicate getSharingById from the destructuring list
(keep a single getSharingById alongside getRecipients, hasSharedChild,
hasSharedParent, revoke) so the destructure expression in the
useSharingContext() assignment is valid.
In
`@packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.spec.jsx`:
- Around line 22-33: The useSharingContext mock is missing getSharingById so the
component receives undefined; update the jest.mock return to include
getSharingById: mockGetSharingById (the mock defined at line 16 and initialized
in beforeEach) so components calling getSharingById get the mocked
implementation; modify the object returned by useSharingContext (in the mocked
module) to add a getSharingById property pointing to mockGetSharingById.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27c6d68f-1482-43f7-a8d4-bd822cbcbb87
📒 Files selected for processing (2)
packages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.jsxpackages/cozy-sharing/src/components/FederatedFolder/FederatedFolderModal.spec.jsx
doubleface
force-pushed
the
fix/hideMailSharingRootSharedDrive
branch
from
40a56f0 to
6ee73e6
Compare
doubleface
force-pushed
the
fix/hideMailSharingRootSharedDrive
branch
from
6ee73e6 to
dcb4f98
Compare
| ) | ||
| const isInsideSharedDrive = Boolean( | ||
| existingDocument?.driveId && !isSharedDriveRoot | ||
| ) |
There was a problem hiding this comment.
This is only for recipient right? For the owner it will be catched by hasSharedParentByPath?
This resulted from a merge of #3023 without rebase
- Bump `cozy-sharing` from `33.1.1` to `33.2.2` - [linagora/cozy-libs#2994](linagora/cozy-libs#2994) (`fix(cozy-sharing): Make matching diacritic-insensitive`) - [linagora/cozy-libs#3021](linagora/cozy-libs#3021) (`feat(cozy-sharing): Show progress icon on done button during share`) - [linagora/cozy-libs#3022](linagora/cozy-libs#3022) (`fix(cozy-sharing): Refresh link state in share modal`) - [linagora/cozy-libs#3023](linagora/cozy-libs#3023) (`fix(cozy-sharing): Allow sharing shared drive folder root by email`)
3 participants
Get the associated sharing to know if a folder is the root of the sharing or not
This was a regression from #3017
https://app.notion.com/p/linagora/5d2f7429646042378356ecb072890d35?v=1c062718bad180d3847f000cd4c97139&p=37a62718bad180ab88bbf091063ce599&pm=s