Skip to content

feat: Allow PermissionsCollection to handle shared drives files#1675

Merged
doubleface merged 1 commit intomasterfrom
feat/shareInSharedDrive
Mar 9, 2026
Merged

feat: Allow PermissionsCollection to handle shared drives files#1675
doubleface merged 1 commit intomasterfrom
feat/shareInSharedDrive

Conversation

@doubleface
Copy link
Copy Markdown
Contributor

@doubleface doubleface commented Mar 9, 2026

Since we now want to be able to share shared drives files even when we are shared drive recipients, we add the possibility to pass a driveId option to the PermissionCollection constructor. When this option is set, all requests will be made to the /shared/{driveId}/permissions endpoint instead of the regular /permissions endpoint.

We follow the same structure as FileCollection to handle shared drive files

Since we now want to be able to share shared drives files even
when we are shared drive recipients, we add the possibility to
pass a `driveId` option to the `PermissionCollection` constructor.
When this option is set, all requests will be made to the
`/shared/{driveId}/permissions` endpoint instead of the regular
`/permissions` endpoint.
@doubleface doubleface requested a review from paultranvan as a code owner March 9, 2026 08:33
@doubleface doubleface requested a review from rezk2ll March 9, 2026 08:33
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 9, 2026

Walkthrough

This change extends the PermissionCollection class to support drive-scoped API endpoints. A new constructor parameter options with an optional driveId property has been added, enabling permissions operations to be routed through shared drive prefixes. All API methods (get, create, add, destroy, createSharingLink, fetchOwnPermissions, etc.) have been refactored to prepend the computed drive prefix to their endpoint paths. Corresponding documentation and test coverage for the drive-scoped functionality have been added.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: extending PermissionCollection to support shared drives files with a driveId option.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/shareInSharedDrive

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

@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.

🧹 Nitpick comments (1)
packages/cozy-stack-client/src/PermissionCollection.spec.js (1)

433-567: Cover the paginated shared-drive flow too.

This suite locks down the first-hop prefixed calls, but findLinksByDoctype and the fetchPermissionsByLink/revokeSharingLink chain were also changed in this PR. A single drive-scoped pagination test would catch regressions where page 2 or delete requests silently fall back to the non-prefixed route.

➕ Suggested coverage
+  describe('revokeSharingLink', () => {
+    it('keeps the shared drive prefix across pagination and deletion', async () => {
+      client.fetchJSON
+        .mockResolvedValueOnce({
+          data: [
+            {
+              type: 'io.cozy.permissions',
+              id: 'perm_id_1',
+              attributes: {
+                type: 'share',
+                permissions: {
+                  files: {
+                    type: 'io.cozy.files',
+                    verbs: ['GET'],
+                    values: ['file_1']
+                  }
+                }
+              }
+            }
+          ],
+          links: {
+            next: '/sharings/drives/abc123drive/permissions/next_page'
+          }
+        })
+        .mockResolvedValueOnce({ data: [] })
+        .mockResolvedValueOnce({})
+
+      await collection.revokeSharingLink({
+        _id: 'file_1',
+        _type: 'io.cozy.files'
+      })
+
+      expect(client.fetchJSON).toHaveBeenNthCalledWith(
+        1,
+        'GET',
+        '/sharings/drives/abc123drive/permissions/doctype/io.cozy.files/shared-by-link'
+      )
+      expect(client.fetchJSON).toHaveBeenNthCalledWith(
+        2,
+        'GET',
+        '/sharings/drives/abc123drive/permissions/next_page'
+      )
+      expect(client.fetchJSON).toHaveBeenNthCalledWith(
+        3,
+        'DELETE',
+        '/sharings/drives/abc123drive/permissions/perm_id_1'
+      )
+    })
+  })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cozy-stack-client/src/PermissionCollection.spec.js` around lines 433
- 567, Add a test that exercises the paginated shared-drive flow to ensure
subsequent page and revoke/delete requests keep the drive prefix: in the
PermissionCollection spec for the driveId case, mock client.fetchJSON to return
a first-page response from findLinksByDoctype containing a next link that points
to a prefixed second-page URL (use the collection.prefix value, e.g.
'/sharings/drives/abc123drive/...'), then assert that the second fetch call
(triggered by findLinksByDoctype or fetchPermissionsByLink) is made to that
prefixed next URL and that a revokeSharingLink/delete call uses the same
prefixed route; reference PermissionCollection, findLinksByDoctype,
fetchPermissionsByLink and revokeSharingLink to locate where to call and assert
client.fetchJSON invocations for page 2 and DELETE so regressions falling back
to the non-prefixed route are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/cozy-stack-client/src/PermissionCollection.spec.js`:
- Around line 433-567: Add a test that exercises the paginated shared-drive flow
to ensure subsequent page and revoke/delete requests keep the drive prefix: in
the PermissionCollection spec for the driveId case, mock client.fetchJSON to
return a first-page response from findLinksByDoctype containing a next link that
points to a prefixed second-page URL (use the collection.prefix value, e.g.
'/sharings/drives/abc123drive/...'), then assert that the second fetch call
(triggered by findLinksByDoctype or fetchPermissionsByLink) is made to that
prefixed next URL and that a revokeSharingLink/delete call uses the same
prefixed route; reference PermissionCollection, findLinksByDoctype,
fetchPermissionsByLink and revokeSharingLink to locate where to call and assert
client.fetchJSON invocations for page 2 and DELETE so regressions falling back
to the non-prefixed route are caught.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 406470e6-3a0d-4831-ad6b-2ff896ebe057

📥 Commits

Reviewing files that changed from the base of the PR and between 89ecca5 and 90172e2.

📒 Files selected for processing (3)
  • docs/api/cozy-stack-client.md
  • packages/cozy-stack-client/src/PermissionCollection.js
  • packages/cozy-stack-client/src/PermissionCollection.spec.js

@doubleface doubleface merged commit 26993b1 into master Mar 9, 2026
4 checks passed
@doubleface doubleface deleted the feat/shareInSharedDrive branch March 9, 2026 08:47
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.

2 participants