Add Shared Drives to GDriveUrl slave.
ClosedPublic

Authored by barchiesi on Sep 9 2019, 6:02 PM.

Details

Summary

Adds listing of account Shared Drives through the url 'gdrive:/account@gmail.com/Shared Drives/'.

Supported operations:

  • Create via 'Create New' -> 'Folder...'
  • Delete (if empty)
  • Rename

Discussion in T10630: Show Team Drives

Diff Detail

Repository
R219 KIO GDrive
Branch
arcpatch-D23804
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16870
Build 16888: arc lint + arc unit
barchiesi requested review of this revision.Sep 9 2019, 6:02 PM
barchiesi created this revision.
barchiesi edited the summary of this revision. (Show Details)Sep 9 2019, 6:36 PM
barchiesi added a project: KIO GDrive.
barchiesi added a subscriber: KIO GDrive.

Here are some screenshots:

Account listing Shared Drives listing

elvisangelaccio requested changes to this revision.Sep 14 2019, 4:11 PM
elvisangelaccio added inline comments.
src/gdriveurl.cpp
24

"Shared Drives" should be translated if possible.

src/kio_gdrive.cpp
364

Q_FOREACH is deprecated, please avoid it in new code.

377

Nitpick: opening brace goes to the next line

384

const

389

Missing pass-by-reference

Nitpick: opening brace goes to the next line

396

Missing pass-by-reference

Nitpick: opening brace goes to the next line

409

Redundant comment, info is already in the apidox of runJob.

413

Prefer at(0)

430–434

Can't we use runJob() here?

455

This comment should be just before the if()

522

Nitpick: else not needed after return

684

Typo: runJob

1173

"error() will have been called in case of error"

1246

runJob already called error() here, no?

src/kio_gdrive.h
95

const QString &; prefer enum instead of bool as arguments.

This revision now requires changes to proceed.Sep 14 2019, 4:11 PM
barchiesi updated this revision to Diff 66066.Sep 14 2019, 4:59 PM
barchiesi marked 12 inline comments as done.

First round of fixes.

barchiesi added inline comments.Sep 14 2019, 5:12 PM
src/kio_gdrive.cpp
430–434

The desired behavior is silently failing if the user isn't allowed to create shared drives and being that runJob() will always call error(), it isn't used.

barchiesi updated this revision to Diff 66072.Sep 14 2019, 5:35 PM
barchiesi marked 2 inline comments as done.

Remove deprecated Q_FOREACH and use enum in fetchSharedDrivesRootEntry() arguments.

src/gdriveurl.cpp
24

Sorry, this makes gdriveurl depend on ki18n (e.g. when building urltest). We should call i18n() directly in KIOGDrive::fetchSharedDrivesRootEntry() imho.

BTW: does this translation even works? Isn't "Shared Drives" part of the path provided by Google? If we translate it on our side, how can we be sure that Google "understands" our translation?

src/kio_gdrive.cpp
35–57

Please keep the list of includes sorted.

276

Missing pass-by-reference

363

const

449

Nitpick: else after closing brace on previous line

barchiesi updated this revision to Diff 66593.Sep 21 2019, 5:25 PM
barchiesi marked 5 inline comments as done.

moved translation of Shared Drive label to kio_gdrive.cpp
ordered kio_gdrive.cpp includes
const and ref fixes

barchiesi added inline comments.Sep 21 2019, 5:25 PM
src/gdriveurl.cpp
24

I moved the translating to kio_gdrive.cpp, using it only for UDS_DISPLAY_NAME. All operations on Shared Drives are done by id, this string or its path is practically only user facing.

elvisangelaccio accepted this revision.Sep 21 2019, 6:08 PM

LGTM now. Ship it! :)

This revision is now accepted and ready to land.Sep 21 2019, 6:08 PM
This revision was automatically updated to reflect the committed changes.