Set supportsAllDrives and includeItemsFromAllDrives on requests that allow it
ClosedPublic

Authored by barchiesi on Apr 24 2019, 2:50 PM.

Details

Summary

The supportsTeamDrives flags where not getting applied to a FileFetchJob with a specific file to fetch. This patch fixes the bug and adds the query parameter to all Jobs that allow it.

Diff Detail

Repository
R477 KGAPI Library
Branch
teamdrive_include_fix2
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11411
Build 11429: arc lint + arc unit
barchiesi created this revision.Apr 24 2019, 2:50 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 24 2019, 2:50 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
barchiesi requested review of this revision.Apr 24 2019, 2:50 PM
dvratil requested changes to this revision.Apr 28 2019, 3:16 PM

According to https://developers.google.com/drive/api/v3/reference/files/get the supportsTeamDrives parameter is deprecated and should not be used. We should be setting supportsAllDrives and includeItemsFromAllDrives and we should be setting them to true unconditionally since we now support Team Drives - in 2020 GDrive will start ignoring those parameters and will always list contents from Team drives, too, so effectively setting them to false in KGAPI would look like a bug.

This revision now requires changes to proceed.Apr 28 2019, 3:16 PM
barchiesi updated this revision to Diff 57143.Apr 28 2019, 6:40 PM
barchiesi retitled this revision from Fix includeTeamDriveItems not being applied to single File fetch to FileFetchJob: unconditionally set `includeTeamDriveItems`, `includeItemsFromAllDrives`, `supportsTeamDrives`, `supportsAllDrives` query params.
barchiesi edited the summary of this revision. (Show Details)

It seems like Google updated the v2 and v3 Drive references. The Teamdrive part of the API that I recently added is now considered deprecated and Drives should be used instead. Sintactically they are very similar, if not same apart from the added features.

The deprecation notice you are referring to was also just recently added, while the default value if unspecified remains false (?!). So we need to unconditionally set all 4 to true (includeTeamDriveItems, includeItemsFromAllDrives, supportsTeamDrives, supportsAllDrives), at least until June 1, 2020 when I presume the default will be changed to true.

dvratil requested changes to this revision.Apr 28 2019, 8:59 PM

supportsTeamDrives documentation says "Use supportsAllDrives instead, for includeTeamDriveItems to use includeItemsFromAllDrives, so I think we don't have to set all four of them, only the "allDrives" versions, they all have the same effect. In this case 1:1 mapping with the REST API is not necessary, IMO

src/drive/filefetchjob.h
137

Deprecate all the getters as well, please.

166

Let's not introduce getters/setters that only alias other functions.

This revision now requires changes to proceed.Apr 28 2019, 8:59 PM
barchiesi updated this revision to Diff 57148.Apr 28 2019, 9:19 PM
dvratil accepted this revision.Apr 28 2019, 9:48 PM

Thanks!

This revision is now accepted and ready to land.Apr 28 2019, 9:48 PM
barchiesi retitled this revision from FileFetchJob: unconditionally set `includeTeamDriveItems`, `includeItemsFromAllDrives`, `supportsTeamDrives`, `supportsAllDrives` query params to FileFetchJob: unconditionally set `includeItemsFromAllDrives`, `supportsAllDrives` query params.Apr 29 2019, 8:27 AM
barchiesi updated this revision to Diff 57325.May 1 2019, 4:49 PM

Added the supportsAllDrives query param to other Jobs that can use it.

barchiesi retitled this revision from FileFetchJob: unconditionally set `includeItemsFromAllDrives`, `supportsAllDrives` query params to Set supportsAllDrives and includeItemsFromAllDrives on requests that allow it.May 1 2019, 4:51 PM
barchiesi edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.