Skip to content

Allow subfolders for Google Drive mounts - #38161

Merged
mmattel merged 1 commit into
masterfrom
mount_gd_in_subfolder
Dec 1, 2020
Merged

Allow subfolders for Google Drive mounts#38161
mmattel merged 1 commit into
masterfrom
mount_gd_in_subfolder

Conversation

@mmattel

@mmattel mmattel commented Nov 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This adds the ability to add a Google Drive mount from a GD subfolder.
Similar when you eg mount form SMB.

Related Issue

No issue related.
Based on the work of: owncloud/files_external_gdrive#9 (added support for changing root folder in google drive)

Motivation and Context

One can now mount from a GD subfolder. This has big benefits:

  • only a fraction of your GD is mounted and seen
  • it is now possibe to use the $user variable to have a mount based on a subfolder structure like:
    sub_folder/$user !
  • when using encryption, only the subfolder gets encrypted

How Has This Been Tested?

Manually having:

  • subfolder = empty (like you have it without this PR)
  • subfolder = name
  • subfolder = name/$user

In case name or name/$user does not exist in GD, the mount does not show up without error.

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

Adding a doc ticket when merged
@mmattel
mmattel force-pushed the mount_gd_in_subfolder branch from 4ca9677 to dfb25ba Compare November 29, 2020 17:46
phil-davis
phil-davis reviewed Nov 30, 2020
View reviewed changes

@phil-davis phil-davis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a typo - I will leave the technical code review to someone else.
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
@mmattel
mmattel force-pushed the mount_gd_in_subfolder branch from dfb25ba to 389ee24 Compare November 30, 2020 08:21
@mmattel

mmattel commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

@phil-davis thanks for finding the typo, this is fixed. Q: I have added a changelog, but the bot does not recognize it. Anything to do here?
@owncloud owncloud deleted a comment from update-docs Bot Nov 30, 2020
@phil-davis

phil-davis commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

@phil-davis thanks for finding the typo, this is fixed. Q: I have added a changelog, but the bot does not recognize it. Anything to do here?

The bot is too stupid to delete its comment. I deleted the changelog comment.
jvillafanez
jvillafanez reviewed Nov 30, 2020
View reviewed changes
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
@jvillafanez

jvillafanez commented Nov 30, 2020

Copy link
Copy Markdown
Member

One can now mount from a GD subfolder. This has big benefits:

  • only a fraction of your GD is mounted and seen
  • it is now possibe to use the $user variable to have a mount based on a subfolder structure like: sub_folder/$user !
  • when using encryption, only the subfolder gets encrypted

For me, the only selling point is the first one. The other 2 are side effects, which might not be bad.

Anyway, pinging @pmaier1 for him to be aware of this change.
@mmattel
mmattel force-pushed the mount_gd_in_subfolder branch from 389ee24 to 6fe043a Compare November 30, 2020 09:55
@mmattel

mmattel commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

@jvillafanez I fixed all your comments, retested and squashed the commits before pushing. Thanks for taking a look over!
@mmattel

mmattel commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

Note regarding selling points:
One driver for me picking this up was, I do not want to use the whole GD but only a fraction of it AND this fraction should be encrypted. The fraction part is used as a backup folder for data I mirror via the client so it cant get lost. This needs to be encrypted as it is on a foreign infrastructure. One having sensitive data can now use this functionality and not waste the whole GD drive for it. Hoping that this can go into 10.6.
jvillafanez
jvillafanez reviewed Nov 30, 2020
View reviewed changes

@jvillafanez jvillafanez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wrote the condition wrongly and you copied wrongly :)
The other 2 are just minor changes taking advantage that you need to change the condition
Comment thread apps/files_external/lib/Lib/Storage/Google.php
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
Comment thread apps/files_external/lib/Lib/Storage/Google.php Outdated
@jvillafanez

jvillafanez commented Nov 30, 2020

Copy link
Copy Markdown
Member

We're starting the release process so I doubt we'll add more features for 10.6 unless they're strictly needed. I'm not the one making the decision, so things might change.
@mmattel
mmattel force-pushed the mount_gd_in_subfolder branch from 6fe043a to 355aa3a Compare November 30, 2020 11:47
@mmattel
mmattel force-pushed the mount_gd_in_subfolder branch from 355aa3a to b50a35b Compare November 30, 2020 11:49
@phil-davis

phil-davis commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

Note: there is currently a problem with drone not actually starting pipelines. Hopefully will be resolved soon.
@mmattel

mmattel commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

@pmaier1 I would be very happy if you would decide that this PR goes into 10.6
@jvillafanez

jvillafanez commented Nov 30, 2020

Copy link
Copy Markdown
Member

No more code changes from my side 👍 . Waiting for CI before approving. Anyway, we still need confirmation whether this will be on 10.6 or not.
@phil-davis

phil-davis commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

I added @micbar to the reviewers so that he notices this tomorrow and also can give an opinion.
@micbar

micbar commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

I am ooto until thursday. Restarted the build.
@micbar

micbar commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

Reasonable Change IMO. But 10.6 is already leaving. There is ongoing QA on 10.6.0RC1.
@sonarqubecloud

sonarqubecloud Bot commented Nov 30, 2020

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information
@mmattel

mmattel commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

There is ongoing QA on 10.6.0RC1

Is there anything I could do to support a mind change ? 😏
(Who not asks will not get the chance for a different outcome 😄)
@mmattel

mmattel commented Dec 1, 2020

Copy link
Copy Markdown
Contributor Author

@jvillafanez

No more code changes from my side

mind to approve?
@mmattel
mmattel merged commit 26294f2 into master Dec 1, 2020
@delete-merged-branch
delete-merged-branch Bot deleted the mount_gd_in_subfolder branch December 1, 2020 11:32
This was referenced Dec 1, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - To Review

4 participants

@mmattel @phil-davis @jvillafanez @micbar