Skip to content

Conversation

@stephentuso
Copy link
Contributor

See #1611 - this is only the backend changes, the frontend will need an update for the issue to be completely resolved

Was the idea for all the adapter.js files to have the same interface? Drive/dropbox required different parameters, I updated the instagram getNextPagePath to match.

Team drive support appeared to be broken, so I made a few changes to get it working. Also, the current team drive API being used is deprecated and I think its usage can be simplified some, I didn't want too make to many changes re that in this PR though

@arturi arturi requested a review from ifedapoolarewaju June 13, 2019 18:03
@arturi
Copy link
Contributor

arturi commented Jun 13, 2019

Hi, thanks for the PR! We’ll try to review as soon as possible.

Copy link
Contributor

@ifedapoolarewaju ifedapoolarewaju left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! I really appreciate the work, and the cleanup done for GoogleDrive. I left some comments here and there. Please let me know what you think.


exports.getItemRequestPath = (item) => {
// If it's from a Team Drive, add the Team Drive ID as a query param.
// If it's from/is a Team Drive, add the Team Drive ID as a query param.
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a typo?

// If it's from a Team Drive, add the Team Drive ID as a query param.
// If it's from/is a Team Drive, add the Team Drive ID as a query param.
// The server needs the Team Drive ID to list files in a Team Drive folder.
if (exports.isTeamDrive(item)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean that we can delete the if block that is right under this one?


let where = {
fields: DRIVE_FILES_FIELDS,
pageToken: query.nextPageToken,
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should make the nextPage query name the same for all providers? so each provider would use it as it requires internally. But between companion and uppy client it will be the same query name. For instagram we are already using max_id but we can change it and still support max_id for backward compatibility.

@stephentuso
Copy link
Contributor Author

@ifedapoolarewaju The team drive thing wasn't a typo but it was a bit redundant - I did some more testing and cleaned that up, the teamDriveId parameter isn't actually needed.

Also updated all providers to use cursor as the parameter

@ifedapoolarewaju ifedapoolarewaju merged commit 324ba3d into transloadit:master Jun 18, 2019
@ifedapoolarewaju
Copy link
Contributor

Thank you for the work! 💪
created a task to implement the FE changes here, but PRs are still welcome by all means 😬

@stephentuso
Copy link
Contributor Author

@ifedapoolarewaju No problem - has any progress been made on that task? If not I could get to it this week

@ifedapoolarewaju
Copy link
Contributor

@stephentuso no work has begun on this yet

@ifedapoolarewaju
Copy link
Contributor

@stephentuso FYI, I realized that the current frontend implementation should already work with the API changes you made in this PR, so there should be no need to add/change anything on the frontend

HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
…gination

companion: return nextPagePath for drive/dropbox
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.

3 participants