diff --git a/backend/endpoints/collections.py b/backend/endpoints/collections.py index ef153cd77..2b12633e2 100644 --- a/backend/endpoints/collections.py +++ b/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, diff --git a/backend/handler/database/collections_handler.py b/backend/handler/database/collections_handler.py index 4b53b2fb9..ad9eea92a 100644 --- a/backend/handler/database/collections_handler.py +++ b/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, diff --git a/backend/handler/database/roms_handler.py b/backend/handler/database/roms_handler.py index bb0fec2cc..67df73be5 100644 --- a/backend/handler/database/roms_handler.py +++ b/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( diff --git a/backend/tests/endpoints/test_collection.py b/backend/tests/endpoints/test_collection.py new file mode 100644 index 000000000..eb4e2243c --- /dev/null +++ b/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} diff --git a/frontend/src/components/Gallery/FabOverlay.vue b/frontend/src/components/Gallery/FabOverlay.vue index 84cbb8865..5e49ffb4b 100644 --- a/frontend/src/components/Gallery/FabOverlay.vue +++ b/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); diff --git a/frontend/src/components/common/Collection/Dialog/AddRoms.vue b/frontend/src/components/common/Collection/Dialog/AddRoms.vue index ef2c795c8..6a07d367a 100644 --- a/frontend/src/components/common/Collection/Dialog/AddRoms.vue +++ b/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(); +const selectedCollection = ref>(); const roms = ref([]); const emitter = inject>("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!`, diff --git a/frontend/src/components/common/Collection/Dialog/RemoveRoms.vue b/frontend/src/components/common/Collection/Dialog/RemoveRoms.vue index 3bc3bf6a0..7cc04edfc 100644 --- a/frontend/src/components/common/Collection/Dialog/RemoveRoms.vue +++ b/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(); +const selectedCollection = ref>(); const roms = ref([]); const router = useRouter(); const emitter = inject>("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() { diff --git a/frontend/src/composables/useFavoriteToggle.ts b/frontend/src/composables/useFavoriteToggle.ts index a975b5a62..336caeca7 100644 --- a/frontend/src/composables/useFavoriteToggle.ts +++ b/frontend/src/composables/useFavoriteToggle.ts @@ -43,24 +43,20 @@ export function useFavoriteToggle(emitter?: Emitter) { 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) { 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", { diff --git a/frontend/src/services/api/collection.ts b/frontend/src/services/api/collection.ts index ed8748279..11da750cc 100644 --- a/frontend/src/services/api/collection.ts +++ b/frontend/src/services/api/collection.ts @@ -156,6 +156,18 @@ async function updateSmartCollection({ ); } +async function addRomsToCollection(collectionId: number, romIds: number[]) { + return api.post(`/collections/${collectionId}/roms`, { + rom_ids: romIds, + }); +} + +async function removeRomsFromCollection(collectionId: number, romIds: number[]) { + return api.delete(`/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,