Fav

web/apps/photos/src/pages/gallery.tsx

@@ -112,8 +112,8 @@ import { useCallback, useEffect, useRef, useState } from "react";
 import { FileWithPath } from "react-dropzone";
 import { Trans } from "react-i18next";
 import {
-    addToFavorites,
-    removeFromFavorites,
+    addToFavorites1,
+    removeFromFavorites1,
 } from "services/collectionService";
 import { uploadManager } from "services/upload-manager";
 import {
@@ -763,9 +763,8 @@ const Page: React.FC = () => {
 
             dispatch({ type: "addPendingFavoriteUpdate", fileID });
             try {
-                await (isFavorite ? removeFromFavorites : addToFavorites)(
+                await (isFavorite ? removeFromFavorites1 : addToFavorites1)(
                     file,
-                    true,
                 );
                 dispatch({
                     type: "unsyncedFavoriteUpdate",

web/apps/photos/src/services/collectionService.ts

@@ -1,14 +1,13 @@
 import type { User } from "ente-accounts/services/user";
-import { ensureLocalUser } from "ente-accounts/services/user";
 import log from "ente-base/log";
 import { apiURL } from "ente-base/origins";
 import { groupFilesByCollectionID, sortFiles } from "ente-gallery/utils/file";
 import { EnteFile } from "ente-media/file";
 import {
-    addToCollection,
-    createFavoritesCollection,
+    addToFavorites,
     createUncategorizedCollection,
     moveFromCollection,
+    savedUserFavoritesCollection,
 } from "ente-new/photos/services/collection";
 import type { CollectionSummary } from "ente-new/photos/services/collection-summary";
 import {
@@ -27,49 +26,16 @@ import { isValidMoveTarget } from "utils/collection";
 
 const REQUEST_BATCH_SIZE = 1000;
 
-export const addToFavorites = async (
-    file: EnteFile,
-    disableOldWorkaround?: boolean,
-) => {
-    await addMultipleToFavorites([file], disableOldWorkaround);
+export const addToFavorites1 = async (file: EnteFile) => {
+    await addToFavorites([file]);
 };
 
-export const addMultipleToFavorites = async (
-    files: EnteFile[],
-    disableOldWorkaround?: boolean,
-) => {
-    try {
-        let favCollection = await getFavCollection();
-        if (!favCollection) {
-            favCollection = await createFavoritesCollection();
-        }
-        await addToCollection(favCollection, files);
-    } catch (e) {
-        log.error("failed to add to favorite", e);
-        // Old code swallowed the error here. This isn't good, but to
-        // avoid changing existing behaviour only new code will set the
-        // disableOldWorkaround flag to instead rethrow it.
-        //
-        // TODO: Migrate old code, remove this flag, always throw.
-        if (disableOldWorkaround) throw e;
-    }
-};
-
-export const removeFromFavorites = async (
-    file: EnteFile,
-    disableOldWorkaround?: boolean,
-) => {
-    try {
-        const favCollection = await getFavCollection();
-        if (!favCollection) {
-            throw Error("favorite collection missing");
-        }
-        await removeFromCollection(favCollection.id, [file]);
-    } catch (e) {
-        log.error("remove from favorite failed", e);
-        // TODO: See disableOldWorkaround in addMultipleToFavorites.
-        if (disableOldWorkaround) throw e;
+export const removeFromFavorites1 = async (file: EnteFile) => {
+    const favCollection = await savedUserFavoritesCollection();
+    if (!favCollection) {
+        throw Error("favorite collection missing");
     }
+    await removeFromCollection(favCollection.id, [file]);
 };
 
 export const removeFromCollection = async (
@@ -223,20 +189,6 @@ export const deleteCollection = async (
     }
 };
 
-/**
- * Return the user's own favorites collection, if any.
- */
-export const getFavCollection = async () => {
-    const collections = await savedCollections();
-    const userID = ensureLocalUser().id;
-    for (const collection of collections) {
-        // See: [Note: User and shared favorites]
-        if (collection.type == "favorites" && collection.owner.id == userID) {
-            return collection;
-        }
-    }
-};
-
 export const sortCollectionSummaries = (
     collectionSummaries: CollectionSummary[],
     by: CollectionsSortBy,

web/apps/photos/src/utils/file/index.ts

@@ -15,6 +15,7 @@ import {
 import { FileType } from "ente-media/file-type";
 import { decodeLivePhoto } from "ente-media/live-photo";
 import {
+    addToFavorites,
     deleteFromTrash,
     hideFiles,
     moveToTrash,
@@ -24,7 +25,6 @@ import { safeFileName } from "ente-new/photos/utils/native-fs";
 import { getData } from "ente-shared/storage/localStorage";
 import { wait } from "ente-utils/promise";
 import { t } from "i18next";
-import { addMultipleToFavorites } from "services/collectionService";
 import {
     SelectedState,
     SetFilesDownloadProgressAttributes,
@@ -358,7 +358,7 @@ export const handleFileOp = async (
             fixCreationTime(files);
             break;
         case "favorite":
-            await addMultipleToFavorites(files);
+            await addToFavorites(files);
             break;
         case "archive":
             await updateFilesVisibility(files, ItemVisibility.archived);

web/packages/new/photos/services/collection.ts

@@ -916,7 +916,7 @@ const putCollectionsShareeMagicMetadata = async (
  * because remote will enforce the constraint and fail the request when we
  * attempt to create a second collection of type "favorites".
  */
-export const createFavoritesCollection = () =>
+const createFavoritesCollection = () =>
     createCollection(favoritesCollectionName, "favorites");
 
 /**
@@ -933,16 +933,57 @@ export const createFavoritesCollection = () =>
 export const createUncategorizedCollection = () =>
     createCollection(uncategorizedCollectionName, "uncategorized");
 
+/**
+ * Return the user's own favorites collection if one is found in the local
+ * database. Otherwise create a new one and return that.
+ *
+ * Reads local state but does not modify it. The effects are on remote.
+ */
+const savedOrCreateUserFavoritesCollection = async () =>
+    (await savedUserFavoritesCollection()) ?? createFavoritesCollection();
+
+/**
+ * Return the user's own favorites collection, if any, present in the local
+ * database.
+ */
+export const savedUserFavoritesCollection = async () => {
+    const userID = ensureLocalUser().id;
+    const collections = await savedCollections();
+    return collections.find(
+        (collection) =>
+            // See: [Note: User and shared favorites]
+            collection.type == "favorites" && collection.owner.id == userID,
+    );
+};
+
+/**
+ * Mark the provided {@link files} as the user's favorites by adding them to the
+ * user's favorites collection.
+ *
+ * If the user doesn't yet have a favorites collection, it is created.
+ *
+ * Reads local state but does not modify it. The effects are on remote.
+ */
+export const addToFavorites = async (files: EnteFile[]) =>
+    addToCollection(await savedOrCreateUserFavoritesCollection(), files);
+
 /**
  * Return the default hidden collection for the user if one is found in the
  * local database. Otherwise create a new one and return that.
  *
  * Reads local state but does not modify it. The effects are on remote.
  */
-const getOrCreateDefaultHiddenCollection = async () =>
+const savedOrCreateDefaultHiddenCollection = async () =>
+    (await savedDefaultHiddenCollection()) ?? createDefaultHiddenCollection();
+
+/**
+ * Return the user's default hidden collection, if any, present in the
+ * local database.
+ */
+const savedDefaultHiddenCollection = async () =>
     (await savedCollections()).find((collection) =>
         isDefaultHiddenCollection(collection),
-    ) ?? createDefaultHiddenCollection();
+    );
 
 /**
  * Create a new collection with hidden visibility on remote, marking it as the
@@ -994,7 +1035,7 @@ export const isHiddenCollection = (collection: Collection) =>
  * Reads local state but does not modify it. The effects are on remote.
  */
 export const hideFiles = async (files: EnteFile[]) =>
-    moveToCollection(await getOrCreateDefaultHiddenCollection(), files);
+    moveToCollection(await savedOrCreateDefaultHiddenCollection(), files);
 
 /**
  * Return true if this is a collection that the user doesn't own.