Skip to content

feat: Use the right endpoint when creating shared drives#1676

Merged
doubleface merged 1 commit intomasterfrom
fix/createSharedDriveEndPoint
Mar 16, 2026
Merged

feat: Use the right endpoint when creating shared drives#1676
doubleface merged 1 commit intomasterfrom
fix/createSharedDriveEndPoint

Conversation

@doubleface
Copy link
Copy Markdown
Contributor

@doubleface doubleface commented Mar 16, 2026

The POST on /sharings/drives has more checks than POST on /sharings with drive: true option

Summary by CodeRabbit

  • New Features

    • Create shared drives with recipients and read-only recipients, and add a description at creation.
  • Tests

    • Added comprehensive test coverage for shared drive creation and recipient permission handling.
  • Documentation

    • Documented the new shared drive creation method and its parameters in the API docs.

@doubleface doubleface requested a review from paultranvan as a code owner March 16, 2026 13:20
@doubleface doubleface requested review from rezk2ll and zatteo March 16, 2026 13:20
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 16, 2026

Walkthrough

A new public method createSharedDrive is added to SharingCollection to create shared drives via POST /sharings/drives. It accepts document, description, and optional recipients and readOnlyRecipients. The existing create method now delegates to createSharedDrive when the sharedDrive flag is truthy instead of embedding a drive attribute in the main payload. Tests and docs are added for the new method and delegation behavior.

Suggested reviewers

  • rezk2ll
  • paultranvan
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: refactoring to use the correct endpoint (/sharings/drives) for shared drive creation instead of the generic /sharings endpoint with a drive flag.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 fix/createSharedDriveEndPoint
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

You can get early access to new features in CodeRabbit.

Enable the early_access setting to enable early access features such as new models, tools, and more.

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.

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 `@packages/cozy-stack-client/src/SharingCollection.js`:
- Around line 201-226: The createSharedDrive method currently calls a
non-existent POST /sharings/drives endpoint; replace that flow so it (1) ensures
the shared-drives container exists by calling POST /files/shared-drives
(createSharedDrive should call
stackClient.fetchJSON('POST','/files/shared-drives') if needed), (2) create the
folder under /Drives for the drive (POST to /files with parent set to the
created shared-drives collection), and (3) create the sharing via POST /sharings
with attributes { drive: true, description } and a relationships.rules entry
that points to the folder id (use stackClient.fetchJSON('POST','/sharings', {
data: { attributes: {...}, relationships: { rules: { data: [ { type: "rule",
attributes: { folder_id: <folderId> } } ] } } } })). Update createSharedDrive to
follow this sequence and return the normalized sharing as before
(normalizeSharing(resp.data)), removing any use of '/sharings/drives'.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ae28ac22-f14f-401b-8d2b-140d2d14f794

📥 Commits

Reviewing files that changed from the base of the PR and between b1d8d07 and 0a6bbd3.

📒 Files selected for processing (2)
  • packages/cozy-stack-client/src/SharingCollection.js
  • packages/cozy-stack-client/src/SharingCollection.spec.js

The POST on `/sharings/drives` has more checks than
POST on `/sharings` with `drive: true` option
@doubleface doubleface force-pushed the fix/createSharedDriveEndPoint branch from 0a6bbd3 to 5e87997 Compare March 16, 2026 13:39
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 (2)
packages/cozy-stack-client/src/SharingCollection.spec.js (1)

516-535: Make spy cleanup failure-safe.

If an assertion throws before Line 534, the spy stays mocked. Prefer afterEach(jest.restoreAllMocks) or try/finally for cleanup.

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

In `@packages/cozy-stack-client/src/SharingCollection.spec.js` around lines 516 -
535, The test sets a spy with jest.spyOn(collection, 'createSharedDrive') but
restores it only at the end of the test, which can leak if an assertion throws;
change the cleanup to be failure-safe by either adding a global
afterEach(jest.restoreAllMocks) in this spec or wrapping the spy usage in a
try/finally where the finally calls spy.mockRestore(), ensuring the spy on
collection.createSharedDrive is always restored after the test.
packages/cozy-stack-client/src/SharingCollection.js (1)

153-160: Consider clarifying the API when sharedDrive is true.

create(...) accepts previewPath, rules, openSharing, and appSlug, but these are silently ignored when sharedDrive is true. No current call sites pass both, but documenting or explicitly rejecting unsupported fields would improve clarity for future usage.

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

In `@packages/cozy-stack-client/src/SharingCollection.js` around lines 153 - 160,
When sharedDrive is true, the create(...) wrapper currently forwards to
createSharedDrive(...) but silently ignores previewPath, rules, openSharing, and
appSlug; update create(...) to validate its args when sharedDrive is true and
explicitly reject unsupported fields: check for presence of previewPath, rules,
openSharing, or appSlug and throw a clear TypeError (or similar) describing that
those fields are not supported with sharedDrive, referencing createSharedDrive
for the supported path; this makes callers fail fast rather than silently
dropping options.
🤖 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/SharingCollection.js`:
- Around line 153-160: When sharedDrive is true, the create(...) wrapper
currently forwards to createSharedDrive(...) but silently ignores previewPath,
rules, openSharing, and appSlug; update create(...) to validate its args when
sharedDrive is true and explicitly reject unsupported fields: check for presence
of previewPath, rules, openSharing, or appSlug and throw a clear TypeError (or
similar) describing that those fields are not supported with sharedDrive,
referencing createSharedDrive for the supported path; this makes callers fail
fast rather than silently dropping options.

In `@packages/cozy-stack-client/src/SharingCollection.spec.js`:
- Around line 516-535: The test sets a spy with jest.spyOn(collection,
'createSharedDrive') but restores it only at the end of the test, which can leak
if an assertion throws; change the cleanup to be failure-safe by either adding a
global afterEach(jest.restoreAllMocks) in this spec or wrapping the spy usage in
a try/finally where the finally calls spy.mockRestore(), ensuring the spy on
collection.createSharedDrive is always restored after the test.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c960f8b9-3750-4e66-88d0-8b24c43af8a0

📥 Commits

Reviewing files that changed from the base of the PR and between 0a6bbd3 and 5e87997.

📒 Files selected for processing (3)
  • docs/api/cozy-stack-client.md
  • packages/cozy-stack-client/src/SharingCollection.js
  • packages/cozy-stack-client/src/SharingCollection.spec.js
✅ Files skipped from review due to trivial changes (1)
  • docs/api/cozy-stack-client.md

@doubleface doubleface merged commit 203d1a0 into master Mar 16, 2026
4 checks passed
@doubleface doubleface deleted the fix/createSharedDriveEndPoint branch March 16, 2026 14:52
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.

3 participants