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

Google photos #5061

Merged
merged 34 commits into from
Jun 18, 2024
Merged

Google photos #5061

merged 34 commits into from
Jun 18, 2024

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Apr 8, 2024

closes #2163

remaining tasks:

  • allow using the same oauth provider (google) for drive and photos, must handle callback differently than currently
  • should we share auth token between google drive and google photos? store accessToken/refreshToken outside providerUserSession, keyed on authProvider?
    • agreed to not share the auth session for now as the easiest implementation.
  • must dynamically set scope based on user requested provider
  • fix tests
  • improve UI (support tiles/grid a.la instagram for album list)
    • support tiles/grid a.la instagram for each album's photo list
    • support tiles/grid a.la instagram for album list - turns out to be harder because we don't seem to have any UI that supports grid tiles for folders/albums.
  • test expired access token / refresh as well as other error codes
  • document at Uppy.io document google photos uppy.io#223
  • deploy to Transloadit and enable Google Photos API
  • show more metadata about photos in the UI? currently the photo name is the file name (XYZ.JPG) which is not very relevant. avilable info is things like: description, date/time, width/height, camera hardware info
  • get more info about images? (batch) https://developers.google.com/photos/library/guides/access-media-items#get-multiple-media-item

Partner program

Potential problems

links:

in companion
- make one authProvider for each of drive/googlephotos, because reusing would have been a big rewrite
- allow the provider.download method to optionally return a size as an optimization
Copy link
Contributor

github-actions bot commented Apr 8, 2024

Diff output files
diff --git a/packages/@uppy/box/lib/Box.js b/packages/@uppy/box/lib/Box.js
index e45f972..1122bd4 100644
--- a/packages/@uppy/box/lib/Box.js
+++ b/packages/@uppy/box/lib/Box.js
@@ -58,6 +58,7 @@ export default class Box extends UIPlugin {
     this.view = new ProviderViews(this, {
       provider: this.provider,
       loadAllFiles: true,
+      virtualList: true,
     });
     const {
       target,
diff --git a/packages/@uppy/companion/lib/config/grant.d.ts b/packages/@uppy/companion/lib/config/grant.d.ts
index f7df73c..97c46b1 100644
--- a/packages/@uppy/companion/lib/config/grant.d.ts
+++ b/packages/@uppy/companion/lib/config/grant.d.ts
@@ -1,12 +1,29 @@
 declare function _exports(): {
-    google: {
-        transport: string;
+    googledrive: {
+        callback: string;
         scope: string[];
+        transport: string;
+        custom_params: {
+            access_type: string;
+            prompt: string;
+        };
+        authorize_url: string;
+        access_url: string;
+        oauth: number;
+        scope_delimiter: string;
+    };
+    googlephotos: {
         callback: string;
+        scope: string[];
+        transport: string;
         custom_params: {
             access_type: string;
             prompt: string;
         };
+        authorize_url: string;
+        access_url: string;
+        oauth: number;
+        scope_delimiter: string;
     };
     dropbox: {
         transport: string;
diff --git a/packages/@uppy/companion/lib/config/grant.js b/packages/@uppy/companion/lib/config/grant.js
index 861adf7..6f113e3 100644
--- a/packages/@uppy/companion/lib/config/grant.js
+++ b/packages/@uppy/companion/lib/config/grant.js
@@ -1,21 +1,36 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
+const google = {
+  transport: "session",
+  // access_type: offline is needed in order to get refresh tokens.
+  // prompt: 'consent' is needed because sometimes a user will get stuck in an authenticated state where we will
+  // receive no refresh tokens from them. This seems to be happen when running on different subdomains.
+  // therefore to be safe that we always get refresh tokens, we set this.
+  // https://stackoverflow.com/questions/10827920/not-receiving-google-oauth-refresh-token/65108513#65108513
+  custom_params: { access_type: "offline", prompt: "consent" },
+  // copied from https://github.com/simov/grant/blob/master/config/oauth.json
+  "authorize_url": "https://accounts.google.com/o/oauth2/v2/auth",
+  "access_url": "https://oauth2.googleapis.com/token",
+  "oauth": 2,
+  "scope_delimiter": " ",
+};
 // oauth configuration for provider services that are used.
 module.exports = () => {
   return {
-    // for drive
-    google: {
-      transport: "session",
-      scope: [
-        "https://www.googleapis.com/auth/drive.readonly",
-      ],
+    // we need separate auth providers because scopes are different,
+    // and because it would be a too big rewrite to allow reuse of the same provider.
+    googledrive: {
+      ...google,
       callback: "/drive/callback",
-      // access_type: offline is needed in order to get refresh tokens.
-      // prompt: 'consent' is needed because sometimes a user will get stuck in an authenticated state where we will
-      // receive no refresh tokens from them. This seems to be happen when running on different subdomains.
-      // therefore to be safe that we always get refresh tokens, we set this.
-      // https://stackoverflow.com/questions/10827920/not-receiving-google-oauth-refresh-token/65108513#65108513
-      custom_params: { access_type: "offline", prompt: "consent" },
+      scope: ["https://www.googleapis.com/auth/drive.readonly"],
+    },
+    googlephotos: {
+      ...google,
+      callback: "/googlephotos/callback",
+      scope: [
+        "https://www.googleapis.com/auth/photoslibrary.readonly",
+        "https://www.googleapis.com/auth/userinfo.email",
+      ], // if name is needed, then add https://www.googleapis.com/auth/userinfo.profile too
     },
     dropbox: {
       transport: "session",
diff --git a/packages/@uppy/companion/lib/server/controllers/get.js b/packages/@uppy/companion/lib/server/controllers/get.js
index c3b5129..44c2d07 100644
--- a/packages/@uppy/companion/lib/server/controllers/get.js
+++ b/packages/@uppy/companion/lib/server/controllers/get.js
@@ -10,10 +10,7 @@ async function get(req, res) {
   async function getSize() {
     return provider.size({ id, token: accessToken, query: req.query });
   }
-  async function download() {
-    const { stream } = await provider.download({ id, token: accessToken, providerUserSession, query: req.query });
-    return stream;
-  }
+  const download = () => provider.download({ id, token: accessToken, providerUserSession, query: req.query });
   try {
     await startDownUpload({ req, res, getSize, download });
   } catch (err) {
diff --git a/packages/@uppy/companion/lib/server/controllers/url.js b/packages/@uppy/companion/lib/server/controllers/url.js
index 4ab5695..9c36bd4 100644
--- a/packages/@uppy/companion/lib/server/controllers/url.js
+++ b/packages/@uppy/companion/lib/server/controllers/url.js
@@ -26,8 +26,8 @@ const downloadURL = async (url, blockLocalIPs, traceId) => {
   try {
     const protectedGot = getProtectedGot({ blockLocalIPs });
     const stream = protectedGot.stream.get(url, { responseType: "json" });
-    await prepareStream(stream);
-    return stream;
+    const { size } = await prepareStream(stream);
+    return { stream, size };
   } catch (err) {
     logger.error(err, "controller.url.download.error", traceId);
     throw err;
@@ -73,9 +73,7 @@ const get = async (req, res) => {
     const { size } = await getURLMeta(req.body.url, !allowLocalUrls);
     return size;
   }
-  async function download() {
-    return downloadURL(req.body.url, !allowLocalUrls, req.id);
-  }
+  const download = () => downloadURL(req.body.url, !allowLocalUrls, req.id);
   try {
     await startDownUpload({ req, res, getSize, download });
   } catch (err) {
diff --git a/packages/@uppy/companion/lib/server/helpers/oauth-state.d.ts b/packages/@uppy/companion/lib/server/helpers/oauth-state.d.ts
index f1c2709..f2b204d 100644
--- a/packages/@uppy/companion/lib/server/helpers/oauth-state.d.ts
+++ b/packages/@uppy/companion/lib/server/helpers/oauth-state.d.ts
@@ -1,4 +1,5 @@
 export function encodeState(state: any, secret: any): string;
+export function decodeState(state: any, secret: any): any;
 export function generateState(): {
     id: string;
 };
diff --git a/packages/@uppy/companion/lib/server/helpers/oauth-state.js b/packages/@uppy/companion/lib/server/helpers/oauth-state.js
index ccdc572..9421cec 100644
--- a/packages/@uppy/companion/lib/server/helpers/oauth-state.js
+++ b/packages/@uppy/companion/lib/server/helpers/oauth-state.js
@@ -7,7 +7,7 @@ module.exports.encodeState = (state, secret) => {
   const encodedState = Buffer.from(JSON.stringify(state)).toString("base64");
   return encrypt(encodedState, secret);
 };
-const decodeState = (state, secret) => {
+module.exports.decodeState = (state, secret) => {
   const encodedState = decrypt(state, secret);
   return JSON.parse(atob(encodedState));
 };
@@ -17,7 +17,7 @@ module.exports.generateState = () => {
   };
 };
 module.exports.getFromState = (state, name, secret) => {
-  return decodeState(state, secret)[name];
+  return module.exports.decodeState(state, secret)[name];
 };
 module.exports.getGrantDynamicFromRequest = (req) => {
   var _a, _b;
diff --git a/packages/@uppy/companion/lib/server/helpers/upload.js b/packages/@uppy/companion/lib/server/helpers/upload.js
index 923af2a..948e8b4 100644
--- a/packages/@uppy/companion/lib/server/helpers/upload.js
+++ b/packages/@uppy/companion/lib/server/helpers/upload.js
@@ -5,12 +5,20 @@ const logger = require("../logger");
 const { respondWithError } = require("../provider/error");
 async function startDownUpload({ req, res, getSize, download }) {
   try {
-    const size = await getSize();
+    logger.debug("Starting download stream.", null, req.id);
+    const { stream, size: maybeSize } = await download();
+    let size;
+    // if the provider already knows the size, we can use that
+    if (typeof maybeSize === "number" && !Number.isNaN(maybeSize) && maybeSize > 0) {
+      size = maybeSize;
+    }
+    // if not we need to get the size
+    if (size == null) {
+      size = await getSize();
+    }
     const { clientSocketConnectTimeout } = req.companion.options;
     logger.debug("Instantiating uploader.", null, req.id);
     const uploader = new Uploader(Uploader.reqToOptions(req, size));
-    logger.debug("Starting download stream.", null, req.id);
-    const stream = await download();
     (async () => {
       // wait till the client has connected to the socket, before starting
       // the download, so that the client can receive all download/upload progress.
diff --git a/packages/@uppy/companion/lib/server/helpers/utils.js b/packages/@uppy/companion/lib/server/helpers/utils.js
index 5b2a4a7..b7b58e4 100644
--- a/packages/@uppy/companion/lib/server/helpers/utils.js
+++ b/packages/@uppy/companion/lib/server/helpers/utils.js
@@ -140,11 +140,14 @@ module.exports.StreamHttpJsonError = StreamHttpJsonError;
 module.exports.prepareStream = async (stream) =>
   new Promise((resolve, reject) => {
     stream
-      .on("response", () => {
+      .on("response", (response) => {
+        const contentLengthStr = response.headers["content-length"];
+        const contentLength = parseInt(contentLengthStr, 10);
+        const size = !Number.isNaN(contentLength) && contentLength >= 0 ? contentLength : undefined;
         // Don't allow any more data to flow yet.
         // https://github.com/request/request/issues/1990#issuecomment-184712275
         stream.pause();
-        resolve();
+        resolve({ size });
       })
       .on("error", (err) => {
         var _a, _b;
diff --git a/packages/@uppy/companion/lib/server/provider/index.js b/packages/@uppy/companion/lib/server/provider/index.js
index de030b5..3c7caea 100644
--- a/packages/@uppy/companion/lib/server/provider/index.js
+++ b/packages/@uppy/companion/lib/server/provider/index.js
@@ -5,7 +5,8 @@ Object.defineProperty(exports, "__esModule", { value: true });
  */
 const dropbox = require("./dropbox");
 const box = require("./box");
-const drive = require("./drive");
+const drive = require("./google/drive");
+const googlephotos = require("./google/googlephotos");
 const instagram = require("./instagram/graph");
 const facebook = require("./facebook");
 const onedrive = require("./onedrive");
@@ -63,7 +64,7 @@ module.exports.getProviderMiddleware = (providers, grantConfig) => {
  * @returns {Record<string, typeof Provider>}
  */
 module.exports.getDefaultProviders = () => {
-  const providers = { dropbox, box, drive, facebook, onedrive, zoom, instagram, unsplash };
+  const providers = { dropbox, box, drive, googlephotos, facebook, onedrive, zoom, instagram, unsplash };
   return providers;
 };
 /**
diff --git a/packages/@uppy/companion/lib/server/provider/providerErrors.d.ts b/packages/@uppy/companion/lib/server/provider/providerErrors.d.ts
index c8b3753..9e37f86 100644
--- a/packages/@uppy/companion/lib/server/provider/providerErrors.d.ts
+++ b/packages/@uppy/companion/lib/server/provider/providerErrors.d.ts
@@ -24,3 +24,4 @@ export function withProviderErrorHandling({ fn, tag, providerName, isAuthError,
     }) => boolean;
     getJsonErrorMessage: (a: object) => string;
 }): Promise<any>;
+export function withGoogleErrorHandling(providerName: any, tag: any, fn: any): Promise<any>;
diff --git a/packages/@uppy/companion/lib/server/provider/providerErrors.js b/packages/@uppy/companion/lib/server/provider/providerErrors.js
index 1595560..9196c01 100644
--- a/packages/@uppy/companion/lib/server/provider/providerErrors.js
+++ b/packages/@uppy/companion/lib/server/provider/providerErrors.js
@@ -57,4 +57,25 @@ async function withProviderErrorHandling(
     throw err;
   }
 }
-module.exports = { withProviderErrorHandling };
+async function withGoogleErrorHandling(providerName, tag, fn) {
+  return withProviderErrorHandling({
+    fn,
+    tag,
+    providerName,
+    isAuthError: (response) => {
+      var _a;
+      return (
+        response.statusCode === 401
+        || (response.statusCode === 400
+          && ((_a = response.body) === null || _a === void 0 ? void 0 : _a.error) === "invalid_grant") // Refresh token has expired or been revoked
+      );
+    },
+    getJsonErrorMessage: (body) => {
+      var _a;
+      return (_a = body === null || body === void 0 ? void 0 : body.error) === null || _a === void 0
+        ? void 0
+        : _a.message;
+    },
+  });
+}
+module.exports = { withProviderErrorHandling, withGoogleErrorHandling };
diff --git a/packages/@uppy/companion/lib/standalone/helper.js b/packages/@uppy/companion/lib/standalone/helper.js
index 9c21b17..6a83a86 100644
--- a/packages/@uppy/companion/lib/standalone/helper.js
+++ b/packages/@uppy/companion/lib/standalone/helper.js
@@ -72,6 +72,11 @@ const getConfigFromEnv = () => {
         secret: getSecret("COMPANION_GOOGLE_SECRET"),
         credentialsURL: process.env.COMPANION_GOOGLE_KEYS_ENDPOINT,
       },
+      googlephotos: {
+        key: process.env.COMPANION_GOOGLE_KEY,
+        secret: getSecret("COMPANION_GOOGLE_SECRET"),
+        credentialsURL: process.env.COMPANION_GOOGLE_KEYS_ENDPOINT,
+      },
       dropbox: {
         key: process.env.COMPANION_DROPBOX_KEY,
         secret: getSecret("COMPANION_DROPBOX_SECRET"),
diff --git a/packages/@uppy/core/lib/locale.js b/packages/@uppy/core/lib/locale.js
index cdb4f7c..7311693 100644
--- a/packages/@uppy/core/lib/locale.js
+++ b/packages/@uppy/core/lib/locale.js
@@ -55,5 +55,6 @@ export default {
       1: "Added %{smart_count} files from %{folder}",
     },
     additionalRestrictionsFailed: "%{count} additional restrictions were not fulfilled",
+    unnamed: "Unnamed",
   },
 };
diff --git a/packages/@uppy/dashboard/lib/utils/copyToClipboard.js b/packages/@uppy/dashboard/lib/utils/copyToClipboard.js
index ed94e73..05bf18e 100644
--- a/packages/@uppy/dashboard/lib/utils/copyToClipboard.js
+++ b/packages/@uppy/dashboard/lib/utils/copyToClipboard.js
@@ -19,7 +19,7 @@ export default function copyToClipboard(textToCopy, fallbackString) {
     textArea.value = textToCopy;
     document.body.appendChild(textArea);
     textArea.select();
-    const magicCopyFailed = cause => {
+    const magicCopyFailed = () => {
       document.body.removeChild(textArea);
       window.prompt(fallbackString, textToCopy);
       resolve();
@@ -27,13 +27,13 @@ export default function copyToClipboard(textToCopy, fallbackString) {
     try {
       const successful = document.execCommand("copy");
       if (!successful) {
-        return magicCopyFailed("copy command unavailable");
+        return magicCopyFailed();
       }
       document.body.removeChild(textArea);
       return resolve();
     } catch (err) {
       document.body.removeChild(textArea);
-      return magicCopyFailed(err);
+      return magicCopyFailed();
     }
   });
 }
diff --git a/packages/@uppy/dropbox/lib/Dropbox.js b/packages/@uppy/dropbox/lib/Dropbox.js
index 3a5bae8..76d0e03 100644
--- a/packages/@uppy/dropbox/lib/Dropbox.js
+++ b/packages/@uppy/dropbox/lib/Dropbox.js
@@ -50,6 +50,7 @@ export default class Dropbox extends UIPlugin {
     this.view = new ProviderViews(this, {
       provider: this.provider,
       loadAllFiles: true,
+      virtualList: true,
     });
     const {
       target,
diff --git a/packages/@uppy/google-drive/lib/GoogleDrive.js b/packages/@uppy/google-drive/lib/GoogleDrive.js
index 1269a20..14c8f7e 100644
--- a/packages/@uppy/google-drive/lib/GoogleDrive.js
+++ b/packages/@uppy/google-drive/lib/GoogleDrive.js
@@ -75,6 +75,7 @@ export default class GoogleDrive extends UIPlugin {
     this.view = new DriveProviderViews(this, {
       provider: this.provider,
       loadAllFiles: true,
+      virtualList: true,
     });
     const {
       target,
diff --git a/packages/@uppy/onedrive/lib/OneDrive.js b/packages/@uppy/onedrive/lib/OneDrive.js
index edec3e4..86c3896 100644
--- a/packages/@uppy/onedrive/lib/OneDrive.js
+++ b/packages/@uppy/onedrive/lib/OneDrive.js
@@ -67,6 +67,7 @@ export default class OneDrive extends UIPlugin {
     this.view = new ProviderViews(this, {
       provider: this.provider,
       loadAllFiles: true,
+      virtualList: true,
     });
     const {
       target,
diff --git a/packages/@uppy/provider-views/lib/Browser.js b/packages/@uppy/provider-views/lib/Browser.js
index 60ad2f8..06876c0 100644
--- a/packages/@uppy/provider-views/lib/Browser.js
+++ b/packages/@uppy/provider-views/lib/Browser.js
@@ -43,7 +43,7 @@ function ListItem(props) {
     id: f.id,
     title: f.name,
     author: f.author,
-    getItemIcon: () => f.icon,
+    getItemIcon: () => viewType === "grid" && f.thumbnail ? f.thumbnail : f.icon,
     isChecked: isChecked(f),
     toggleCheckbox: event => toggleCheckbox(event, f),
     isCheckboxDisabled: false,
@@ -84,7 +84,7 @@ function Browser(props) {
     cancel,
     done,
     noResultsLabel,
-    loadAllFiles,
+    virtualList,
   } = props;
   const selected = currentSelection.length;
   const rows = useMemo(() => [...folders, ...files], [folders, files]);
@@ -131,7 +131,7 @@ function Browser(props) {
           className: "uppy-Provider-empty",
         }, noResultsLabel);
       }
-      if (loadAllFiles) {
+      if (virtualList) {
         return h(
           "div",
           {
diff --git a/packages/@uppy/provider-views/lib/Item/components/ListLi.js b/packages/@uppy/provider-views/lib/Item/components/ListLi.js
index 8784707..7e79d2c 100644
--- a/packages/@uppy/provider-views/lib/Item/components/ListLi.js
+++ b/packages/@uppy/provider-views/lib/Item/components/ListLi.js
@@ -66,7 +66,7 @@ export default function ListItem(props) {
         h("div", {
           className: "uppy-ProviderBrowserItem-iconWrap",
         }, itemIconEl),
-        showTitles && h("span", null, title),
+        showTitles && title ? h("span", null, title) : i18n("unnamed"),
       ),
   );
 }
diff --git a/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js b/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
index 33f6b85..19d6936 100644
--- a/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
+++ b/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
@@ -47,6 +47,7 @@ const defaultOptions = {
   showFilter: true,
   showBreadcrumbs: true,
   loadAllFiles: false,
+  virtualList: false,
 };
 var _abortController = _classPrivateFieldLooseKey("abortController");
 var _withAbort = _classPrivateFieldLooseKey("withAbort");
@@ -386,6 +387,7 @@ export default class ProviderView extends View {
       getNextFolder: this.getNextFolder,
       getFolder: this.getFolder,
       loadAllFiles: this.opts.loadAllFiles,
+      virtualList: this.opts.virtualList,
       showSearchFilter: targetViewOptions.showFilter,
       search: this.filterQuery,
       clearSearch: this.clearFilter,
diff --git a/packages/@uppy/provider-views/lib/View.js b/packages/@uppy/provider-views/lib/View.js
index 69cd639..0886ac5 100644
--- a/packages/@uppy/provider-views/lib/View.js
+++ b/packages/@uppy/provider-views/lib/View.js
@@ -1,5 +1,3 @@
-import getFileType from "@uppy/utils/lib/getFileType";
-import isPreviewSupported from "@uppy/utils/lib/isPreviewSupported";
 import remoteFileObjToLocal from "@uppy/utils/lib/remoteFileObjToLocal";
 export default class View {
   constructor(plugin, opts) {
@@ -104,8 +102,7 @@ export default class View {
         requestClientId: this.requestClientId,
       },
     };
-    const fileType = getFileType(tagFile);
-    if (fileType && isPreviewSupported(fileType)) {
+    if (file.thumbnail) {
       tagFile.preview = file.thumbnail;
     }
     if (file.author) {
diff --git a/packages/@uppy/remote-sources/lib/index.js b/packages/@uppy/remote-sources/lib/index.js
index a8af254..17d3e0b 100644
--- a/packages/@uppy/remote-sources/lib/index.js
+++ b/packages/@uppy/remote-sources/lib/index.js
@@ -13,6 +13,7 @@ import { BasePlugin } from "@uppy/core";
 import Dropbox from "@uppy/dropbox";
 import Facebook from "@uppy/facebook";
 import GoogleDrive from "@uppy/google-drive";
+import GooglePhotos from "@uppy/google-photos";
 import Instagram from "@uppy/instagram";
 import OneDrive from "@uppy/onedrive";
 import Unsplash from "@uppy/unsplash";
@@ -27,6 +28,7 @@ const availablePlugins = {
   Dropbox,
   Facebook,
   GoogleDrive,
+  GooglePhotos,
   Instagram,
   OneDrive,
   Unsplash,
diff --git a/packages/@uppy/transloadit/lib/index.js b/packages/@uppy/transloadit/lib/index.js
index 529121d..2683802 100644
--- a/packages/@uppy/transloadit/lib/index.js
+++ b/packages/@uppy/transloadit/lib/index.js
@@ -533,6 +533,7 @@ function _getClientVersion2() {
   addPluginVersion("Box", "uppy-box");
   addPluginVersion("Facebook", "uppy-facebook");
   addPluginVersion("GoogleDrive", "uppy-google-drive");
+  addPluginVersion("GooglePhotos", "uppy-google-photos");
   addPluginVersion("Instagram", "uppy-instagram");
   addPluginVersion("OneDrive", "uppy-onedrive");
   addPluginVersion("Zoom", "uppy-zoom");

@Murderlon Murderlon marked this pull request as draft April 8, 2024 12:15
@mifi mifi marked this pull request as ready for review April 12, 2024 16:20
@mifi
Copy link
Contributor Author

mifi commented Apr 12, 2024

I think this is now ready for MVP

mifi added a commit to transloadit/uppy.io that referenced this pull request Apr 12, 2024

This comment was marked as resolved.

@mifi
Copy link
Contributor Author

mifi commented Apr 22, 2024

hmm i ran yarn format however it doesn't seem that it worked

@mifi mifi requested review from aduh95 and Murderlon May 8, 2024 18:43
Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

Code wise in very good shape! Thanks for picking this up 👌

Still have to check this out locally to see how it feels

docs/sources/companion-plugins/google-photos.mdx Outdated Show resolved Hide resolved
packages/@uppy/dropbox/src/Dropbox.tsx Show resolved Hide resolved
packages/@uppy/remote-sources/src/index.js Outdated Show resolved Hide resolved
packages/@uppy/utils/src/VirtualList.tsx Outdated Show resolved Hide resolved
@mifi
Copy link
Contributor Author

mifi commented Jun 12, 2024

have now merged main and updated the folder icon. as for the missing folder name, let's just leave it as is because we don't know how to reproduce it?

@Murderlon
Copy link
Member

have now merged main and updated the folder icon. as for the missing folder name, let's just leave it as is because we don't know how to reproduce it?

Shall we just add Unnamed as a fallback? Makes it a bit less mysterious than nothing. Then we're good to go I think

Copy link

socket-security bot commented Jun 13, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@mifi
Copy link
Contributor Author

mifi commented Jun 13, 2024

done!

"version": "0.0.1",
"license": "MIT",
"main": "lib/index.js",
"types": "types/index.d.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to remember to remove that when it lands on 4.x

mifi and others added 2 commits June 13, 2024 19:12
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
…x.js

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
@aduh95 aduh95 merged commit e6674a1 into main Jun 18, 2024
21 checks passed
@aduh95 aduh95 deleted the google-photos branch June 18, 2024 09:13
@github-actions github-actions bot mentioned this pull request Jun 18, 2024
github-actions bot added a commit that referenced this pull request Jun 18, 2024
| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/box            |   2.4.0 | @uppy/onedrive       |   3.4.0 |
| @uppy/companion      |  4.14.0 | @uppy/provider-views |  3.13.0 |
| @uppy/core           |  3.13.0 | @uppy/react          |   3.4.0 |
| @uppy/dashboard      |   3.9.0 | @uppy/remote-sources |   1.3.0 |
| @uppy/dropbox        |   3.4.0 | @uppy/transloadit    |   3.8.0 |
| @uppy/google-drive   |   3.6.0 | uppy                 |  3.27.0 |
| @uppy/google-photos  |   0.1.0 |                      |         |

- @uppy/google-photos: add plugin (Mikael Finstad / #5061)
- examples: updating aws-nodejs example listParts logic for resuming uploads (Mitchell Rhoads / #5192)
- meta: Bump docker/login-action from 3.1.0 to 3.2.0 (dependabot\[bot] / #5217)
- meta: Bump docker/build-push-action from 5.3.0 to 5.4.0 (dependabot\[bot] / #5252)
- @uppy/transloadit: also fix outdated assembly transloadit:result (Merlijn Vos / #5246)
- docs: fix typo in the url (Evgenia Karunus)
- @uppy/companion: Bump ws from 8.8.1 to 8.17.1 (dependabot\[bot] / #5256)
aduh95 added a commit that referenced this pull request Jun 19, 2024
aduh95 added a commit that referenced this pull request Jun 19, 2024
@lakesare lakesare self-requested a review June 20, 2024 00:29
@lakesare
Copy link
Contributor

lakesare commented Jun 20, 2024

Testing this PR from within 4.x, found multiple issues that have to do with Google Photos api responses for some folder structures.


When the folder only has other folders inside of it (so - no video/photo files) - mediaItems is undefined, leading to companion errors:

image image

Similar issue happens when we do not have any files at all:

image image

Another issue is that when I click on the Unnamed folder, there is nothing in the breadcrumbs.

image

Murderlon added a commit that referenced this pull request Jun 20, 2024
* 4.x:
  @uppy/companion: fix merge conflicts
  Release: uppy@3.27.0 (#5257)
  Bump ws from 8.8.1 to 8.17.1 (#5256)
  @uppy/google-photos: add plugin (#5061)
  updating aws-nodejs example listParts logic for resuming uploads (#5192)
  Bump docker/login-action from 3.1.0 to 3.2.0 (#5217)
  Bump docker/build-push-action from 5.3.0 to 5.4.0 (#5252)
@mifi
Copy link
Contributor Author

mifi commented Jun 25, 2024

that's odd, according to google's documentation mediaItems will not be undefined , however when I test it I also get undefined, so there's clearly something wrong.

anyways I can reproduce the problems and will account for undefined values.

Another issue is that when I click on the Untitled folder, there is nothing in the breadcrumbs.

I don't know how to reproduce an unnamed folder in google photos, but I can reporduce the problem if I manually comment out the name property in companion. will fix it too!

mifi added a commit that referenced this pull request Jun 25, 2024
#5061 (comment)
also change the type of UppyFile.name, because it can be undefined
aduh95 pushed a commit that referenced this pull request Jun 27, 2024
Fixes: #5061 (comment)

Also change the type of `UppyFile.name`, because it can be `undefined`.
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.

Plugin for Google Photos
5 participants