Fix race condition in collection and favorite rom membership updates

Replace full rom_ids list replacement with atomic POST/DELETE endpoints
that add or remove individual ROMs from a collection. This prevents
concurrent rapid clicks from overwriting each other (last-write-wins).

Also fix missing session.flush() in add_rom_user() and add collection
endpoint tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

backend/endpoints/collections.py

@@ -6,6 +6,7 @@ from typing import Annotated
 from fastapi import File, Form, HTTPException
 from fastapi import Path as PathVar
 from fastapi import Query, Request, UploadFile, status
+from pydantic import BaseModel as PydanticBaseModel
 
 from decorators.auth import protected_route
 from endpoints.responses.collection import (
@@ -474,6 +475,68 @@ async def update_collection(
     return CollectionSchema.model_validate(updated_collection)
 
 
+class CollectionRomsPayload(PydanticBaseModel):
+    rom_ids: list[int]
+
+
+@protected_route(router.post, "/{id}/roms", [Scope.COLLECTIONS_WRITE])
+async def add_roms_to_collection(
+    request: Request,
+    id: int,
+    payload: CollectionRomsPayload,
+) -> CollectionSchema:
+    """Atomically add ROMs to a collection without replacing the full list.
+
+    Args:
+        request (Request): Fastapi Request object
+        id (int): Collection id
+        payload (CollectionRomsPayload): ROM IDs to add
+
+    Returns:
+        CollectionSchema: Updated collection
+    """
+    collection = db_collection_handler.get_collection(id)
+    if not collection:
+        raise CollectionNotFoundInDatabaseException(id)
+
+    if collection.user_id != request.user.id:
+        raise CollectionPermissionError(id)
+
+    updated_collection = db_collection_handler.add_roms_to_collection(
+        id, payload.rom_ids
+    )
+    return CollectionSchema.model_validate(updated_collection)
+
+
+@protected_route(router.delete, "/{id}/roms", [Scope.COLLECTIONS_WRITE])
+async def remove_roms_from_collection(
+    request: Request,
+    id: int,
+    payload: CollectionRomsPayload,
+) -> CollectionSchema:
+    """Atomically remove ROMs from a collection without replacing the full list.
+
+    Args:
+        request (Request): Fastapi Request object
+        id (int): Collection id
+        payload (CollectionRomsPayload): ROM IDs to remove
+
+    Returns:
+        CollectionSchema: Updated collection
+    """
+    collection = db_collection_handler.get_collection(id)
+    if not collection:
+        raise CollectionNotFoundInDatabaseException(id)
+
+    if collection.user_id != request.user.id:
+        raise CollectionPermissionError(id)
+
+    updated_collection = db_collection_handler.remove_roms_from_collection(
+        id, payload.rom_ids
+    )
+    return CollectionSchema.model_validate(updated_collection)
+
+
 @protected_route(router.put, "/smart/{id}", [Scope.COLLECTIONS_WRITE])
 async def update_smart_collection(
     request: Request,

backend/handler/database/collections_handler.py

@@ -1,6 +1,6 @@
 import functools
 from collections.abc import Sequence
-from datetime import datetime
+from datetime import datetime, timezone
 from typing import Any
 
 from sqlalchemy import delete, insert, literal, or_, select, update
@@ -146,6 +146,66 @@ class DBCollectionsHandler(DBBaseHandler):
 
         return session.scalar(query.filter_by(id=id).limit(1))
 
+    @begin_session
+    @with_roms
+    def add_roms_to_collection(
+        self,
+        id: int,
+        rom_ids: list[int],
+        query: Query = None,  # type: ignore
+        session: Session = None,  # type: ignore
+    ) -> Collection:
+        if rom_ids:
+            valid_rom_ids = set(
+                session.scalars(select(Rom.id).where(Rom.id.in_(rom_ids))).all()
+            )
+            existing_ids = set(
+                session.scalars(
+                    select(CollectionRom.rom_id).where(
+                        CollectionRom.collection_id == id
+                    )
+                ).all()
+            )
+            new_ids = valid_rom_ids - existing_ids
+            if new_ids:
+                session.execute(
+                    insert(CollectionRom),
+                    [{"collection_id": id, "rom_id": rom_id} for rom_id in new_ids],
+                )
+                session.execute(
+                    update(Collection)
+                    .where(Collection.id == id)
+                    .values(updated_at=datetime.now(timezone.utc))
+                    .execution_options(synchronize_session="evaluate")
+                )
+
+        return session.scalar(query.filter_by(id=id).limit(1))
+
+    @begin_session
+    @with_roms
+    def remove_roms_from_collection(
+        self,
+        id: int,
+        rom_ids: list[int],
+        query: Query = None,  # type: ignore
+        session: Session = None,  # type: ignore
+    ) -> Collection:
+        if rom_ids:
+            session.execute(
+                delete(CollectionRom).where(
+                    CollectionRom.collection_id == id,
+                    CollectionRom.rom_id.in_(rom_ids),
+                )
+            )
+            session.execute(
+                update(Collection)
+                .where(Collection.id == id)
+                .values(updated_at=datetime.now(timezone.utc))
+                .execution_options(synchronize_session="evaluate")
+            )
+
+        return session.scalar(query.filter_by(id=id).limit(1))
+
     @begin_session
     def delete_collection(
         self,

backend/handler/database/roms_handler.py

@@ -1066,7 +1066,9 @@ class DBRomsHandler(DBBaseHandler):
         user_id: int,
         session: Session = None,  # type: ignore
     ) -> RomUser:
-        return session.merge(RomUser(rom_id=rom_id, user_id=user_id))
+        rom_user = session.merge(RomUser(rom_id=rom_id, user_id=user_id))
+        session.flush()
+        return rom_user
 
     @begin_session
     def get_rom_user(

backend/tests/endpoints/test_collection.py

@@ -0,0 +1,557 @@
+from datetime import timedelta
+
+import pytest
+from fastapi import status
+
+from config import OAUTH_ACCESS_TOKEN_EXPIRE_SECONDS
+from handler.auth import oauth_handler
+from handler.database import db_collection_handler, db_rom_handler, db_user_handler
+from models.collection import Collection
+from models.platform import Platform
+from models.rom import Rom
+from models.user import Role, User
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+def collection(admin_user: User) -> Collection:
+    return db_collection_handler.add_collection(
+        Collection(
+            name="Test Collection",
+            description="A test collection",
+            is_public=False,
+            is_favorite=False,
+            user_id=admin_user.id,
+        )
+    )
+
+
+@pytest.fixture
+def favorite_collection(admin_user: User) -> Collection:
+    return db_collection_handler.add_collection(
+        Collection(
+            name="Favorites",
+            description="",
+            is_public=False,
+            is_favorite=True,
+            user_id=admin_user.id,
+        )
+    )
+
+
+@pytest.fixture
+def second_rom(admin_user: User, platform: Platform) -> Rom:
+    rom = Rom(
+        platform_id=platform.id,
+        name="test_rom_2",
+        slug="test_rom_slug_2",
+        fs_name="test_rom_2.zip",
+        fs_name_no_tags="test_rom_2",
+        fs_name_no_ext="test_rom_2",
+        fs_extension="zip",
+        fs_path=f"{platform.slug}/roms",
+    )
+    return db_rom_handler.add_rom(rom)
+
+
+@pytest.fixture
+def other_user_token(editor_user: User) -> str:
+    """Access token for a second user — used to test ownership checks."""
+    return oauth_handler.create_access_token(
+        data={
+            "sub": editor_user.username,
+            "iss": "romm:oauth",
+            "scopes": " ".join(editor_user.oauth_scopes),
+        },
+        expires_delta=timedelta(seconds=OAUTH_ACCESS_TOKEN_EXPIRE_SECONDS),
+    )
+
+
+@pytest.fixture
+def other_user_collection(editor_user: User) -> Collection:
+    """A collection owned by the editor user, not the admin user."""
+    return db_collection_handler.add_collection(
+        Collection(
+            name="Editor Collection",
+            description="",
+            is_public=False,
+            is_favorite=False,
+            user_id=editor_user.id,
+        )
+    )
+
+
+# ---------------------------------------------------------------------------
+# Collection CRUD
+# ---------------------------------------------------------------------------
+
+
+class TestCreateCollection:
+    def test_creates_collection(self, client, access_token: str):
+        response = client.post(
+            "/api/collections",
+            data={"name": "My Games", "description": "Classic games"},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert data["name"] == "My Games"
+        assert data["description"] == "Classic games"
+        assert data["is_favorite"] is False
+        assert data["is_public"] is False
+        assert data["rom_ids"] == []
+
+    def test_creates_favorite_collection(self, client, access_token: str):
+        response = client.post(
+            "/api/collections",
+            data={"name": "Favorites"},
+            params={"is_favorite": True},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert data["is_favorite"] is True
+
+    def test_duplicate_name_returns_conflict(
+        self, client, access_token: str, collection: Collection
+    ):
+        response = client.post(
+            "/api/collections",
+            data={"name": collection.name},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_409_CONFLICT
+
+    def test_requires_auth(self, client):
+        response = client.post("/api/collections", data={"name": "No Auth"})
+        assert response.status_code in (
+            status.HTTP_401_UNAUTHORIZED,
+            status.HTTP_403_FORBIDDEN,
+        )
+
+
+class TestGetCollections:
+    def test_returns_own_collections(
+        self, client, access_token: str, collection: Collection
+    ):
+        response = client.get(
+            "/api/collections",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert len(data) == 1
+        assert data[0]["id"] == collection.id
+        assert data[0]["name"] == collection.name
+
+    def test_returns_public_collections_from_other_users(
+        self,
+        client,
+        access_token: str,
+        admin_user: User,
+        editor_user: User,
+    ):
+        public_col = db_collection_handler.add_collection(
+            Collection(
+                name="Public Editor Collection",
+                is_public=True,
+                is_favorite=False,
+                user_id=editor_user.id,
+            )
+        )
+        db_collection_handler.add_collection(
+            Collection(
+                name="Private Editor Collection",
+                is_public=False,
+                is_favorite=False,
+                user_id=editor_user.id,
+            )
+        )
+
+        response = client.get(
+            "/api/collections",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        ids = [c["id"] for c in response.json()]
+        assert public_col.id in ids
+
+    def test_empty_when_no_collections(self, client, access_token: str):
+        response = client.get(
+            "/api/collections",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json() == []
+
+
+class TestDeleteCollection:
+    def test_deletes_own_collection(
+        self, client, access_token: str, collection: Collection
+    ):
+        response = client.delete(
+            f"/api/collections/{collection.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_200_OK
+        assert db_collection_handler.get_collection(collection.id) is None
+
+    def test_cannot_delete_other_users_collection(
+        self,
+        client,
+        access_token: str,
+        other_user_collection: Collection,
+    ):
+        response = client.delete(
+            f"/api/collections/{other_user_collection.id}",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_returns_404_for_missing_collection(self, client, access_token: str):
+        response = client.delete(
+            "/api/collections/999999",
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
+# ---------------------------------------------------------------------------
+# POST /collections/{id}/roms  —  atomic add
+# ---------------------------------------------------------------------------
+
+
+class TestAddRomsToCollection:
+    def test_adds_roms_to_empty_collection(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert rom.id in data["rom_ids"]
+        assert data["rom_count"] == 1
+
+    def test_adds_multiple_roms(
+        self,
+        client,
+        access_token: str,
+        collection: Collection,
+        rom: Rom,
+        second_rom: Rom,
+    ):
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id, second_rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert set(data["rom_ids"]) == {rom.id, second_rom.id}
+        assert data["rom_count"] == 2
+
+    def test_is_idempotent_no_duplicates(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        """Adding a ROM that is already in the collection should not duplicate it."""
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert data["rom_ids"].count(rom.id) == 1
+        assert data["rom_count"] == 1
+
+    def test_preserves_existing_roms_when_adding_new(
+        self,
+        client,
+        access_token: str,
+        collection: Collection,
+        rom: Rom,
+        second_rom: Rom,
+    ):
+        """Adding a new ROM must not remove previously added ROMs."""
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [second_rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert set(data["rom_ids"]) == {rom.id, second_rom.id}
+
+    def test_silently_ignores_nonexistent_rom_ids(
+        self, client, access_token: str, collection: Collection
+    ):
+        """Invalid ROM IDs should be filtered out without raising an error."""
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [999999]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["rom_count"] == 0
+
+    def test_returns_404_for_missing_collection(
+        self, client, access_token: str, rom: Rom
+    ):
+        response = client.post(
+            "/api/collections/999999/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_404_NOT_FOUND
+
+    def test_returns_403_for_other_users_collection(
+        self,
+        client,
+        access_token: str,
+        other_user_collection: Collection,
+        rom: Rom,
+    ):
+        response = client.post(
+            f"/api/collections/{other_user_collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_requires_auth(self, client, collection: Collection, rom: Rom):
+        response = client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+        )
+        assert response.status_code in (
+            status.HTTP_401_UNAUTHORIZED,
+            status.HTTP_403_FORBIDDEN,
+        )
+
+    def test_bumps_updated_at(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        original_updated_at = collection.updated_at
+
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        refreshed = db_collection_handler.get_collection(collection.id)
+        assert refreshed is not None
+        assert refreshed.updated_at > original_updated_at
+
+
+# ---------------------------------------------------------------------------
+# DELETE /collections/{id}/roms  —  atomic remove
+# ---------------------------------------------------------------------------
+
+
+class TestRemoveRomsFromCollection:
+    def _seed(self, client, access_token, collection_id, rom_ids):
+        """Helper: add ROMs to a collection before testing removal."""
+        client.post(
+            f"/api/collections/{collection_id}/roms",
+            json={"rom_ids": rom_ids},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+    def test_removes_rom_from_collection(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        self._seed(client, access_token, collection.id, [rom.id])
+
+        response = client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert rom.id not in data["rom_ids"]
+        assert data["rom_count"] == 0
+
+    def test_removes_only_specified_roms(
+        self,
+        client,
+        access_token: str,
+        collection: Collection,
+        rom: Rom,
+        second_rom: Rom,
+    ):
+        self._seed(client, access_token, collection.id, [rom.id, second_rom.id])
+
+        response = client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert rom.id not in data["rom_ids"]
+        assert second_rom.id in data["rom_ids"]
+        assert data["rom_count"] == 1
+
+    def test_removing_absent_rom_is_a_noop(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        """Removing a ROM that isn't in the collection should not raise an error."""
+        response = client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["rom_count"] == 0
+
+    def test_returns_404_for_missing_collection(
+        self, client, access_token: str, rom: Rom
+    ):
+        response = client.delete(
+            "/api/collections/999999/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_404_NOT_FOUND
+
+    def test_returns_403_for_other_users_collection(
+        self,
+        client,
+        access_token: str,
+        other_user_collection: Collection,
+        rom: Rom,
+    ):
+        response = client.delete(
+            f"/api/collections/{other_user_collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_requires_auth(self, client, collection: Collection, rom: Rom):
+        response = client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+        )
+        assert response.status_code in (
+            status.HTTP_401_UNAUTHORIZED,
+            status.HTTP_403_FORBIDDEN,
+        )
+
+    def test_bumps_updated_at(
+        self, client, access_token: str, collection: Collection, rom: Rom
+    ):
+        self._seed(client, access_token, collection.id, [rom.id])
+        seeded = db_collection_handler.get_collection(collection.id)
+        assert seeded is not None
+        updated_at_after_seed = seeded.updated_at
+
+        client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        refreshed = db_collection_handler.get_collection(collection.id)
+        assert refreshed is not None
+        assert refreshed.updated_at > updated_at_after_seed
+
+
+# ---------------------------------------------------------------------------
+# Race condition regression: concurrent adds must not lose data
+# ---------------------------------------------------------------------------
+
+
+class TestAtomicBehavior:
+    def test_sequential_adds_accumulate(
+        self,
+        client,
+        access_token: str,
+        collection: Collection,
+        rom: Rom,
+        second_rom: Rom,
+    ):
+        """
+        Simulates the corrected behavior: two separate add calls, each with a
+        single ROM, should result in both ROMs being present — even if they
+        arrive close together. This would previously fail under the full-replace
+        approach when requests arrived out of order.
+        """
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [second_rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        refreshed = db_collection_handler.get_collection(collection.id)
+        assert refreshed is not None
+        assert set(refreshed.rom_ids) == {rom.id, second_rom.id}
+
+    def test_interleaved_add_remove_stays_consistent(
+        self,
+        client,
+        access_token: str,
+        collection: Collection,
+        rom: Rom,
+        second_rom: Rom,
+    ):
+        """
+        Add both ROMs, remove one, add it back — final state should reflect
+        only the last operation per ROM.
+        """
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id, second_rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        client.delete(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+        client.post(
+            f"/api/collections/{collection.id}/roms",
+            json={"rom_ids": [rom.id]},
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
+
+        refreshed = db_collection_handler.get_collection(collection.id)
+        assert refreshed is not None
+        assert set(refreshed.rom_ids) == {rom.id, second_rom.id}

frontend/src/components/Gallery/FabOverlay.vue

@@ -70,12 +70,12 @@ function resetSelection() {
 
 async function addToFavorites() {
   if (!favoriteCollection.value) return;
-  favoriteCollection.value.rom_ids = favoriteCollection.value.rom_ids.concat(
-    selectedRoms.value.map((r) => r.id),
-  );
+  const romIds = selectedRoms.value.map((r) => r.id);
   await collectionApi
-    .updateCollection({ collection: favoriteCollection.value as Collection })
-    .then(() => {
+    .addRomsToCollection(favoriteCollection.value.id, romIds)
+    .then(({ data }) => {
+      collectionsStore.updateCollection(data);
+      collectionsStore.setFavoriteCollection(data);
       emitter?.emit("snackbarShow", {
         msg: "Roms added to favorites successfully!",
         icon: "mdi-check-bold",
@@ -99,23 +99,21 @@ async function addToFavorites() {
 
 async function removeFromFavorites() {
   if (!favoriteCollection.value) return;
-  favoriteCollection.value.rom_ids = favoriteCollection.value.rom_ids.filter(
-    (value) => !selectedRoms.value.map((r) => r.id).includes(value),
-  );
+  const romIds = selectedRoms.value.map((r) => r.id);
   if (romsStore.currentCollection?.is_favorite) {
     romsStore.remove(selectedRoms.value);
   }
   await collectionApi
-    .updateCollection({ collection: favoriteCollection.value as Collection })
+    .removeRomsFromCollection(favoriteCollection.value.id, romIds)
     .then(({ data }) => {
+      collectionsStore.updateCollection(data);
+      collectionsStore.setFavoriteCollection(data);
       emitter?.emit("snackbarShow", {
         msg: "Roms removed from favorites successfully!",
         icon: "mdi-check-bold",
         color: "green",
         timeout: 2000,
       });
-      favoriteCollection.value = data;
-      collectionsStore.updateCollection(data);
     })
     .catch((error) => {
       console.error(error);

frontend/src/components/common/Collection/Dialog/AddRoms.vue

@@ -7,7 +7,6 @@ import CollectionListItem from "@/components/common/Collection/ListItem.vue";
 import RAvatarCollection from "@/components/common/Collection/RAvatar.vue";
 import RomListItem from "@/components/common/Game/ListItem.vue";
 import RDialog from "@/components/common/RDialog.vue";
-import type { UpdatedCollection } from "@/services/api/collection";
 import collectionApi from "@/services/api/collection";
 import storeCollections from "@/stores/collections";
 import storeRoms, { type SimpleRom } from "@/stores/roms";
@@ -18,7 +17,7 @@ const { mdAndUp } = useDisplay();
 const show = ref(false);
 const romsStore = storeRoms();
 const collectionsStore = storeCollections();
-const selectedCollection = ref<UpdatedCollection>();
+const selectedCollection = ref<ReturnType<typeof collectionsStore.getCollection>>();
 const roms = ref<SimpleRom[]>([]);
 const emitter = inject<Emitter<Events>>("emitter");
 emitter?.on("showAddToCollectionDialog", (romsToAdd) => {
@@ -36,9 +35,9 @@ const HEADERS = [
 
 async function addRomsToCollection() {
   if (!selectedCollection.value) return;
-  selectedCollection.value.rom_ids.push(...roms.value.map((r) => r.id));
+  const romIds = roms.value.map((r) => r.id);
   await collectionApi
-    .updateCollection({ collection: selectedCollection.value })
+    .addRomsToCollection(selectedCollection.value.id, romIds)
     .then(({ data }) => {
       emitter?.emit("snackbarShow", {
         msg: `Roms added to ${selectedCollection.value?.name} successfully!`,

frontend/src/components/common/Collection/Dialog/RemoveRoms.vue

@@ -9,7 +9,6 @@ import RAvatarCollection from "@/components/common/Collection/RAvatar.vue";
 import RomListItem from "@/components/common/Game/ListItem.vue";
 import RDialog from "@/components/common/RDialog.vue";
 import { ROUTES } from "@/plugins/router";
-import type { UpdatedCollection } from "@/services/api/collection";
 import collectionApi from "@/services/api/collection";
 import storeCollections from "@/stores/collections";
 import storeRoms, { type SimpleRom } from "@/stores/roms";
@@ -20,7 +19,7 @@ const { mdAndUp } = useDisplay();
 const show = ref(false);
 const romsStore = storeRoms();
 const collectionsStore = storeCollections();
-const selectedCollection = ref<UpdatedCollection>();
+const selectedCollection = ref<ReturnType<typeof collectionsStore.getCollection>>();
 const roms = ref<SimpleRom[]>([]);
 const router = useRouter();
 const emitter = inject<Emitter<Events>>("emitter");
@@ -39,11 +38,9 @@ const HEADERS = [
 
 async function removeRomsFromCollection() {
   if (!selectedCollection.value) return;
-  selectedCollection.value.rom_ids = selectedCollection.value.rom_ids.filter(
-    (id) => !roms.value.map((r) => r.id).includes(id),
-  );
+  const romIds = roms.value.map((r) => r.id);
   await collectionApi
-    .updateCollection({ collection: selectedCollection.value })
+    .removeRomsFromCollection(selectedCollection.value.id, romIds)
     .then(({ data }) => {
       emitter?.emit("snackbarShow", {
         msg: `Roms removed from ${selectedCollection.value?.name} successfully!`,
@@ -53,6 +50,9 @@ async function removeRomsFromCollection() {
       });
       emitter?.emit("refreshDrawer", null);
       collectionsStore.updateCollection(data);
+      if (data.rom_ids.length === 0) {
+        router.push({ name: ROUTES.HOME });
+      }
     })
     .catch((error) => {
       console.error(error);
@@ -66,12 +66,8 @@ async function removeRomsFromCollection() {
     .finally(() => {
       emitter?.emit("showLoadingDialog", { loading: false, scrim: false });
       romsStore.resetSelection();
-      if (selectedCollection.value?.rom_ids.length == 0) {
-        router.push({ name: ROUTES.HOME });
-      }
       closeDialog();
     });
-  closeDialog();
 }
 
 function closeDialog() {

frontend/src/composables/useFavoriteToggle.ts

@@ -43,24 +43,20 @@ export function useFavoriteToggle(emitter?: Emitter<Events>) {
 
   async function toggleFavorite(rom: SimpleRom) {
     const fav = await ensureFavoriteCollection();
-    if (!fav.rom_ids) (fav as Collection).rom_ids = [] as unknown as number[]; // ensure array exists
-
-    const currentlyFav = fav.rom_ids.includes(rom.id);
-    if (currentlyFav) {
-      fav.rom_ids = fav.rom_ids.filter((id) => id !== rom.id);
-      if (romsStore.currentCollection?.id === fav.id) {
-        romsStore.remove([rom]);
-      }
-    } else {
-      fav.rom_ids.push(rom.id);
-    }
+    const currentlyFav = fav.rom_ids?.includes(rom.id) ?? false;
 
     try {
-      const { data } = await collectionApi.updateCollection({
-        collection: fav as Collection,
-      });
+      const { data } = currentlyFav
+        ? await collectionApi.removeRomsFromCollection(fav.id, [rom.id])
+        : await collectionApi.addRomsToCollection(fav.id, [rom.id]);
+
       collectionsStore.updateCollection(data);
       collectionsStore.setFavoriteCollection(data);
+
+      if (currentlyFav && romsStore.currentCollection?.id === fav.id) {
+        romsStore.remove([rom]);
+      }
+
       emitter?.emit("snackbarShow", {
         msg: `${rom.name} ${currentlyFav ? "removed from" : "added to"} ${data.name} successfully!`,
         icon: "mdi-check-bold",
@@ -68,12 +64,6 @@ export function useFavoriteToggle(emitter?: Emitter<Events>) {
         timeout: 2000,
       });
     } catch (error: unknown) {
-      // Rollback
-      if (currentlyFav) {
-        fav.rom_ids.push(rom.id);
-      } else {
-        fav.rom_ids = fav.rom_ids.filter((id) => id !== rom.id);
-      }
       const detail = (error as { response?: { data?: { detail?: string } } })
         ?.response?.data?.detail;
       emitter?.emit("snackbarShow", {

frontend/src/services/api/collection.ts

@@ -156,6 +156,18 @@ async function updateSmartCollection({
   );
 }
 
+async function addRomsToCollection(collectionId: number, romIds: number[]) {
+  return api.post<Collection>(`/collections/${collectionId}/roms`, {
+    rom_ids: romIds,
+  });
+}
+
+async function removeRomsFromCollection(collectionId: number, romIds: number[]) {
+  return api.delete<Collection>(`/collections/${collectionId}/roms`, {
+    data: { rom_ids: romIds },
+  });
+}
+
 async function deleteCollection({ collection }: { collection: Collection }) {
   return api.delete(`/collections/${collection.id}`);
 }
@@ -172,6 +184,8 @@ export default {
   getCollection,
   getVirtualCollection,
   updateCollection,
+  addRomsToCollection,
+  removeRomsFromCollection,
   deleteCollection,
   getSmartCollections,
   getSmartCollection,