-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Implements the OneDrive Sync Service #14648
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mereghost
force-pushed
the
impl/sharepoint-sync-service
branch
4 times, most recently
from
January 30, 2024 11:37
d9c9ca6
to
e2a30b8
Compare
- Adds VCR options for re-recording and filtering sensitive data - Adds error handling to Util.using_admin_token - Improves logging of multiple commands
mereghost
force-pushed
the
impl/sharepoint-sync-service
branch
from
January 30, 2024 17:47
4088380
to
03d5664
Compare
mereghost
changed the title
[WIP] Implements the OneDrive Sync Service
Implements the OneDrive Sync Service
Jan 30, 2024
wielinde
reviewed
Jan 31, 2024
modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
Outdated
Show resolved
Hide resolved
Kharonus
requested changes
Jan 31, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting it to "request changes", even with me not seeing a big blocker.
but there are a few points, I'd at least like to discuss first.
...rages/app/common/storages/peripherals/storage_interaction/one_drive/create_folder_command.rb
Outdated
Show resolved
Hide resolved
modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
Outdated
Show resolved
Hide resolved
modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
Show resolved
Hide resolved
modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
Show resolved
Hide resolved
modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
Show resolved
Hide resolved
Kharonus
approved these changes
Feb 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR introduces the
OneDriveSyncService
as required by the OP#52394. The objective here being creating a similar service to the one used by Nextcloud -NextcloudGroupFolderSyncService
, formerly known asGroupFolderPropertiesSyncService
.What it should do?
Why?
This is one of the final pieces needed for the Automatically Managed Project Folders on OneDrive/SharePoint and intends to ensure feature parity between SharePoint and Nextcloud.
How?
I will go through the order of operations here:
Early exit if, somehow, the storage passed has
automatically_managed
set tofalse
Get all existing folders, their names and IDs: this information is parsed and just the ID/name tuples are returned. If an error occurs here, we need to terminate in an error state as, probably, the credentials are broken or the storage mis-configured.
Following this we ensure that the folders do exist by:
And then we hide the inactive project folders. Hiding a folder just means removing everyone's access from it.
None of these operations warrant an early exit and will be logged by the Rails Logger in case some error happens using the structure:
From there we can move to setting permissions. On
Sharepoint/OneDrive
there's only 2 levels of access we can grant:read
andwrite
. So, when mapping toOpenProject
permission system, I only grantread
permissions if the only file related permission given isread_file
andwrite
if the user has access towrite_files
, this implies theshare_files, delete_files
are also given.When applying the new permissions we ensure that the OpenProject Admin has full write access to the folder and we loop through all the users checking their permissions and creating the hash argument for the
SetPermissionsCommand
. Any errors here will also be logged by the Rails Logger.