Skip to content
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

Migrate provider adapter to Companion #1093

Merged
merged 4 commits into from
Oct 31, 2018
Merged

Conversation

ifedapoolarewaju
Copy link
Contributor

@ifedapoolarewaju ifedapoolarewaju commented Oct 8, 2018

saves us 5KB on the frontend. Not much but it's something 🙂

Fixes #1083

@ifedapoolarewaju ifedapoolarewaju requested review from arturi and goto-bus-stop and removed request for arturi October 8, 2018 12:57
@goto-bus-stop
Copy link
Contributor

5KB is not bad at all 🎉 Excited for this! looks good on first glance, will test it in a bit 👍

thumbnail: uppy.buildURL(adapter.getItemThumbnailUrl(item), true),
requestPath: adapter.getItemRequestPath(item),
modifiedDate: adapter.getItemModifiedDate(item),
raw: item
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like raw is only used for team drive detection. Could we also do that on the server side instead and add an isTeamDrive property to GDrive items? So we don't send unnecessary data to the client.

Copy link
Contributor Author

@ifedapoolarewaju ifedapoolarewaju Oct 15, 2018

Choose a reason for hiding this comment

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

nice! I guess I was restricting myself from sending varying properties per Provider. But I think what I can do is something like this:

custom: {
    isTeamDrive: true
}

so all custom properties per provider can live there

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 That looks good to me too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated this now

@@ -0,0 +1,66 @@
exports.getUsername = (data) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we find the username somehow even if there are no files?

@@ -0,0 +1,66 @@
exports.getUsername = (data) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps in the future we can grab the username during authentication and store it in the JWT 🤔

@@ -0,0 +1,73 @@
exports.getUsername = (data) => {
return data.data[0].user.username
Copy link
Contributor

Choose a reason for hiding this comment

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

Should only do this if data.data[0] actually exists, in case there are no files

@arturi
Copy link
Contributor

arturi commented Oct 28, 2018

Tested, works for me! Happy about moving this to the server, not only for 5KB, but for consistency and simpler new integrations, like React Native.

Would be nice to add a common API response example — what a general client provider should expect to get from the server. Or maybe do this for each provider somewhere, shape of the object with all the files and folders, and their properties.

@goto-bus-stop
Copy link
Contributor

Maybe we can merge this today with the Instagram username fix (#1093 (comment)) and then do the docs later. Would be nice to get this in 0.28 Client/0.15 Companion

Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

hellyea

@arturi arturi merged commit 847ab3e into master Oct 31, 2018
@arturi arturi deleted the migrate-provider-adapter branch October 31, 2018 01:53
goto-bus-stop added a commit that referenced this pull request Mar 25, 2019
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
…-adapter

Migrate provider adapter to Companion
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
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