fix various google photos issues #5275

mifi · 2024-06-25T11:49:37Z

#5061 (comment) also change the type of UppyFile.name, because it can be undefined

github-actions · 2024-06-25T11:51:01Z

Diff output files

diff --git a/packages/@uppy/companion/lib/server/provider/google/googlephotos/index.js b/packages/@uppy/companion/lib/server/provider/google/googlephotos/index.js
index ce54a6a..46249cc 100644
--- a/packages/@uppy/companion/lib/server/provider/google/googlephotos/index.js
    b/packages/@uppy/companion/lib/server/provider/google/googlephotos/index.js
@@ -59,7  59,7 @@ class GooglePhotos extends Provider {
         return paginate(
           (pageToken) =>
             client.get("albums", { searchParams: { pageToken, pageSize: 50 }, responseType: "json" }).json(),
-          (response) => response.albums,
           (response) => response.albums ?? [],
         );
       }
       async function fetchSharedAlbums() {
@@ -82,7  82,8 @@ class GooglePhotos extends Provider {
         }).json();
         return resp;
       }
-      const [sharedAlbums, albums, { mediaItems, nextPageToken }] = await Promise.all([
       // mediaItems seems to be undefined if empty folder
       const [sharedAlbums, albums, { mediaItems = [], nextPageToken }] = await Promise.all([
         fetchSharedAlbums(),
         fetchAlbums(),
         fetchMediaItems(),
diff --git a/packages/@uppy/core/lib/Restricter.js b/packages/@uppy/core/lib/Restricter.js
index 0dd13c1..b63360c 100644
--- a/packages/@uppy/core/lib/Restricter.js
    b/packages/@uppy/core/lib/Restricter.js
@@ -97,10  97,11 @@ class Restricter {
       }
     }
     if (maxFileSize && file.size != null && file.size > maxFileSize) {
       var _file$name;
       throw new RestrictionError(
         this.getI18n()("exceedsSize", {
           size: prettierBytes(maxFileSize),
-          file: file.name,
           file: (_file$name = file.name) != null ? _file$name : this.getI18n()("unnamed"),
         }),
         {
           file,
@@ -137,9  138,10 @@ class Restricter {
     }
   }
   getMissingRequiredMetaFields(file) {
     var _file$name2;
     const error = new RestrictionError(
       this.getI18n()("missingRequiredMetaFieldOnFile", {
-        fileName: file.name,
         fileName: (_file$name2 = file.name) != null ? _file$name2 : this.getI18n()("unnamed"),
       }),
     );
     const {
diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index dd04229..0577f7e 100644
--- a/packages/@uppy/core/lib/Uppy.js
    b/packages/@uppy/core/lib/Uppy.js
@@ -1168,9  1168,10 @@ function _checkAndUpdateFileState2(filesToAdd) {
       }
       const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(newFile, nextFilesState);
       if (!onBeforeFileAddedResult && this.checkIfFileAlreadyExists(newFile.id)) {
         var _newFile$name;
         throw new RestrictionError(
           this.i18n("noDuplicates", {
-            fileName: newFile.name,
             fileName: (_newFile$name = newFile.name) != null ? _newFile$name : this.i18n("unnamed"),
           }),
           {
             file: fileToAdd,
diff --git a/packages/@uppy/image-editor/lib/ImageEditor.js b/packages/@uppy/image-editor/lib/ImageEditor.js
index b74b720..cc110db 100644
--- a/packages/@uppy/image-editor/lib/ImageEditor.js
    b/packages/@uppy/image-editor/lib/ImageEditor.js
@@ -47,11  47,12 @@ export default class ImageEditor extends UIPlugin {
     });
     this.save = () => {
       const saveBlobCallback = blob => {
         var _name;
         const {
           currentImage,
         } = this.getPluginState();
         this.uppy.setFileState(currentImage.id, {
-          data: new File([blob], currentImage.name, {
           data: new File([blob], (_name = currentImage.name) != null ? _name : this.i18n("unnamed"), {
             type: blob.type,
           }),
           size: blob.size,
diff --git a/packages/@uppy/provider-views/lib/Breadcrumbs.js b/packages/@uppy/provider-views/lib/Breadcrumbs.js
index a911a5c..1b0f1e6 100644
--- a/packages/@uppy/provider-views/lib/Breadcrumbs.js
    b/packages/@uppy/provider-views/lib/Breadcrumbs.js
@@ -5,6  5,7 @@ export default function Breadcrumbs(props) {
     title,
     breadcrumbsIcon,
     breadcrumbs,
     i18n,
   } = props;
   return h(
     "div",
@@ -14,18  15,27 @@ export default function Breadcrumbs(props) {
     h("div", {
       className: "uppy-Provider-breadcrumbsIcon",
     }, breadcrumbsIcon),
-    breadcrumbs.map((folder, index) =>
-      h(
     breadcrumbs.map((folder, index) => {
       var _folder$data$name;
       return h(
         Fragment,
         null,
-        h("button", {
-          key: folder.id,
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          onClick: () => openFolder(folder.id),
-        }, folder.type === "root" ? title : folder.data.name),
         h(
           "button",
           {
             key: folder.id,
             type: "button",
             className: "uppy-u-reset uppy-c-btn",
             onClick: () => openFolder(folder.id),
           },
           folder.type === "root"
             ? title
             : (_folder$data$name = folder.data.name) != null
             ? _folder$data$name
             : i18n("unnamed"),
         ),
         breadcrumbs.length === index   1 ? "" : " / ",
-      )
-    ),
       );
     }),
   );
 }
diff --git a/packages/@uppy/provider-views/lib/Item/components/GridItem.js b/packages/@uppy/provider-views/lib/Item/components/GridItem.js
index 9b425f2..1393a50 100644
--- a/packages/@uppy/provider-views/lib/Item/components/GridItem.js
    b/packages/@uppy/provider-views/lib/Item/components/GridItem.js
@@ -1,6  1,7 @@
 import { h } from "preact";
 import ItemIcon from "./ItemIcon.js";
 function GridItem(_ref) {
   var _file$data$name, _file$data$name2;
   let {
     file,
     toggleCheckbox,
@@ -9,6  10,7 @@ function GridItem(_ref) {
     restrictionError,
     showTitles,
     children = null,
     i18n,
   } = _ref;
   return h(
     "li",
@@ -30,13  32,13 @@ function GridItem(_ref) {
       "label",
       {
         htmlFor: file.id,
-        "aria-label": file.data.name,
         "aria-label": (_file$data$name = file.data.name) != null ? _file$data$name : i18n("unnamed"),
         className: "uppy-u-reset uppy-ProviderBrowserItem-inner",
       },
       h(ItemIcon, {
         itemIconString: file.data.thumbnail || file.data.icon,
       }),
-      showTitles && file.data.name,
       showTitles && ((_file$data$name2 = file.data.name) != null ? _file$data$name2 : i18n("unnamed")),
       children,
     ),
   );
diff --git a/packages/@uppy/provider-views/lib/Item/components/ListItem.js b/packages/@uppy/provider-views/lib/Item/components/ListItem.js
index 7417bac..143d98e 100644
--- a/packages/@uppy/provider-views/lib/Item/components/ListItem.js
    b/packages/@uppy/provider-views/lib/Item/components/ListItem.js
@@ -1,6  1,7 @@
 import { h } from "preact";
 import ItemIcon from "./ItemIcon.js";
 export default function ListItem(_ref) {
   var _file$data$name, _file$data$name2, _file$data$name3;
   let {
     file,
     openFolder,
@@ -26,7  27,7 @@ export default function ListItem(_ref) {
       checked: file.status === "checked",
       "aria-label": file.data.isFolder
         ? i18n("allFilesFromFolderNamed", {
-          name: file.data.name,
           name: (_file$data$name = file.data.name) != null ? _file$data$name : i18n("unnamed"),
         })
         : null,
       disabled: isDisabled,
@@ -40,7  41,7 @@ export default function ListItem(_ref) {
           className: "uppy-u-reset uppy-c-btn uppy-ProviderBrowserItem-inner",
           onClick: () => openFolder(file.id),
           "aria-label": i18n("openFolderNamed", {
-            name: file.data.name,
             name: (_file$data$name2 = file.data.name) != null ? _file$data$name2 : i18n("unnamed"),
           }),
         },
         h(
@@ -69,7  70,7 @@ export default function ListItem(_ref) {
             itemIconString: file.data.icon,
           }),
         ),
-        showTitles && file.data.name,
         showTitles && ((_file$data$name3 = file.data.name) != null ? _file$data$name3 : i18n("unnamed")),
       ),
   );
 }
diff --git a/packages/@uppy/provider-views/lib/ProviderView/Header.js b/packages/@uppy/provider-views/lib/ProviderView/Header.js
index aa73a46..c930a83 100644
--- a/packages/@uppy/provider-views/lib/ProviderView/Header.js
    b/packages/@uppy/provider-views/lib/ProviderView/Header.js
@@ -21,6  21,7 @@ export default function Header(props) {
         breadcrumbs: props.breadcrumbs,
         breadcrumbsIcon: props.pluginIcon && props.pluginIcon(),
         title: props.title,
         i18n: props.i18n,
       }),
       h(User, {
         logout: props.logout,
diff --git a/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js b/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
index 66f9861..3e84e3e 100644
--- a/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
    b/packages/@uppy/provider-views/lib/ProviderView/ProviderView.js
@@ -78,9  78,11 @@ export default class ProviderView {
         searchString,
       } = this.plugin.getPluginState();
       const inThisFolder = partialTree.filter(item => item.type !== "root" && item.parentId === currentFolderId);
-      const filtered = searchString === ""
-        ? inThisFolder
-        : inThisFolder.filter(item => item.data.name.toLowerCase().indexOf(searchString.toLowerCase()) !== -1);
       const filtered = searchString === "" ? inThisFolder : inThisFolder.filter(item => {
         var _item$data$name;
         return ((_item$data$name = item.data.name) != null ? _item$data$name : this.plugin.uppy.i18n("unnamed"))
           .toLowerCase().indexOf(searchString.toLowerCase()) !== -1;
       });
       return filtered;
     };
     this.validateAggregateRestrictions = partialTree => {
diff --git a/packages/@uppy/transloadit/lib/Client.js b/packages/@uppy/transloadit/lib/Client.js
index 3c83a61..d1187c3 100644
--- a/packages/@uppy/transloadit/lib/Client.js
    b/packages/@uppy/transloadit/lib/Client.js
@@ -103,12  103,13 @@ export default class Client {
     );
   }
   async addFile(assembly, file) {
     var _file$name;
     if (!file.uploadURL) {
       return Promise.reject(new Error("File does not have an `uploadURL`."));
     }
     const size = encodeURIComponent(file.size);
     const uploadUrl = encodeURIComponent(file.uploadURL);
-    const filename = encodeURIComponent(file.name);
     const filename = encodeURIComponent((_file$name = file.name) != null ? _file$name : "Unnamed");
     const fieldname = "file";
     const qs = `size=${size}&filename=${filename}&fieldname=${fieldname}&s3Url=${uploadUrl}`;
     const url = `${assembly.assembly_ssl_url}/add_file?${qs}`;

lakesare · 2024-06-26T04:48:49Z

packages/@uppy/transloadit/src/Client.ts

@@ -159,7 159,7 @@ export default class Client<M extends Meta, B extends Body> {
 }
 const size = encodeURIComponent(file.size!)
 const uploadUrl = encodeURIComponent(file.uploadURL)
- const filename = encodeURIComponent(file.name)
+ const filename = encodeURIComponent(file.name ?? 'Unnamed')


Hm it looks like we have to repeat "Unnamed" in a lot of places.
And in general, we have to think about the possibility that .name can be absent.

Have you considered injecting "Unnamed" as a .name in GooglePhotos.tsx?

lakesare · 2024-06-26T04:50:06Z

All the issue described here are fixed in this PR 👍

Not sure about making .name optional however, left a comment.

lakesare · 2024-06-26T05:06:13Z

Some thoughts about the UI of GooglePhotos.

Here is what Uppy shows to me:

5 normal albums - this is expected - I also have these 5 albums in GooglePhotos
then goes "NDR discussions", this is apparently the album that was shared with me
and then go 4 unnamed albums - I do not know where they come from.
The photos are mine, but I do not see why one of the videos of the cow in a lake I took is in a separate album.
3 of those albums are just empty.

Suggestions:

Can we remove the 4 unnamed albums? Are there any folder properties google api returns to us that could indicate these albums are unusual, or is their only unusual quality the fact that they are unnamed?
Even if that's the case - I would go for removing "Unnamed" albums, I think this is almost certainly a GooglePhotos artifact. If someone created an album on purpose, they would sure name it. This won't prevent people from selecting a photo from that album, they can simply select the photo from the "Timeline" album.
Can we move the shared albums to the fake "Shared with me" folder, like we do in GoogleDrive?

So, taken together with my messages in Slack, I suggest the following interface:

remove "Unnamed" albums
add 3 fake folders: "Shared with me" (contains all photos/albums shared with me), "Albums" (contains a list of my albums), and "Timeline" (contains both albumless and within-album photos, exactly what we see in GooglePhotos interface).

aduh95 · 2024-06-27T08:30:11Z

Taking @lakesare to a new issue as I think 3.x is also affected (and therefore it should be fixed in a separate PR).

lakesare · 2024-06-27T10:20:39Z

@aduh95, I feel like the merge was a bit premature, we should have let Mikael respond to our questions & then merge it when some consensus is reached.
I understand this feels like an urgent fix for 3.x, but GooglePhotos is very clearly mvp functionality that only shows albums - it's not a big deal that some folder ids weren't working.
UPDATE: wait this is merged into 4.x, don't see a good reason to merge so soon then?

@mifi, one problem with letting .name sometimes be undefined is that in getCheckedFilesWithPaths.ts this gives us absDirPath and relDirPath that look like "/my.jpg", even though in reality it's "Unnamed/my.jpg".

aduh95 · 2024-06-27T10:41:18Z

My understanding was this PR was bringing 4.x at the same feature level as main, and your suggestions were not related to 4.x but to the plugin in general.

lakesare · 2024-06-27T10:49:23Z

You probably missed #5275 (comment) (name? change isn't in main yet).
The #5275 (comment) suggestion is here because it might remove the need for "Unnamed" folders altogether, removing the necessity of name?.

aduh95 · 2024-06-27T11:06:02Z

@mifi can you please open a PR against main that implements that change, and address Evgenia's comment?

| Package | Version | Package | Version | | ---------------------- | ------------- | ---------------------- | ------------- | | @uppy/audio | 2.0.0-beta.7 | @uppy/image-editor | 3.0.0-beta.6 | | @uppy/aws-s3 | 4.0.0-beta.8 | @uppy/instagram | 4.0.0-beta.7 | | @uppy/box | 3.0.0-beta.8 | @uppy/onedrive | 4.0.0-beta.8 | | @uppy/companion | 5.0.0-beta.11 | @uppy/provider-views | 4.0.0-beta.10 | | @uppy/companion-client | 4.0.0-beta.8 | @uppy/react | 4.0.0-beta.8 | | @uppy/core | 4.0.0-beta.11 | @uppy/screen-capture | 4.0.0-beta.6 | | @uppy/dashboard | 4.0.0-beta.11 | @uppy/transloadit | 4.0.0-beta.10 | | @uppy/drop-target | 3.0.0-beta.6 | @uppy/unsplash | 4.0.0-beta.8 | | @uppy/dropbox | 4.0.0-beta.9 | @uppy/url | 4.0.0-beta.8 | | @uppy/facebook | 4.0.0-beta.7 | @uppy/utils | 6.0.0-beta.9 | | @uppy/file-input | 4.0.0-beta.6 | @uppy/vue | 2.0.0-beta.4 | | @uppy/form | 4.0.0-beta.5 | @uppy/webcam | 4.0.0-beta.9 | | @uppy/golden-retriever | 4.0.0-beta.6 | @uppy/xhr-upload | 4.0.0-beta.7 | | @uppy/google-drive | 4.0.0-beta.1 | @uppy/zoom | 3.0.0-beta.7 | | @uppy/google-photos | 0.2.0-beta.2 | uppy | 4.0.0-beta.13 | - @uppy/companion: implement facebook app secret proof (Mikael Finstad / #5249) - @uppy/provider-views: `Loader.tsx` - delete the file (Evgenia Karunus / #5284) - @uppy/vue: fix passing of `props` (Antoine du Hamel / #5281) - @uppy/google-photos: fix various issues (Mikael Finstad / #5275) - @uppy/transloadit: fix strict type errors (Antoine du Hamel / #5271) - @uppy/transloadit: simplify plugin to always run a single assembly (Merlijn Vos / #5158) - meta: update Yarn version and npm deps (Antoine du Hamel / #5269) - docs: prettier: 3.2.5 -> 3.3.2 (Antoine du Hamel / #5270) - @uppy/provider-views: Provider views rewrite (.files, .folders => .partialTree) (Evgenia Karunus / #5050) - @uppy/react: TS strict mode (Merlijn Vos / #5258) - meta: simplify `build:ts` script (Antoine du Hamel / #5262)

mifi · 2024-07-01T12:21:58Z

@aduh95 but this PR was merged into 4.x, and if we decide on removing unnamed folders, I need to revert most of what's done in this PR (because name will never be undefined anymore), so I think it has to be done in 4.x

mifi · 2024-07-01T12:28:02Z

Btw should we i18n "Shared with me", "Albums" and "Timeline"? currently for Google Drive, the static string Shared with me is returned as name from Companion so it's always in english. Not sure the best way to support i18n, but maybe leave name empty and have a separate field named something like specialFolderType which can then be i18n in the Uppy frontend? (maybe Uppy can set name to this i18n value when it receives a folder with specialFolderType from companion)

fix various issues

168ca5a

#5061 (comment) also change the type of UppyFile.name, because it can be undefined

mifi changed the title ~~fix various issues~~ fix various google photos issues Jun 25, 2024

aduh95 self-requested a review June 25, 2024 14:53

lakesare self-requested a review June 26, 2024 03:53

lakesare reviewed Jun 26, 2024

View reviewed changes

aduh95 mentioned this pull request Jun 27, 2024

UI improvements for Google Photos #5280

Open

aduh95 merged commit d536a35 into 4.x Jun 27, 2024
20 checks passed

aduh95 deleted the google-photos-issues branch June 27, 2024 08:31

github-actions bot mentioned this pull request Jun 27, 2024

Release: [email protected] #5286

Merged

mifi mentioned this pull request Jul 1, 2024

i18n "Shared with me" etc #5294

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix various google photos issues #5275

fix various google photos issues #5275

mifi commented Jun 25, 2024

github-actions bot commented Jun 25, 2024

lakesare Jun 26, 2024

lakesare commented Jun 26, 2024 •

edited

Loading

lakesare commented Jun 26, 2024 •

edited

Loading

aduh95 commented Jun 27, 2024

lakesare commented Jun 27, 2024 •

edited by aduh95

Loading

aduh95 commented Jun 27, 2024

lakesare commented Jun 27, 2024 •

edited

Loading

aduh95 commented Jun 27, 2024

mifi commented Jul 1, 2024

mifi commented Jul 1, 2024 •

edited

Loading

fix various google photos issues #5275

fix various google photos issues #5275

Conversation

mifi commented Jun 25, 2024

github-actions bot commented Jun 25, 2024

lakesare Jun 26, 2024

Choose a reason for hiding this comment

lakesare commented Jun 26, 2024 • edited Loading

lakesare commented Jun 26, 2024 • edited Loading

aduh95 commented Jun 27, 2024

lakesare commented Jun 27, 2024 • edited by aduh95 Loading

aduh95 commented Jun 27, 2024

lakesare commented Jun 27, 2024 • edited Loading

aduh95 commented Jun 27, 2024

mifi commented Jul 1, 2024

mifi commented Jul 1, 2024 • edited Loading

lakesare commented Jun 26, 2024 •

edited

Loading

lakesare commented Jun 26, 2024 •

edited

Loading

lakesare commented Jun 27, 2024 •

edited by aduh95

Loading

lakesare commented Jun 27, 2024 •

edited

Loading

mifi commented Jul 1, 2024 •

edited

Loading