Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make unsynced shares accessible #10887

Merged
merged 3 commits into from
May 21, 2024
Merged

Conversation

JammingBen
Copy link
Collaborator

@JammingBen JammingBen commented May 8, 2024

Description

Makes unsynced shares accessible. The way to make this work is by using the remote item id instead of the share jail id for opening/resolving into incoming shares.

Notable changes in the course of this:

  • Renames shareId to remoteItemId and shareRoot to remoteItemPath on resources to match with the Graph terminology.
  • Removes the remoteItemId on space resources because it only matters for share spaces, in which case this is simply the space id. That means instead of blindly going for space.remoteItemId, it now checks if the space is actually a share space and then uses its id.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@JammingBen JammingBen self-assigned this May 8, 2024
Copy link

update-docs bot commented May 8, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@dschmidt
Copy link
Member

dschmidt commented May 8, 2024

Todo: handle legacy urls

@JammingBen
Copy link
Collaborator Author

JammingBen commented May 8, 2024

Todo: handle legacy urls

Discussed with @tbsbdr , it's okay for the URLs to break because of the shareId.

Edit: We decided to built a fallback anyway since it was easy.

Makes unsynced shares accessible. The way to make this work is by using the remote item id instead of the share jail id for opening/resolving into incoming shares.

In the future we should rethink the `shareId` property since the name is not really fitting anymore. `remoteItemId` would be more fitting.
Renames `shareId` to `remoteItemId` and `shareRoot` to `remoteItemPath` on resources to match with the Graph terminology.

Also removes the `remoteItemId` on space resources because it only matters for share spaces, in which case this is simply the space id. That means instead of blindly going for `space.remoteItemId`, it now checks if the space is actually a share space and then uses its id.
Copy link

sonarcloud bot commented May 8, 2024

@JammingBen JammingBen marked this pull request as ready for review May 16, 2024 08:10
Copy link
Contributor

@AlexAndBear AlexAndBear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
we should add E2E tests for this

@JammingBen
Copy link
Collaborator Author

LGTM, we should add E2E tests for this

Agreed, I created #10931 to keep track.

@JammingBen JammingBen merged commit 29f02a1 into master May 21, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the access-unsynced-shares branch May 21, 2024 07:14
@dschmidt
Copy link
Member

Congrats to us! 🎉

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.

[web] Open shares without having to accept it
3 participants