diff --git a/backend/app/database/albums.py b/backend/app/database/albums.py index d39cd1ceb..16e2e0951 100644 --- a/backend/app/database/albums.py +++ b/backend/app/database/albums.py @@ -1,13 +1,61 @@ import sqlite3 +from typing import Any, List, Optional, Tuple, TypedDict + import bcrypt from app.config.settings import DATABASE_PATH from app.database.connection import get_db_connection +class AlbumRow(TypedDict): + """A row of the albums table, as the read helpers below return it.""" + + album_id: str + album_name: str + description: Optional[str] + is_locked: bool + password_hash: Optional[str] + cover_image_path: Optional[str] + created_at: Optional[str] + updated_at: Optional[str] + + +def _connect() -> sqlite3.Connection: + conn = sqlite3.connect(DATABASE_PATH) + # Ensure ON DELETE CASCADE and other FKs are enforced + conn.execute("PRAGMA foreign_keys = ON") + return conn + + +# Named once so the SELECTs and the mapper below cannot drift apart. +_ALBUM_COLUMNS = ( + "album_id, album_name, description, is_locked, " + "password_hash, cover_image_path, created_at, updated_at" +) + +# Built once from the column list rather than interpolated at each call site. +_SELECT_ALL_ALBUMS = f"SELECT {_ALBUM_COLUMNS} FROM albums ORDER BY rowid" +_SELECT_ALBUM_BY_NAME = f"SELECT {_ALBUM_COLUMNS} FROM albums WHERE album_name = ?" +_SELECT_ALBUM_BY_ID = f"SELECT {_ALBUM_COLUMNS} FROM albums WHERE album_id = ?" + + +def _to_album_row(row: Tuple[Any, ...]) -> AlbumRow: + """Map a SELECT of _ALBUM_COLUMNS onto a named record.""" + return AlbumRow( + album_id=row[0], + album_name=row[1], + description=row[2], + is_locked=bool(row[3]), + password_hash=row[4], + cover_image_path=row[5], + created_at=row[6], + updated_at=row[7], + ) + + def db_create_albums_table() -> None: conn = None try: - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() cursor.execute( """ @@ -17,18 +65,29 @@ def db_create_albums_table() -> None: description TEXT, is_locked BOOLEAN DEFAULT 0, password_hash TEXT, - cover_image_path TEXT + cover_image_path TEXT, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME DEFAULT CURRENT_TIMESTAMP ) """ ) # Shipped databases predate the is_hidden -> is_locked rename and the - # cover_image_path column, and CREATE IF NOT EXISTS won't add either. + # cover_image_path and created_at columns, and CREATE IF NOT EXISTS + # won't add any of them. cursor.execute("PRAGMA table_info(albums)") columns = {row[1] for row in cursor.fetchall()} if "is_locked" not in columns and "is_hidden" in columns: cursor.execute("ALTER TABLE albums RENAME COLUMN is_hidden TO is_locked") if "cover_image_path" not in columns: cursor.execute("ALTER TABLE albums ADD COLUMN cover_image_path TEXT") + if "created_at" not in columns: + # No default: SQLite rejects a non-constant one on ALTER TABLE, and + # stamping every existing album with the upgrade time would be a + # date that never happened. They stay NULL and read as oldest, + # which their insertion order already reflects. + cursor.execute("ALTER TABLE albums ADD COLUMN created_at DATETIME") + if "updated_at" not in columns: + cursor.execute("ALTER TABLE albums ADD COLUMN updated_at DATETIME") conn.commit() finally: if conn is not None: @@ -38,7 +97,7 @@ def db_create_albums_table() -> None: def db_create_album_images_table() -> None: conn = None try: - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() cursor.execute( """ @@ -63,44 +122,50 @@ def db_create_album_images_table() -> None: conn.close() -def db_get_all_albums(): +def _touch_album(cursor: sqlite3.Cursor, album_id: str) -> None: + """ + Mark an album as changed just now. + + Adding or removing photos counts: to a user, that is the album changing, + not just its name or its lock. + """ + cursor.execute( + "UPDATE albums SET updated_at = CURRENT_TIMESTAMP WHERE album_id = ?", + (album_id,), + ) + + +def db_get_all_albums() -> List[AlbumRow]: """Get all albums (both locked and unlocked).""" - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: - cursor.execute( - "SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums" - ) - albums = cursor.fetchall() - return albums + # Insertion order, so albums predating created_at keep the order they + # were made in rather than an arbitrary one. + cursor.execute(_SELECT_ALL_ALBUMS) + return [_to_album_row(row) for row in cursor.fetchall()] finally: conn.close() -def db_get_album_by_name(name: str): - conn = sqlite3.connect(DATABASE_PATH) +def db_get_album_by_name(name: str) -> Optional[AlbumRow]: + conn = _connect() cursor = conn.cursor() try: - cursor.execute( - "SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums WHERE album_name = ?", - (name,), - ) + cursor.execute(_SELECT_ALBUM_BY_NAME, (name,)) album = cursor.fetchone() - return album if album else None + return _to_album_row(album) if album else None finally: conn.close() -def db_get_album(album_id: str): - conn = sqlite3.connect(DATABASE_PATH) +def db_get_album(album_id: str) -> Optional[AlbumRow]: + conn = _connect() cursor = conn.cursor() try: - cursor.execute( - "SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums WHERE album_id = ?", - (album_id,), - ) + cursor.execute(_SELECT_ALBUM_BY_ID, (album_id,)) album = cursor.fetchone() - return album if album else None + return _to_album_row(album) if album else None finally: conn.close() @@ -110,9 +175,9 @@ def db_insert_album( album_name: str, description: str = "", is_locked: bool = False, - password: str = None, + password: Optional[str] = None, ): - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: password_hash = None @@ -120,10 +185,15 @@ def db_insert_album( password_hash = bcrypt.hashpw( password.encode("utf-8"), bcrypt.gensalt() ).decode("utf-8") + # created_at is set here rather than left to the column default: a + # database migrated with ALTER TABLE has no default to fall back on. cursor.execute( """ - INSERT INTO albums (album_id, album_name, description, is_locked, password_hash) - VALUES (?, ?, ?, ?, ?) + INSERT INTO albums ( + album_id, album_name, description, is_locked, + password_hash, created_at, updated_at + ) + VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) """, (album_id, album_name, description, int(is_locked), password_hash), ) @@ -132,14 +202,45 @@ def db_insert_album( conn.close() +def db_create_album_with_images( + album_id: str, album_name: str, description: str, image_ids: list[str] +) -> int: + """ + Create an album and link its images in a single transaction. + + Both halves commit together, so a failed link never strands an empty album. + Takes image ids rather than the id of whatever they came from, so the + caller owns that choice. Returns the number of images actually linked. + """ + with get_db_connection() as conn: + cursor = conn.cursor() + cursor.execute( + """ + INSERT INTO albums ( + album_id, album_name, description, is_locked, + password_hash, created_at, updated_at + ) + VALUES (?, ?, ?, 0, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + """, + (album_id, album_name, description), + ) + # Foreign keys are on for this connection, so an image id that no + # longer exists rolls the album back with it rather than half-writing. + cursor.executemany( + "INSERT OR IGNORE INTO album_images (album_id, image_id) VALUES (?, ?)", + [(album_id, image_id) for image_id in image_ids], + ) + return cursor.rowcount + + def db_update_album( album_id: str, album_name: str, description: str, is_locked: bool, - password: str = None, + password: Optional[str] = None, ): - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: if password is not None: @@ -150,7 +251,8 @@ def db_update_album( cursor.execute( """ UPDATE albums - SET album_name = ?, description = ?, is_locked = ?, password_hash = ? + SET album_name = ?, description = ?, is_locked = ?, password_hash = ?, + updated_at = CURRENT_TIMESTAMP WHERE album_id = ? """, (album_name, description, int(is_locked), password_hash, album_id), @@ -160,7 +262,8 @@ def db_update_album( cursor.execute( """ UPDATE albums - SET album_name = ?, description = ?, is_locked = ? + SET album_name = ?, description = ?, is_locked = ?, + updated_at = CURRENT_TIMESTAMP WHERE album_id = ? """, (album_name, description, int(is_locked), album_id), @@ -178,7 +281,7 @@ def db_delete_album(album_id: str): def db_get_album_cover_path(album_id: str) -> str | None: """Path of the album's cover: its first image, by insertion order.""" - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: cursor.execute( @@ -199,7 +302,7 @@ def db_get_album_cover_path(album_id: str) -> str | None: def db_get_album_images(album_id: str): - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: cursor.execute( @@ -247,6 +350,11 @@ def db_add_images_to_album(album_id: str, image_ids: list[str]): "INSERT OR IGNORE INTO album_images (album_id, image_id) VALUES (?, ?)", [(album_id, img_id) for img_id in valid_images], ) + # Every id may already be in the album, in which case OR IGNORE writes + # nothing and the album has not actually changed. Read before touching: + # the touch overwrites rowcount. + if cursor.rowcount: + _touch_album(cursor, album_id) conn.commit() @@ -265,25 +373,29 @@ def db_remove_image_from_album(album_id: str, image_id: str): "DELETE FROM album_images WHERE album_id = ? AND image_id = ?", (album_id, image_id), ) + _touch_album(cursor, album_id) else: raise ValueError("Image not found in the specified album") def db_remove_images_from_album(album_id: str, image_ids: list[str]): - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: cursor.executemany( "DELETE FROM album_images WHERE album_id = ? AND image_id = ?", [(album_id, img_id) for img_id in image_ids], ) + # Same as the insert: ids that were not in the album delete nothing. + if cursor.rowcount: + _touch_album(cursor, album_id) conn.commit() finally: conn.close() def verify_album_password(album_id: str, password: str) -> bool: - conn = sqlite3.connect(DATABASE_PATH) + conn = _connect() cursor = conn.cursor() try: cursor.execute( diff --git a/backend/app/routes/albums.py b/backend/app/routes/albums.py index adecc6afc..fdbed2378 100644 --- a/backend/app/routes/albums.py +++ b/backend/app/routes/albums.py @@ -4,6 +4,10 @@ GetAlbumsResponse, CreateAlbumRequest, CreateAlbumResponse, + CreateAlbumFromMemoryData, + CreateAlbumFromMemoryRequest, + CreateAlbumFromMemoryResponse, + ErrorResponseEnvelope, GetAlbumResponse, GetAlbumImagesRequest, GetAlbumImagesResponse, @@ -27,10 +31,36 @@ db_get_album_cover_path, verify_album_password, ) +from app.utils.albums import ( + AlbumNameTakenError, + MemoryHasNoPhotosError, + MemoryNotFoundError, + album_util_create_from_memory, +) router = APIRouter() +def _album_exists(name: str) -> HTTPException: + return HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=ErrorResponse( + success=False, + error="Album Already Exists", + message=f"Album '{name}' is already in the database.", + ).model_dump(), + ) + + +def _internal_error(message: str) -> HTTPException: + return HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=ErrorResponse( + success=False, error="Internal Server Error", message=message + ).model_dump(), + ) + + # GET /albums/ - Get all albums (including locked ones) @router.get("/", response_model=GetAlbumsResponse) def get_albums(): @@ -39,22 +69,24 @@ def get_albums(): album_list = [] for album in albums: # Get image count for each album - image_ids = db_get_album_images(album[0]) + image_ids = db_get_album_images(album["album_id"]) image_count = len(image_ids) - is_locked = bool(album[3]) + is_locked = album["is_locked"] album_list.append( Album( - album_id=album[0], - album_name=album[1], - description=album[2] or "", + album_id=album["album_id"], + album_name=album["album_name"], + description=album["description"] or "", is_locked=is_locked, # A locked album's cover would show the very content the # password is protecting, so never send it. cover_image_path=( - None if is_locked else db_get_album_cover_path(album[0]) + None if is_locked else db_get_album_cover_path(album["album_id"]) ), image_count=image_count, + created_at=album["created_at"], + updated_at=album["updated_at"], ) ) return GetAlbumsResponse(success=True, albums=album_list) @@ -65,14 +97,7 @@ def get_albums(): def create_album(body: CreateAlbumRequest): existing_album = db_get_album_by_name(body.name) if existing_album: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=ErrorResponse( - success=False, - error="Album Already Exists", - message=f"Album '{body.name}' is already in the database.", - ).model_dump(), - ) + raise _album_exists(body.name) album_id = str(uuid.uuid4()) try: @@ -81,14 +106,55 @@ def create_album(body: CreateAlbumRequest): ) return CreateAlbumResponse(success=True, album_id=album_id) except Exception as e: + raise _internal_error(f"Failed to create album: {e}") from e + + +# POST /albums/from-memory - Create an album from a curated memory +@router.post( + "/from-memory", + response_model=CreateAlbumFromMemoryResponse, + responses={code: {"model": ErrorResponseEnvelope} for code in [400, 404, 409, 500]}, +) +def create_album_from_memory( + body: CreateAlbumFromMemoryRequest, +) -> CreateAlbumFromMemoryResponse: + """ + Copy a memory's photos into a new album. + + The memory itself is left untouched, so the same one can be converted + again under a different name. Any clips are left behind: album_images + references images, and albums have no video support. + """ + try: + result = album_util_create_from_memory(body.memory_id, body.name) + except MemoryNotFoundError as e: raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + status_code=status.HTTP_404_NOT_FOUND, detail=ErrorResponse( success=False, - error="Internal Server Error", - message=f"Failed to create album: {str(e)}", + error="Memory Not Found", + message="No memory exists with the provided ID.", ).model_dump(), - ) + ) from e + except MemoryHasNoPhotosError as e: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Empty Memory", + message="This memory has no photos to convert.", + ).model_dump(), + ) from e + except AlbumNameTakenError as e: + raise _album_exists(body.name) from e + except Exception as e: + raise _internal_error(f"Failed to create album from memory: {e}") from e + + return CreateAlbumFromMemoryResponse( + success=True, + message=f"Created album '{body.name}' with {result['image_count']} photos", + data=CreateAlbumFromMemoryData(**result), + ) # GET /albums/{album_id} - Get specific album details @@ -108,15 +174,17 @@ def get_album(album_id: str = Path(...)): image_ids = db_get_album_images(album_id) image_count = len(image_ids) - is_locked = bool(album[3]) + is_locked = album["is_locked"] album_obj = Album( - album_id=album[0], - album_name=album[1], - description=album[2] or "", + album_id=album["album_id"], + album_name=album["album_name"], + description=album["description"] or "", is_locked=is_locked, # Same reasoning as the listing: the cover gives away the contents. cover_image_path=(None if is_locked else db_get_album_cover_path(album_id)), image_count=image_count, + created_at=album["created_at"], + updated_at=album["updated_at"], ) return GetAlbumResponse(success=True, data=album_obj) except Exception as e: @@ -144,15 +212,7 @@ def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...) ).model_dump(), ) - album_dict = { - "album_id": album[0], - "album_name": album[1], - "description": album[2], - "is_locked": bool(album[3]), - "password_hash": album[4], - } - - if album_dict["is_locked"]: + if album["is_locked"]: if not body.current_password: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -232,15 +292,7 @@ def get_album_images( ).model_dump(), ) - album_dict = { - "album_id": album[0], - "album_name": album[1], - "description": album[2], - "is_locked": bool(album[3]), - "password_hash": album[4], - } - - if album_dict["is_locked"]: + if album["is_locked"]: if not body.password: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/backend/app/schemas/album.py b/backend/app/schemas/album.py index cced4adbe..9a6c7b471 100644 --- a/backend/app/schemas/album.py +++ b/backend/app/schemas/album.py @@ -10,6 +10,10 @@ class Album(BaseModel): is_locked: bool cover_image_path: Optional[str] = None image_count: int = 0 + # Null for albums that predate these columns; they read as oldest. + created_at: Optional[str] = None + # Touched by metadata edits and by adding or removing photos. + updated_at: Optional[str] = None # ############################## @@ -30,6 +34,20 @@ def check_password(cls, value, info: ValidationInfo): return value +class CreateAlbumFromMemoryRequest(BaseModel): + memory_id: str = Field(..., min_length=1) + name: str = Field(..., min_length=1) + + @field_validator("memory_id", "name") + def check_not_blank(cls, value: str) -> str: + # min_length counts the spaces, so " " would otherwise get through + # and create an album with a blank name. + cleaned = value.strip() + if not cleaned: + raise ValueError("must not be blank") + return cleaned + + class UpdateAlbumRequest(BaseModel): name: str description: Optional[str] = "" @@ -85,6 +103,19 @@ class CreateAlbumResponse(BaseModel): album_id: str +class CreateAlbumFromMemoryData(BaseModel): + album_id: str + image_count: int + + +class CreateAlbumFromMemoryResponse(BaseModel): + # The {success, message, data} envelope the project standardised on, unlike + # the flat responses above that predate it. + success: bool + message: str + data: CreateAlbumFromMemoryData + + class GetAlbumResponse(BaseModel): success: bool data: Album @@ -104,3 +135,14 @@ class ErrorResponse(BaseModel): success: bool = False message: str error: str + + +class ErrorResponseEnvelope(BaseModel): + """ + How an ErrorResponse actually reaches the client. + + HTTPException nests whatever it is given under `detail`, so documenting + ErrorResponse alone would describe a shape no client ever receives. + """ + + detail: ErrorResponse diff --git a/backend/app/utils/albums.py b/backend/app/utils/albums.py new file mode 100644 index 000000000..fe0e92986 --- /dev/null +++ b/backend/app/utils/albums.py @@ -0,0 +1,66 @@ +"""Album workflows that reach across more than one table.""" + +import sqlite3 +import uuid +from typing import TypedDict + +from app.database.albums import db_create_album_with_images, db_get_album_by_name +from app.database.memories import db_get_memory, db_get_memory_images + + +class AlbumFromMemoryResult(TypedDict): + album_id: str + image_count: int + + +class AlbumFromMemoryError(Exception): + """Base for the ways a conversion can legitimately fail.""" + + +class MemoryNotFoundError(AlbumFromMemoryError): + pass + + +class MemoryHasNoPhotosError(AlbumFromMemoryError): + pass + + +class AlbumNameTakenError(AlbumFromMemoryError): + pass + + +def album_util_create_from_memory(memory_id: str, name: str) -> AlbumFromMemoryResult: + """ + Copy a memory's photos into a new album. + + The memory itself is left untouched, so the same one can be converted + again under a different name. Any clips are left behind: album_images + references images, and albums have no video support. + + Raises AlbumFromMemoryError subclasses for the expected failures, so the + caller decides how to report them. + """ + memory = db_get_memory(memory_id) + if not memory: + raise MemoryNotFoundError(memory_id) + + image_ids = [image["id"] for image in db_get_memory_images(memory_id)] + if not image_ids: + raise MemoryHasNoPhotosError(memory_id) + + if db_get_album_by_name(name): + raise AlbumNameTakenError(name) + + album_id = str(uuid.uuid4()) + try: + image_count = db_create_album_with_images( + album_id, name, memory.get("subtitle") or "", image_ids + ) + except sqlite3.IntegrityError as e: + # The name check above is not atomic. Re-check rather than assume a + # conflict: the same error covers an image that vanished mid-request. + if db_get_album_by_name(name): + raise AlbumNameTakenError(name) from e + raise + + return AlbumFromMemoryResult(album_id=album_id, image_count=image_count) diff --git a/backend/tests/test_album_utils.py b/backend/tests/test_album_utils.py new file mode 100644 index 000000000..7357f1350 --- /dev/null +++ b/backend/tests/test_album_utils.py @@ -0,0 +1,45 @@ +import sqlite3 +from unittest.mock import patch + +import pytest + +from app.utils.albums import ( + AlbumFromMemoryResult, + AlbumNameTakenError, + album_util_create_from_memory, +) + + +class TestAlbumFromMemoryRace: + """ + The name check is not atomic, so the insert can still hit the UNIQUE + constraint. These cover the branch that decides what that meant. + """ + + def run_with_integrity_error( + self, name_taken_on_recheck: bool + ) -> AlbumFromMemoryResult: + with patch("app.utils.albums.db_get_memory") as mock_get_memory, patch( + "app.utils.albums.db_get_memory_images" + ) as mock_get_images, patch( + "app.utils.albums.db_get_album_by_name" + ) as mock_get_by_name, patch( + "app.utils.albums.db_create_album_with_images" + ) as mock_create: + mock_get_memory.return_value = {"memory_id": "mem-1", "subtitle": "July"} + mock_get_images.return_value = [{"id": "img-1"}] + # Free when first checked, then possibly taken by the time it fails. + recheck = {"album_id": "other"} if name_taken_on_recheck else None + mock_get_by_name.side_effect = [None, recheck] + mock_create.side_effect = sqlite3.IntegrityError("UNIQUE constraint failed") + + return album_util_create_from_memory("mem-1", "Paris 2022") + + def test_a_name_taken_mid_request_reads_as_a_conflict(self) -> None: + with pytest.raises(AlbumNameTakenError): + self.run_with_integrity_error(name_taken_on_recheck=True) + + def test_any_other_integrity_error_is_not_swallowed(self) -> None: + """A vanished image must not be reported as a duplicate name.""" + with pytest.raises(sqlite3.IntegrityError): + self.run_with_integrity_error(name_taken_on_recheck=False) diff --git a/backend/tests/test_albums.py b/backend/tests/test_albums.py index b24cb7376..3f1a524ff 100644 --- a/backend/tests/test_albums.py +++ b/backend/tests/test_albums.py @@ -6,6 +6,9 @@ from fastapi.testclient import TestClient from unittest.mock import patch import uuid +from typing import Any, Optional + +from app.database.albums import AlbumRow from app.routes import albums as albums_router sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) @@ -20,6 +23,30 @@ # ############################## +def album_row( + album: dict[str, Any], + cover_image_path: Optional[str] = None, + created_at: Optional[str] = None, + updated_at: Optional[str] = None, +) -> AlbumRow: + """ + An albums row as the database helpers return it. + + Built as the production AlbumRow rather than a look-alike dict, so a + column added there fails here instead of drifting silently. + """ + return AlbumRow( + album_id=album["album_id"], + album_name=album["album_name"], + description=album["description"], + is_locked=album["is_locked"], + password_hash=album["password_hash"], + cover_image_path=cover_image_path, + created_at=created_at, + updated_at=updated_at, + ) + + @pytest.fixture def mock_db_album(): return { @@ -31,6 +58,24 @@ def mock_db_album(): } +@pytest.fixture +def mock_memory() -> dict[str, Any]: + """A memories row as db_get_memory returns it, trimmed to what the route reads.""" + return { + "memory_id": str(uuid.uuid4()), + "title": "3 years ago in Paris", + "subtitle": "July 2022 · Paris", + } + + +@pytest.fixture +def mock_memory_images() -> list[dict[str, Any]]: + return [ + {"id": str(uuid.uuid4()), "sort_order": 0}, + {"id": str(uuid.uuid4()), "sort_order": 1}, + ] + + @pytest.fixture def mock_db_locked_album(): return { @@ -96,12 +141,14 @@ def test_create_album_duplicate_name(self): } with patch("app.routes.albums.db_get_album_by_name") as mock_get_by_name: - mock_get_by_name.return_value = ( - "existing-id", - "Existing Album", - "desc", - 0, - None, + mock_get_by_name.return_value = album_row( + { + "album_id": "existing-id", + "album_name": "Existing Album", + "description": "desc", + "is_locked": False, + "password_hash": None, + } ) response = client.post("/albums/", json=album_data) @@ -117,13 +164,10 @@ def test_get_all_albums_public_only(self, mock_db_album): """ with patch("app.routes.albums.db_get_all_albums") as mock_get_all: mock_get_all.return_value = [ - ( - mock_db_album["album_id"], - mock_db_album["album_name"], - mock_db_album["description"], - mock_db_album["is_locked"], - mock_db_album["password_hash"], - None, # cover_image_path + album_row( + mock_db_album, + created_at="2026-08-01 09:00:00", + updated_at="2026-08-05 09:00:00", ) ] @@ -152,21 +196,15 @@ def test_get_all_albums_include_hidden(self, mock_db_album, mock_db_locked_album """ with patch("app.routes.albums.db_get_all_albums") as mock_get_all: mock_get_all.return_value = [ - ( - mock_db_album["album_id"], - mock_db_album["album_name"], - mock_db_album["description"], - mock_db_album["is_locked"], - mock_db_album["password_hash"], - None, # cover_image_path + album_row( + mock_db_album, + created_at="2026-08-01 09:00:00", + updated_at="2026-08-05 09:00:00", ), - ( - mock_db_locked_album["album_id"], - mock_db_locked_album["album_name"], - mock_db_locked_album["description"], - mock_db_locked_album["is_locked"], - mock_db_locked_album["password_hash"], - None, # cover_image_path + album_row( + mock_db_locked_album, + created_at="2026-08-02 09:00:00", + updated_at="2026-08-06 09:00:00", ), ] @@ -190,21 +228,15 @@ def test_locked_album_cover_is_withheld(self, mock_db_album, mock_db_locked_albu "app.routes.albums.db_get_album_cover_path" ) as mock_cover: mock_get_all.return_value = [ - ( - mock_db_album["album_id"], - mock_db_album["album_name"], - mock_db_album["description"], - mock_db_album["is_locked"], - mock_db_album["password_hash"], - None, + album_row( + mock_db_album, + created_at="2026-08-01 09:00:00", + updated_at="2026-08-05 09:00:00", ), - ( - mock_db_locked_album["album_id"], - mock_db_locked_album["album_name"], - mock_db_locked_album["description"], - mock_db_locked_album["is_locked"], - mock_db_locked_album["password_hash"], - None, + album_row( + mock_db_locked_album, + created_at="2026-08-02 09:00:00", + updated_at="2026-08-06 09:00:00", ), ] mock_cover.return_value = "/photos/secret.jpg" @@ -226,13 +258,10 @@ def test_get_album_by_id_withholds_a_locked_cover(self, mock_db_locked_album): with patch("app.routes.albums.db_get_album") as mock_get_album, patch( "app.routes.albums.db_get_album_cover_path" ) as mock_cover: - mock_get_album.return_value = ( - mock_db_locked_album["album_id"], - mock_db_locked_album["album_name"], - mock_db_locked_album["description"], - mock_db_locked_album["is_locked"], - mock_db_locked_album["password_hash"], - None, + mock_get_album.return_value = album_row( + mock_db_locked_album, + created_at="2026-08-02 09:00:00", + updated_at="2026-08-06 09:00:00", ) mock_cover.return_value = "/photos/secret.jpg" @@ -262,13 +291,10 @@ def test_get_album_by_id_success(self, mock_db_album): Test fetching a single album by its ID successfully. """ with patch("app.routes.albums.db_get_album") as mock_get_album: - mock_get_album.return_value = ( - mock_db_album["album_id"], - mock_db_album["album_name"], - mock_db_album["description"], - mock_db_album["is_locked"], - mock_db_album["password_hash"], - None, # cover_image_path + mock_get_album.return_value = album_row( + mock_db_album, + created_at="2026-08-01 09:00:00", + updated_at="2026-08-05 09:00:00", ) response = client.get(f"/albums/{mock_db_album['album_id']}") @@ -305,7 +331,15 @@ def test_get_album_by_id_not_found(self): [ # Case 1: Public album (no password protection) ( - ("abc-123", "Old Name", "Old Desc", 0, None, 0), + album_row( + { + "album_id": "abc-123", + "album_name": "Old Name", + "description": "Old Desc", + "is_locked": False, + "password_hash": None, + } + ), { "name": "Updated Public Album", "description": "Updated description", @@ -318,13 +352,16 @@ def test_get_album_by_id_not_found(self): ), # Case 2: Locked album with correct current password ( - ( - "abc-456", - "Locked Album", - "Secret", - 1, - bcrypt.hashpw("oldpass".encode(), bcrypt.gensalt()).decode(), - 0, + album_row( + { + "album_id": "abc-456", + "album_name": "Locked Album", + "description": "Secret", + "is_locked": True, + "password_hash": bcrypt.hashpw( + b"oldpass", bcrypt.gensalt() + ).decode(), + } ), { "name": "Updated Locked Album", @@ -338,13 +375,16 @@ def test_get_album_by_id_not_found(self): ), # Case 3: Locked album with incorrect current password ( - ( - "abc-789", - "Locked Album", - "Secret", - 1, - bcrypt.hashpw("correctpass".encode(), bcrypt.gensalt()).decode(), - 0, + album_row( + { + "album_id": "abc-789", + "album_name": "Locked Album", + "description": "Secret", + "is_locked": True, + "password_hash": bcrypt.hashpw( + b"correctpass", bcrypt.gensalt() + ).decode(), + } ), { "name": "Invalid Attempt", @@ -369,7 +409,9 @@ def test_update_album( mock_get_album.return_value = album_data mock_verify.return_value = verify_password_return - response = client.put(f"/albums/{album_data[0]}", json=request_data) + response = client.put( + f"/albums/{album_data['album_id']}", json=request_data + ) assert response.status_code == expected_status if expected_status == 200: @@ -384,14 +426,7 @@ def test_delete_album_success(self, mock_db_album): Test successfully deleting an existing album. """ album_id = mock_db_album["album_id"] - album_tuple = ( - album_id, - mock_db_album["album_name"], - mock_db_album["description"], - int(mock_db_album["is_locked"]), - mock_db_album["password_hash"], - 0, # image_count - ) + album_tuple = album_row(mock_db_album) with patch("app.routes.albums.db_get_album") as mock_get_album, patch( "app.routes.albums.db_delete_album" @@ -426,14 +461,7 @@ def test_add_images_to_album_success(self, mock_db_album): ] } - album_tuple = ( - album_id, - mock_db_album["album_name"], - mock_db_album["description"], - int(mock_db_album["is_locked"]), - mock_db_album["password_hash"], - 0, # image_count - ) + album_tuple = album_row(mock_db_album) with patch("app.routes.albums.db_get_album") as mock_get_album, patch( "app.routes.albums.db_add_images_to_album" @@ -462,14 +490,7 @@ def test_get_album_images_success(self, mock_db_album): "2d4bff29-1111-43a4-9e76-b78504bea999", ] - album_tuple = ( - album_id, - mock_db_album["album_name"], - mock_db_album["description"], - int(mock_db_album["is_locked"]), - mock_db_album["password_hash"], - 0, # image_count - ) + album_tuple = album_row(mock_db_album) with patch("app.routes.albums.db_get_album") as mock_get_album, patch( "app.routes.albums.db_get_album_images" @@ -495,14 +516,7 @@ def test_remove_image_from_album_success(self, mock_db_album): album_id = mock_db_album["album_id"] image_id = "71abff29-27b4-43a4-9e76-b78504bea325" - album_tuple = ( - album_id, - mock_db_album["album_name"], - mock_db_album["description"], - int(mock_db_album["is_locked"]), - mock_db_album["password_hash"], - 0, # image_count - ) + album_tuple = album_row(mock_db_album) with patch("app.routes.albums.db_get_album") as mock_get_album, patch( "app.routes.albums.db_remove_image_from_album" @@ -531,7 +545,7 @@ def test_remove_multiple_images_from_album(self, mock_db_album): with patch("app.routes.albums.db_get_album") as mock_get, patch( "app.routes.albums.db_remove_images_from_album" ) as mock_remove_bulk: - mock_get.return_value = tuple(mock_db_album.values()) + mock_get.return_value = album_row(mock_db_album) response = client.request( "DELETE", f"/albums/{album_id}/images", json=image_ids_to_remove ) @@ -543,3 +557,140 @@ def test_remove_multiple_images_from_album(self, mock_db_album): mock_remove_bulk.assert_called_once_with( album_id, image_ids_to_remove["image_ids"] ) + + +class TestCreateAlbumFromMemory: + """Test suite for converting a curated memory into an album.""" + + def test_create_album_from_memory_success( + self, mock_memory: dict[str, Any], mock_memory_images: list[dict[str, Any]] + ) -> None: + with patch("app.utils.albums.db_get_memory") as mock_get_memory, patch( + "app.utils.albums.db_get_memory_images" + ) as mock_get_images, patch( + "app.utils.albums.db_get_album_by_name" + ) as mock_get_by_name, patch( + "app.utils.albums.db_create_album_with_images" + ) as mock_create: + mock_get_memory.return_value = mock_memory + mock_get_images.return_value = mock_memory_images + mock_get_by_name.return_value = None + mock_create.return_value = len(mock_memory_images) + + response = client.post( + "/albums/from-memory", + json={"memory_id": mock_memory["memory_id"], "name": "Paris 2022"}, + ) + assert response.status_code == 200 + + json_response = response.json() + assert json_response["success"] is True + assert json_response["data"]["image_count"] == len(mock_memory_images) + uuid.UUID(json_response["data"]["album_id"]) + + # The memory's subtitle becomes the album description, and only the + # image ids are handed over - clips are left behind. + album_id, name, description, image_ids = mock_create.call_args.args + # The id that was persisted has to be the one handed back, or the + # client navigates to an album that does not exist. + assert album_id == json_response["data"]["album_id"] + assert name == "Paris 2022" + assert description == mock_memory["subtitle"] + assert image_ids == [image["id"] for image in mock_memory_images] + + @pytest.mark.parametrize( + "payload", + [ + {"memory_id": "mem-1", "name": " "}, + {"memory_id": " ", "name": "Paris 2022"}, + ], + ) + def test_create_album_from_memory_rejects_blank_fields( + self, payload: dict[str, str] + ) -> None: + """A name of spaces satisfies min_length but is not a name.""" + with patch("app.utils.albums.db_create_album_with_images") as mock_create: + response = client.post("/albums/from-memory", json=payload) + + assert response.status_code == 422 + mock_create.assert_not_called() + + def test_create_album_from_memory_trims_the_name( + self, mock_memory: dict[str, Any], mock_memory_images: list[dict[str, Any]] + ) -> None: + with patch("app.utils.albums.db_get_memory") as mock_get_memory, patch( + "app.utils.albums.db_get_memory_images" + ) as mock_get_images, patch( + "app.utils.albums.db_get_album_by_name" + ) as mock_get_by_name, patch( + "app.utils.albums.db_create_album_with_images" + ) as mock_create: + mock_get_memory.return_value = mock_memory + mock_get_images.return_value = mock_memory_images + mock_get_by_name.return_value = None + mock_create.return_value = len(mock_memory_images) + + response = client.post( + "/albums/from-memory", + json={"memory_id": mock_memory["memory_id"], "name": " Paris "}, + ) + + assert response.status_code == 200 + assert mock_create.call_args.args[1] == "Paris" + + def test_create_album_from_memory_not_found(self) -> None: + with patch("app.utils.albums.db_get_memory") as mock_get_memory: + mock_get_memory.return_value = None + + response = client.post( + "/albums/from-memory", + json={"memory_id": str(uuid.uuid4()), "name": "Paris 2022"}, + ) + assert response.status_code == 404 + assert response.json()["detail"]["error"] == "Memory Not Found" + + def test_create_album_from_empty_memory(self, mock_memory: dict[str, Any]) -> None: + """A memory with no photos cannot become an album.""" + with patch("app.utils.albums.db_get_memory") as mock_get_memory, patch( + "app.utils.albums.db_get_memory_images" + ) as mock_get_images, patch( + "app.utils.albums.db_create_album_with_images" + ) as mock_create: + mock_get_memory.return_value = mock_memory + mock_get_images.return_value = [] + + response = client.post( + "/albums/from-memory", + json={"memory_id": mock_memory["memory_id"], "name": "Paris 2022"}, + ) + assert response.status_code == 400 + assert response.json()["detail"]["error"] == "Empty Memory" + mock_create.assert_not_called() + + def test_create_album_from_memory_duplicate_name( + self, + mock_memory: dict[str, Any], + mock_memory_images: list[dict[str, Any]], + mock_db_album: dict[str, Any], + ) -> None: + with patch("app.utils.albums.db_get_memory") as mock_get_memory, patch( + "app.utils.albums.db_get_memory_images" + ) as mock_get_images, patch( + "app.utils.albums.db_get_album_by_name" + ) as mock_get_by_name, patch( + "app.utils.albums.db_create_album_with_images" + ) as mock_create: + mock_get_memory.return_value = mock_memory + mock_get_images.return_value = mock_memory_images + mock_get_by_name.return_value = album_row(mock_db_album) + + response = client.post( + "/albums/from-memory", + json={ + "memory_id": mock_memory["memory_id"], + "name": mock_db_album["album_name"], + }, + ) + assert response.status_code == 409 + assert response.json()["detail"]["error"] == "Album Already Exists" + mock_create.assert_not_called() diff --git a/backend/tests/test_albums_db.py b/backend/tests/test_albums_db.py index b494a6329..f3db472e0 100644 --- a/backend/tests/test_albums_db.py +++ b/backend/tests/test_albums_db.py @@ -10,6 +10,7 @@ from app.database.albums import ( db_create_albums_table, db_create_album_images_table, + db_create_album_with_images, db_get_all_albums, db_get_album_by_name, db_get_album, @@ -17,6 +18,8 @@ db_update_album, db_delete_album, db_get_album_images, + db_add_images_to_album, + db_remove_image_from_album, db_remove_images_from_album, db_get_album_cover_path, verify_album_password, @@ -149,7 +152,10 @@ def test_migrates_a_legacy_is_hidden_schema(self, test_db): assert "is_hidden" not in columns assert {"is_locked", "cover_image_path"} <= set(columns) # the legacy row survives, its flag carried over under the new name - assert [row[1:4] for row in db_get_all_albums()] == [("Old", None, 1)] + legacy = db_get_all_albums() + assert [ + (row["album_name"], row["description"], row["is_locked"]) for row in legacy + ] == [("Old", None, True)] @pytest.mark.parametrize( "create_table", [db_create_albums_table, db_create_album_images_table] @@ -181,7 +187,9 @@ def test_insert_then_fetch_by_id_and_name(self, test_db): make_album("album-1", "Summer Trip", "Fun times") by_id = db_get_album("album-1") - assert by_id[:3] == ("album-1", "Summer Trip", "Fun times") + assert by_id["album_id"] == "album-1" + assert by_id["album_name"] == "Summer Trip" + assert by_id["description"] == "Fun times" assert db_get_album_by_name("Summer Trip") == by_id def test_missing_album_returns_none(self, test_db): @@ -200,8 +208,11 @@ def test_get_all_albums_returns_locked_albums_too(self, test_db): make_album("album-2", "Secret", locked=True, password="pw") rows = db_get_all_albums() - assert sorted(row[1] for row in rows) == ["Public", "Secret"] - assert {row[1]: row[3] for row in rows} == {"Public": 0, "Secret": 1} + assert sorted(row["album_name"] for row in rows) == ["Public", "Secret"] + assert {row["album_name"]: row["is_locked"] for row in rows} == { + "Public": False, + "Secret": True, + } def test_update_changes_fields(self, test_db): make_album("album-1", "Old", "Old desc") @@ -209,9 +220,9 @@ def test_update_changes_fields(self, test_db): db_update_album("album-1", "New", "New desc", True, None) album = db_get_album("album-1") - assert album[1] == "New" - assert album[2] == "New desc" - assert album[3] == 1 + assert album["album_name"] == "New" + assert album["description"] == "New desc" + assert album["is_locked"] is True def test_delete_removes_the_row(self, test_db): make_album("album-1", "Trip") @@ -244,6 +255,204 @@ def test_remove_images_drops_only_the_named_ones(self, test_db): assert db_get_album_images("album-1") == ["img-2"] +class TestAlbumCreatedAt: + def test_insert_stamps_a_creation_time(self, test_db): + make_album("album-1", "Trip") + + assert db_get_album("album-1")["created_at"] is not None + + def test_create_with_images_stamps_a_creation_time(self, test_db): + make_images(test_db, ["img-1"]) + + db_create_album_with_images("album-1", "Paris", "", ["img-1"]) + + assert db_get_album("album-1")["created_at"] is not None + + def test_edits_never_change_it(self, test_db): + """Renaming, re-describing or adding photos is not a new creation.""" + make_album("album-1", "Trip") + make_images(test_db, ["img-1"]) + created = db_get_album("album-1")["created_at"] + + db_update_album("album-1", "Trip Renamed", "A new description", False) + db_add_images_to_album("album-1", ["img-1"]) + + album = db_get_album("album-1") + assert album["created_at"] == created + # ...but all of that does count as an update. + assert album["updated_at"] >= created + + def test_listing_keeps_insertion_order(self, test_db): + """Albums predating created_at sort by when they were made.""" + make_album("album-1", "First") + make_album("album-2", "Second") + + assert [album["album_id"] for album in db_get_all_albums()] == [ + "album-1", + "album-2", + ] + + def test_migration_adds_the_column_to_a_legacy_table(self, test_db): + """A database shipped before created_at existed still upgrades.""" + conn = sqlite3.connect(test_db) + conn.execute("DROP TABLE albums") + conn.execute( + """ + CREATE TABLE albums ( + album_id TEXT PRIMARY KEY, + album_name TEXT UNIQUE, + description TEXT, + is_hidden BOOLEAN DEFAULT 0, + password_hash TEXT + ) + """ + ) + conn.execute( + "INSERT INTO albums (album_id, album_name) VALUES ('old-1', 'Old')" + ) + conn.commit() + conn.close() + + db_create_albums_table() + + old = db_get_album("old-1") + assert old["album_name"] == "Old" + # Nothing to backfill it with, so it reads as oldest. + assert old["created_at"] is None + # And the table still takes new albums. + make_album("album-1", "New") + assert db_get_album("album-1")["created_at"] is not None + + +class TestAlbumUpdatedAt: + """ + updated_at is compared against a pinned earlier value rather than against + a timestamp taken during the test: CURRENT_TIMESTAMP has one-second + resolution, so two writes in the same second read as equal. + """ + + EARLIER = "2020-01-01 00:00:00" + + def set_updated_at(self, db_path: str, album_id: str) -> None: + conn = sqlite3.connect(db_path) + conn.execute( + "UPDATE albums SET updated_at = ? WHERE album_id = ?", + (self.EARLIER, album_id), + ) + conn.commit() + conn.close() + + def test_insert_stamps_an_update_time(self, test_db): + make_album("album-1", "Trip") + + assert db_get_album("album-1")["updated_at"] is not None + + def test_editing_the_album_touches_it(self, test_db): + make_album("album-1", "Trip") + self.set_updated_at(test_db, "album-1") + + db_update_album("album-1", "Trip Renamed", "", False) + + assert db_get_album("album-1")["updated_at"] > self.EARLIER + + def test_adding_images_touches_it(self, test_db): + """Adding photos is the commonest way an album changes.""" + make_album("album-1", "Trip") + make_images(test_db, ["img-1"]) + self.set_updated_at(test_db, "album-1") + + db_add_images_to_album("album-1", ["img-1"]) + + assert db_get_album("album-1")["updated_at"] > self.EARLIER + + def test_removing_an_image_touches_it(self, test_db): + make_album("album-1", "Trip") + make_images(test_db, ["img-1"]) + db_add_images_to_album("album-1", ["img-1"]) + self.set_updated_at(test_db, "album-1") + + db_remove_image_from_album("album-1", "img-1") + + assert db_get_album("album-1")["updated_at"] > self.EARLIER + + def test_removing_images_in_bulk_touches_it(self, test_db): + make_album("album-1", "Trip") + make_images(test_db, ["img-1", "img-2"]) + db_add_images_to_album("album-1", ["img-1", "img-2"]) + self.set_updated_at(test_db, "album-1") + + db_remove_images_from_album("album-1", ["img-1"]) + + assert db_get_album("album-1")["updated_at"] > self.EARLIER + + def test_re_adding_the_same_images_leaves_it_alone(self, test_db): + """Nothing was inserted, so nothing about the album changed.""" + make_album("album-1", "Trip") + make_images(test_db, ["img-1"]) + db_add_images_to_album("album-1", ["img-1"]) + self.set_updated_at(test_db, "album-1") + + db_add_images_to_album("album-1", ["img-1"]) + + assert db_get_album("album-1")["updated_at"] == self.EARLIER + + def test_bulk_removing_absent_images_leaves_it_alone(self, test_db): + make_album("album-1", "Trip") + self.set_updated_at(test_db, "album-1") + + db_remove_images_from_album("album-1", ["img-missing"]) + + assert db_get_album("album-1")["updated_at"] == self.EARLIER + + def test_a_failed_removal_leaves_it_alone(self, test_db): + """The image was never in the album, so nothing about it changed.""" + make_album("album-1", "Trip") + self.set_updated_at(test_db, "album-1") + + with pytest.raises(ValueError): + db_remove_image_from_album("album-1", "img-missing") + + assert db_get_album("album-1")["updated_at"] == self.EARLIER + + +class TestCreateAlbumWithImages: + def test_creates_the_album_and_links_every_image(self, test_db): + make_images(test_db, ["img-1", "img-2"]) + + linked = db_create_album_with_images( + "album-1", "Paris 2022", "July 2022", ["img-1", "img-2"] + ) + + assert linked == 2 + assert db_get_album_images("album-1") == ["img-1", "img-2"] + album = db_get_album("album-1") + assert album["album_name"] == "Paris 2022" + assert album["description"] == "July 2022" + # Always an open album: locking is done afterwards, from Edit Album. + assert album["is_locked"] is False + assert album["password_hash"] is None + + def test_an_unknown_image_leaves_no_album_behind(self, test_db): + """The album and its links commit together, or not at all.""" + make_images(test_db, ["img-1"]) + + with pytest.raises(sqlite3.IntegrityError): + db_create_album_with_images( + "album-1", "Paris 2022", "", ["img-1", "img-missing"] + ) + + assert db_get_album("album-1") is None + + def test_a_duplicate_name_is_rejected(self, test_db): + make_album("album-1", "Paris 2022") + make_images(test_db, ["img-1"]) + + with pytest.raises(sqlite3.IntegrityError): + db_create_album_with_images("album-2", "Paris 2022", "", ["img-1"]) + + assert db_get_album("album-2") is None + + # ############################## # Password handling # ############################## diff --git a/docs/backend/backend_python/openapi.json b/docs/backend/backend_python/openapi.json index d6f10904a..3cf429f57 100644 --- a/docs/backend/backend_python/openapi.json +++ b/docs/backend/backend_python/openapi.json @@ -467,6 +467,88 @@ } } }, + "/albums/from-memory": { + "post": { + "tags": [ + "Albums" + ], + "summary": "Create Album From Memory", + "description": "Copy a memory's photos into a new album.\n\nThe memory itself is left untouched, so the same one can be converted\nagain under a different name. Any clips are left behind: album_images\nreferences images, and albums have no video support.", + "operationId": "create_album_from_memory_albums_from_memory_post", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreateAlbumFromMemoryRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreateAlbumFromMemoryResponse" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponseEnvelope" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponseEnvelope" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponseEnvelope" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponseEnvelope" + } + } + } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } + } + } + } + }, "/albums/{album_id}": { "get": { "tags": [ @@ -1515,14 +1597,9 @@ "in": "query", "required": false, "schema": { - "allOf": [ - { - "$ref": "#/components/schemas/InputType" - } - ], + "$ref": "#/components/schemas/InputType", "description": "Choose input type: 'path' or 'base64'", - "default": "path", - "title": "Input Type" + "default": "path" }, "description": "Choose input type: 'path' or 'base64'" } @@ -2580,6 +2657,28 @@ "type": "integer", "title": "Image Count", "default": 0 + }, + "created_at": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Created At" + }, + "updated_at": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Updated At" } }, "type": "object", @@ -2633,6 +2732,66 @@ ], "title": "ClusterMetadata" }, + "CreateAlbumFromMemoryData": { + "properties": { + "album_id": { + "type": "string", + "title": "Album Id" + }, + "image_count": { + "type": "integer", + "title": "Image Count" + } + }, + "type": "object", + "required": [ + "album_id", + "image_count" + ], + "title": "CreateAlbumFromMemoryData" + }, + "CreateAlbumFromMemoryRequest": { + "properties": { + "memory_id": { + "type": "string", + "minLength": 1, + "title": "Memory Id" + }, + "name": { + "type": "string", + "minLength": 1, + "title": "Name" + } + }, + "type": "object", + "required": [ + "memory_id", + "name" + ], + "title": "CreateAlbumFromMemoryRequest" + }, + "CreateAlbumFromMemoryResponse": { + "properties": { + "success": { + "type": "boolean", + "title": "Success" + }, + "message": { + "type": "string", + "title": "Message" + }, + "data": { + "$ref": "#/components/schemas/CreateAlbumFromMemoryData" + } + }, + "type": "object", + "required": [ + "success", + "message", + "data" + ], + "title": "CreateAlbumFromMemoryResponse" + }, "CreateAlbumRequest": { "properties": { "name": { @@ -2811,6 +2970,19 @@ ], "title": "DeleteMemoryResponse" }, + "ErrorResponseEnvelope": { + "properties": { + "detail": { + "$ref": "#/components/schemas/app__schemas__album__ErrorResponse" + } + }, + "type": "object", + "required": [ + "detail" + ], + "title": "ErrorResponseEnvelope", + "description": "How an ErrorResponse actually reaches the client.\n\nHTTPException nests whatever it is given under `detail`, so documenting\nErrorResponse alone would describe a shape no client ever receives." + }, "FaceSearchRequest": { "properties": { "path": { @@ -3644,6 +3816,7 @@ "metadata": { "anyOf": [ { + "additionalProperties": true, "type": "object" }, { @@ -4575,6 +4748,7 @@ "signals": { "anyOf": [ { + "additionalProperties": true, "type": "object" }, { @@ -4802,6 +4976,7 @@ "metadata": { "anyOf": [ { + "additionalProperties": true, "type": "object" }, { @@ -5886,6 +6061,29 @@ ], "title": "ToggleFavouriteRequest" }, + "app__schemas__album__ErrorResponse": { + "properties": { + "success": { + "type": "boolean", + "title": "Success", + "default": false + }, + "message": { + "type": "string", + "title": "Message" + }, + "error": { + "type": "string", + "title": "Error" + } + }, + "type": "object", + "required": [ + "message", + "error" + ], + "title": "ErrorResponse" + }, "app__schemas__face_clusters__ErrorResponse": { "properties": { "success": { diff --git a/frontend/src/api/api-functions/albums.ts b/frontend/src/api/api-functions/albums.ts index 98a16597a..6f755f8f7 100644 --- a/frontend/src/api/api-functions/albums.ts +++ b/frontend/src/api/api-functions/albums.ts @@ -1,8 +1,11 @@ import { albumsEndpoints } from '../apiEndpoints'; import { apiClient } from '../axiosConfig'; import { APIResponse } from '@/types/API'; +import type { BackendRes } from '@/hooks/useQueryExtension'; import { CreateAlbumRequest, + CreateAlbumFromMemoryData, + CreateAlbumFromMemoryRequest, UpdateAlbumRequest, AddImagesToAlbumRequest, GetAlbumImagesRequest, @@ -44,6 +47,20 @@ export const createAlbum = async ( return response.data; }; +/** + * Create an album from a curated memory's photos + * @param data - Source memory and the new album's name + */ +export const createAlbumFromMemory = async ( + data: CreateAlbumFromMemoryRequest, +): Promise> => { + const response = await apiClient.post( + albumsEndpoints.createAlbumFromMemory, + data, + ); + return response.data; +}; + /** * Update an existing album * @param albumId - Album UUID diff --git a/frontend/src/api/apiEndpoints.ts b/frontend/src/api/apiEndpoints.ts index 0ac942d95..384f56975 100644 --- a/frontend/src/api/apiEndpoints.ts +++ b/frontend/src/api/apiEndpoints.ts @@ -48,6 +48,7 @@ export const albumsEndpoints = { getAllAlbums: '/albums/', getAlbumById: (albumId: string) => `/albums/${albumId}`, createAlbum: '/albums/', + createAlbumFromMemory: '/albums/from-memory', updateAlbum: (albumId: string) => `/albums/${albumId}`, deleteAlbum: (albumId: string) => `/albums/${albumId}`, addImagesToAlbum: (albumId: string) => `/albums/${albumId}/images`, diff --git a/frontend/src/components/GallerySortDropdown.tsx b/frontend/src/components/GallerySortDropdown.tsx index 3ae391395..79b1d7ebc 100644 --- a/frontend/src/components/GallerySortDropdown.tsx +++ b/frontend/src/components/GallerySortDropdown.tsx @@ -22,7 +22,7 @@ export interface SortOption { export type GallerySortValue = 'best_match' | 'date'; -const GALLERY_SORT_OPTIONS: SortOption[] = [ +export const GALLERY_SORT_OPTIONS: SortOption[] = [ { value: 'best_match', label: 'Best match', icon: Star }, { value: 'date', label: 'Date', icon: Calendar }, ]; diff --git a/frontend/src/components/Media/ChronologicalGallery.tsx b/frontend/src/components/Media/ChronologicalGallery.tsx index f228f8f14..80b7de4b7 100644 --- a/frontend/src/components/Media/ChronologicalGallery.tsx +++ b/frontend/src/components/Media/ChronologicalGallery.tsx @@ -1,6 +1,8 @@ import { useMemo, useRef, useEffect, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { ImageCard } from '@/components/Media/ImageCard'; +import { MEDIA_GRID_CLASS } from '@/constants/layout'; +import { cn } from '@/lib/utils'; import { Image } from '@/types/Media'; import { groupImagesByYearMonthFromMetadata } from '@/utils/dateUtils'; import { setCurrentViewIndex } from '@/features/imageSlice'; @@ -160,7 +162,7 @@ export const ChronologicalGallery = ({ {/* Images Grid */} -
+
{imgs.map((img) => { const chronologicalIndex = imageIndexMap.get(img.id) ?? -1; diff --git a/frontend/src/components/Media/ChronologicalVideoGallery.tsx b/frontend/src/components/Media/ChronologicalVideoGallery.tsx index a5824b1dc..a84c829d7 100644 --- a/frontend/src/components/Media/ChronologicalVideoGallery.tsx +++ b/frontend/src/components/Media/ChronologicalVideoGallery.tsx @@ -1,6 +1,8 @@ import { useMemo, useRef, useEffect, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { VideoCard } from '@/components/Media/VideoCard'; +import { MEDIA_GRID_CLASS } from '@/constants/layout'; +import { cn } from '@/lib/utils'; import { Video } from '@/types/Media'; import { groupImagesByYearMonthFromMetadata } from '@/utils/dateUtils'; import { MonthMarker } from './ChronologicalGallery'; @@ -155,7 +157,7 @@ export const ChronologicalVideoGallery = ({
{/* Videos Grid */} -
+
{vids.map((video) => { const chronologicalIndex = videoIndexMap.get(video.id) ?? -1; diff --git a/frontend/src/components/Media/RankedGallery.tsx b/frontend/src/components/Media/RankedGallery.tsx index 3209c65a4..ff23c0535 100644 --- a/frontend/src/components/Media/RankedGallery.tsx +++ b/frontend/src/components/Media/RankedGallery.tsx @@ -1,6 +1,8 @@ import { useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { ImageCard } from '@/components/Media/ImageCard'; +import { MEDIA_GRID_CLASS } from '@/constants/layout'; +import { cn } from '@/lib/utils'; import { Image } from '@/types/Media'; import { setCurrentViewIndex } from '@/features/imageSlice'; import { MediaView } from './MediaView'; @@ -36,7 +38,7 @@ export function RankedGallery({ {titleRight &&
{titleRight}
}
)} -
+
{images.map((img) => (
void; +} + +/** + * Copies a memory's photos into a new album. The name is pre-filled from the + * memory's title; everything else about the album is edited afterwards. + */ +export const ConvertMemoryToAlbumDialog: React.FC< + ConvertMemoryToAlbumDialogProps +> = ({ memory, isOpen, onClose }) => { + const navigate = useNavigate(); + const [name, setName] = useState(''); + const [error, setError] = useState(''); + + // One mounted dialog serves every tile, so the name follows the selected + // memory rather than the first render. Keyed on opening as well, so a name + // typed and then abandoned does not come back the next time it opens. + useEffect(() => { + if (isOpen && memory) { + setName(memory.title); + setError(''); + } + }, [isOpen, memory]); + + const convertMutation = usePictoMutation({ + mutationFn: createAlbumFromMemory, + autoInvalidateTags: ['albums'], + }); + + useMutationFeedback(convertMutation, { + loadingMessage: 'Creating album...', + successTitle: 'Success', + successMessage: 'Album created from memory!', + errorTitle: 'Error', + // A duplicate name comes back as a 409 and getErrorMessage surfaces the + // backend's own wording, so the dialog stays open to be renamed. + errorMessage: 'Failed to create the album. Please try again.', + onSuccess: () => { + const albumId = convertMutation.successData?.album_id; + onClose(); + if (albumId) { + navigate(`/${ROUTES.ALBUMS}/${albumId}`); + } + }, + }); + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + + if (!memory) return; + + if (!name.trim()) { + setError('Album name is required'); + return; + } + + setError(''); + convertMutation.mutate({ memory_id: memory.memory_id, name: name.trim() }); + }; + + return ( + + +
+ + Convert to Album + + Creates an album from this memory's{' '} + {formatPhotoCount(memory?.image_count ?? 0)}. The memory itself is + kept. + {memory !== null && + memory.video_count > 0 && + ' Videos are left out — albums hold photos only.'} + + + +
+
+ + setName(e.target.value)} + className={error ? 'border-destructive' : ''} + /> + {error &&

{error}

} +
+
+ + + + + +
+
+
+ ); +}; diff --git a/frontend/src/components/Memories/MemoryCard.tsx b/frontend/src/components/Memories/MemoryCard.tsx index 12a0a98bd..dd16b80bb 100644 --- a/frontend/src/components/Memories/MemoryCard.tsx +++ b/frontend/src/components/Memories/MemoryCard.tsx @@ -1,7 +1,14 @@ import React from 'react'; -import { Images } from 'lucide-react'; +import { FolderPlus, Images, MoreVertical } from 'lucide-react'; import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/components/ui/dropdown-menu'; import { createImageErrorHandler } from '@/utils/imageFallback'; import type { MemoryCard as MemoryCardType } from '@/api/api-functions/memories'; import { @@ -17,64 +24,96 @@ const handleCoverError = createImageErrorHandler(MEMORY_PLACEHOLDER_IMAGE); interface MemoryCardProps { memory: MemoryCardType; onOpen: (memoryId: string) => void; + onConvertToAlbum: (memoryId: string) => void; } /** * Grid tile for one memory. The cover scales and the caption lifts on hover; - * the whole tile opens the story viewer. + * the tile opens the story viewer, and the actions menu sits beside it rather + * than inside it - a button cannot contain another button. */ -export const MemoryCard: React.FC = ({ memory, onOpen }) => { +export const MemoryCard: React.FC = ({ + memory, + onOpen, + onConvertToAlbum, +}) => { const subtitle = formatMemorySubtitle(memory); const isUnviewed = memory.viewed_at === null; return ( - + + {/* Actions menu, matching the album grid's affordance. */} +
+ + + + + + onConvertToAlbum(memory.memory_id)} + > + + Convert to Album + + +
- +
); }; diff --git a/frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx b/frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx new file mode 100644 index 000000000..261ba4d18 --- /dev/null +++ b/frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx @@ -0,0 +1,63 @@ +import userEvent from '@testing-library/user-event'; +import { render, screen } from '@/test-utils'; +import type { MemoryCard } from '@/api/api-functions/memories'; +import { ConvertMemoryToAlbumDialog } from '../ConvertMemoryToAlbumDialog'; + +jest.mock('@/api/api-functions', () => ({ + createAlbumFromMemory: jest.fn(), +})); + +const memory = { + memory_id: 'mem-1', + title: 'Beach', + subtitle: '26 July 2024', + image_count: 4, + video_count: 0, +} as MemoryCard; + +const renderDialog = (isOpen: boolean) => + render( + , + ); + +const nameInput = () => screen.getByLabelText(/album name/i); + +describe('ConvertMemoryToAlbumDialog', () => { + it('prefills the name from the memory title', () => { + renderDialog(true); + + expect(nameInput()).toHaveValue('Beach'); + }); + + // The parent happens to null the memory between opens, but the dialog must + // not depend on that: reopening for the same memory has to start clean. + it('clears a typed name when reopened for the same memory', async () => { + const user = userEvent.setup(); + const { rerender } = renderDialog(true); + + await user.clear(nameInput()); + await user.type(nameInput(), 'Something else'); + expect(nameInput()).toHaveValue('Something else'); + + rerender( + , + ); + rerender( + , + ); + + expect(nameInput()).toHaveValue('Beach'); + }); +}); diff --git a/frontend/src/components/Memories/__tests__/MemoryCard.test.tsx b/frontend/src/components/Memories/__tests__/MemoryCard.test.tsx new file mode 100644 index 000000000..f00ece19d --- /dev/null +++ b/frontend/src/components/Memories/__tests__/MemoryCard.test.tsx @@ -0,0 +1,72 @@ +import userEvent from '@testing-library/user-event'; +import { render, screen } from '@/test-utils'; +import type { MemoryCard as MemoryCardType } from '@/api/api-functions/memories'; +import { MemoryCard } from '../MemoryCard'; + +jest.mock('@tauri-apps/api/core', () => ({ + invoke: jest.fn().mockResolvedValue(null), + convertFileSrc: jest.fn((path: string) => `asset://localhost/${path}`), +})); + +const memory: MemoryCardType = { + memory_id: 'mem-1', + dedupe_key: 'import:2024-07-26..2024-07-26', + event_type: 'import_event', + status: 'complete', + title: 'Beach', + subtitle: '26 July 2024', + place_label: null, + center_lat: null, + center_lon: null, + surface_date: '2024-07-26', + period_start: '2024-07-26T10:00:00', + period_end: '2024-07-26T10:05:00', + image_count: 4, + video_count: 0, + cover_image_id: null, + cover_thumbnail_path: null, + score: 1, + notified_at: null, + viewed_at: null, + dismissed: false, + created_at: '2024-07-26T12:00:00', +}; + +const renderCard = () => { + const onOpen = jest.fn(); + const onConvertToAlbum = jest.fn(); + render( + , + ); + return { onOpen, onConvertToAlbum }; +}; + +describe('MemoryCard', () => { + it('opens the memory when the tile is clicked', async () => { + const user = userEvent.setup(); + const { onOpen } = renderCard(); + + await user.click( + screen.getByRole('button', { name: 'Open memory: Beach' }), + ); + + expect(onOpen).toHaveBeenCalledWith('mem-1'); + }); + + it('converts the memory from the actions menu without opening it', async () => { + const user = userEvent.setup(); + const { onOpen, onConvertToAlbum } = renderCard(); + + await user.click(screen.getByRole('button', { name: 'Options for Beach' })); + await user.click(await screen.findByText('Convert to Album')); + + expect(onConvertToAlbum).toHaveBeenCalledWith('mem-1'); + // The menu is a sibling of the tile, not a child, so acting on it must + // never open the story viewer. + expect(onOpen).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/constants/layout.ts b/frontend/src/constants/layout.ts new file mode 100644 index 000000000..57948f11a --- /dev/null +++ b/frontend/src/constants/layout.ts @@ -0,0 +1,9 @@ +/** + * The card grid used by every media surface. + * + * Intrinsic rather than breakpoint-driven: the cards keep a near-constant + * width and the column count follows the window, so resizing reflows the + * grid instead of resizing every card. Callers add their own padding. + */ +export const MEDIA_GRID_CLASS = + 'grid grid-cols-[repeat(auto-fill,_minmax(224px,_1fr))] gap-4'; diff --git a/frontend/src/hooks/__tests__/usePersistedSort.test.tsx b/frontend/src/hooks/__tests__/usePersistedSort.test.tsx new file mode 100644 index 000000000..9a83e2607 --- /dev/null +++ b/frontend/src/hooks/__tests__/usePersistedSort.test.tsx @@ -0,0 +1,46 @@ +import { act, renderHook } from '@testing-library/react'; +import { usePersistedSort } from '../usePersistedSort'; + +const KEY = 'test-sort'; +const VALUES = ['name', 'date'] as const; +type TestSort = (typeof VALUES)[number]; + +const renderSort = () => + renderHook(() => usePersistedSort(KEY, 'name', VALUES)); + +describe('usePersistedSort', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('starts on the default when nothing is stored', () => { + const { result } = renderSort(); + + expect(result.current[0]).toBe('name'); + }); + + it('restores the stored selection', () => { + localStorage.setItem(KEY, 'date'); + + const { result } = renderSort(); + + expect(result.current[0]).toBe('date'); + }); + + it('keeps the selection across a remount', () => { + const first = renderSort(); + act(() => first.result.current[1]('date')); + first.unmount(); + + // A remount stands in for the reload that used to reset the sort. + expect(renderSort().result.current[0]).toBe('date'); + }); + + it('falls back to the default for a sort that no longer exists', () => { + localStorage.setItem(KEY, 'photoCount'); + + const { result } = renderSort(); + + expect(result.current[0]).toBe('name'); + }); +}); diff --git a/frontend/src/hooks/usePersistedSort.ts b/frontend/src/hooks/usePersistedSort.ts new file mode 100644 index 000000000..768035470 --- /dev/null +++ b/frontend/src/hooks/usePersistedSort.ts @@ -0,0 +1,31 @@ +import { useCallback, useState } from 'react'; + +/** + * A sort selection that survives a reload, stored per surface like the theme. + * + * The stored value is checked against the options currently on offer: one left + * behind by an older build would otherwise sort the grid by nothing, with no + * option showing as selected and no way to tell why. + */ +export function usePersistedSort( + storageKey: string, + defaultValue: T, + allowedValues: readonly T[], +): [T, (value: T) => void] { + const [sort, setSort] = useState(() => { + // Matching against the allowed values narrows the stored string to T, so + // no cast is needed to trust what came out of storage. + const stored = localStorage.getItem(storageKey); + return allowedValues.find((value) => value === stored) ?? defaultValue; + }); + + const selectSort = useCallback( + (value: T) => { + setSort(value); + localStorage.setItem(storageKey, value); + }, + [storageKey], + ); + + return [sort, selectSort]; +} diff --git a/frontend/src/pages/AITagging/AITagging.tsx b/frontend/src/pages/AITagging/AITagging.tsx index ac4851039..43583a261 100644 --- a/frontend/src/pages/AITagging/AITagging.tsx +++ b/frontend/src/pages/AITagging/AITagging.tsx @@ -18,13 +18,27 @@ import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; import { formatPeopleTitle } from '@/utils/personUtils'; import { RankedGallery } from '@/components/Media/RankedGallery'; -import { GallerySortDropdown } from '@/components/GallerySortDropdown'; +import { + GALLERY_SORT_OPTIONS, + GallerySortDropdown, + type GallerySortValue, +} from '@/components/GallerySortDropdown'; +import { usePersistedSort } from '@/hooks/usePersistedSort'; + +const AI_TAGGING_SORT_STORAGE_KEY = 'pictopy-ai-tagging-sort'; + +// Derived from the options above so a removed sort stops being restorable. +const GALLERY_SORT_VALUES = GALLERY_SORT_OPTIONS.map((option) => option.value); export const AITagging = () => { const dispatch = useDispatch(); const scrollableRef = useRef(null); const [monthMarkers, setMonthMarkers] = useState([]); - const [sortMode, setSortMode] = useState<'best_match' | 'date'>('best_match'); + const [sortMode, setSortMode] = usePersistedSort( + AI_TAGGING_SORT_STORAGE_KEY, + 'best_match', + GALLERY_SORT_VALUES, + ); const [searchState, setSearchState] = useState<{ active: boolean; peopleNames: string[]; diff --git a/frontend/src/pages/Album/Album.tsx b/frontend/src/pages/Album/Album.tsx index 6a7223b9f..57497e36b 100644 --- a/frontend/src/pages/Album/Album.tsx +++ b/frontend/src/pages/Album/Album.tsx @@ -2,7 +2,14 @@ import { useEffect, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { useNavigate } from 'react-router'; import { Button } from '@/components/ui/button'; -import { Plus, RefreshCw, ArrowDownAZ, Images } from 'lucide-react'; +import { + Plus, + RefreshCw, + ArrowDownAZ, + CalendarClock, + History, + Images, +} from 'lucide-react'; import { AlbumCard } from '@/components/Albums/AlbumCard'; import { CreateAlbumDialog } from '@/components/Albums/CreateAlbumDialog'; import { EditAlbumDialog } from '@/components/Albums/EditAlbumDialog'; @@ -15,19 +22,37 @@ import { setAlbums } from '@/features/albumsSlice'; import { selectAlbums } from '@/features/albumSelectors'; import { showInfoDialog } from '@/features/infoDialogSlice'; import { useMutationFeedback } from '@/hooks/useMutationFeedback'; +import { usePersistedSort } from '@/hooks/usePersistedSort'; +import { MEDIA_GRID_CLASS } from '@/constants/layout'; +import { cn } from '@/lib/utils'; import { Album } from '@/types/Album'; import { GallerySortDropdown, type SortOption, } from '@/components/GallerySortDropdown'; -type AlbumSortValue = 'name' | 'photoCount'; +type AlbumSortValue = 'name' | 'photoCount' | 'dateCreated' | 'recentlyUpdated'; const ALBUM_SORT_OPTIONS: SortOption[] = [ { value: 'name', label: 'Name (A-Z)', icon: ArrowDownAZ }, { value: 'photoCount', label: 'Photo Count', icon: Images }, + { value: 'dateCreated', label: 'Date Created', icon: CalendarClock }, + { value: 'recentlyUpdated', label: 'Recently Updated', icon: History }, ]; +const ALBUM_SORT_STORAGE_KEY = 'pictopy-albums-sort'; + +// Derived from the options above so a removed sort stops being restorable. +const ALBUM_SORT_VALUES = ALBUM_SORT_OPTIONS.map((option) => option.value); + +/** + * Newest first. SQLite timestamps are zero-padded, so they compare correctly + * as strings. Albums predating these columns have no timestamp: they read as + * oldest and keep the insertion order the backend lists them in. + */ +const newestFirst = (a: string | null, b: string | null): number => + (b ?? '').localeCompare(a ?? ''); + // Mirrors an AlbumCard: the same 4/5 cover as a memory tile, plus the name and // count bars, so the grid does not jump when the real cards arrive. const AlbumCardSkeleton: React.FC = () => ( @@ -51,7 +76,11 @@ function Albums() { const [isPasswordDialogOpen, setIsPasswordDialogOpen] = useState(false); const [albumToDelete, setAlbumToDelete] = useState(null); const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false); - const [sortBy, setSortBy] = useState('name'); + const [sortBy, setSortBy] = usePersistedSort( + ALBUM_SORT_STORAGE_KEY, + 'name', + ALBUM_SORT_VALUES, + ); const { data: albumsData, @@ -98,6 +127,8 @@ function Albums() { is_locked: Boolean(album.is_locked), cover_image_path: album.cover_image_path, image_count: album.image_count || 0, + created_at: album.created_at ?? null, + updated_at: album.updated_at ?? null, })) as Album[]; dispatch(setAlbums(albumsList)); } @@ -157,6 +188,10 @@ function Albums() { return a.name.localeCompare(b.name); } else if (sortBy === 'photoCount') { return b.image_count - a.image_count; + } else if (sortBy === 'dateCreated') { + return newestFirst(a.created_at, b.created_at); + } else if (sortBy === 'recentlyUpdated') { + return newestFirst(a.updated_at, b.updated_at); } return 0; }); @@ -190,7 +225,7 @@ function Albums() {
{isLoading ? ( -
+
{Array.from({ length: 10 }).map((_, index) => ( ))} @@ -198,7 +233,7 @@ function Albums() { ) : albums.length === 0 ? ( ) : ( -
+
{sortedAlbums.map((album) => ( { is_locked: backendAlbum.is_locked || false, cover_image_path: backendAlbum.cover_image_path, image_count: backendAlbum.image_count || 0, + created_at: backendAlbum.created_at ?? null, + updated_at: backendAlbum.updated_at ?? null, }; dispatch(setSelectedAlbum(albumInfo)); } diff --git a/frontend/src/pages/Memories/Memories.tsx b/frontend/src/pages/Memories/Memories.tsx index 3d70802d0..f81c6534c 100644 --- a/frontend/src/pages/Memories/Memories.tsx +++ b/frontend/src/pages/Memories/Memories.tsx @@ -1,13 +1,16 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { RefreshCw } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { MemoryCard } from '@/components/Memories/MemoryCard'; +import { ConvertMemoryToAlbumDialog } from '@/components/Memories/ConvertMemoryToAlbumDialog'; import { MemoryStoryViewer } from '@/components/Memories/MemoryStoryViewer'; import { showInfoDialog } from '@/features/infoDialogSlice'; import { useAppDispatch, useAppSelector } from '@/store/hooks'; import { useUserPreferences } from '@/hooks/useUserPreferences'; +import { MEDIA_GRID_CLASS } from '@/constants/layout'; import { useMemories, useRefreshMemories } from '@/hooks/useMemories'; +import type { MemoryCard as MemoryCardType } from '@/api/api-functions/memories'; import { openMemory, selectActiveMemoryId, @@ -33,6 +36,10 @@ export const Memories: React.FC = () => { const dispatch = useAppDispatch(); const activeMemoryId = useAppSelector(selectActiveMemoryId); + const [memoryToConvert, setMemoryToConvert] = useState( + null, + ); + const memoriesQuery = useMemories({ limit: 60 }); const { refresh, isRefreshing, status: statusQuery } = useRefreshMemories(); const { memoriesPreferences } = useUserPreferences(); @@ -106,7 +113,7 @@ export const Memories: React.FC = () => {
{memoriesQuery.isLoading ? ( -
+
{Array.from({ length: 10 }).map((_, index) => ( ))} @@ -114,18 +121,25 @@ export const Memories: React.FC = () => { ) : memories.length === 0 ? ( ) : ( -
+
{memories.map((memory) => ( dispatch(openMemory(id))} + onConvertToAlbum={() => setMemoryToConvert(memory)} /> ))}
)}
+ setMemoryToConvert(null)} + /> + {activeMemoryId && ( { expect(cover.getAttribute('src')).toMatch(/placeholder-album/); }, 30000); + const chooseSort = async ( + user: ReturnType, + label: RegExp, + ) => { + await user.click(screen.getByRole('button', { name: /sort by/i })); + const menuItems = await screen.findAllByRole('menuitem'); + const option = menuItems.find((item) => label.test(item.textContent || '')); + await user.click(option as HTMLElement); + }; + + const albumOrder = () => + screen.getAllByRole('heading', { level: 3 }).map((h) => h.textContent); + + // 'Legacy' predates both timestamp columns, so it carries neither. + const timestampedAlbums = [ + { album_id: 'a1', album_name: 'Legacy', image_count: 1 }, + { + album_id: 'a2', + album_name: 'MadeFirst', + image_count: 1, + created_at: '2026-07-01 10:00:00', + updated_at: '2026-08-09 10:00:00', + }, + { + album_id: 'a3', + album_name: 'MadeLast', + image_count: 1, + created_at: '2026-08-01 10:00:00', + updated_at: '2026-08-02 10:00:00', + }, + ]; + + test('sorts by creation date, newest first', async () => { + const user = userEvent.setup(); + serverAlbums = timestampedAlbums; + + render(); + await screen.findByText('Legacy'); + + await chooseSort(user, /date created/i); + + await waitFor(() => + expect(albumOrder()).toEqual(['MadeLast', 'MadeFirst', 'Legacy']), + ); + }, 30000); + + // The album made first was edited most recently, so the two date sorts must + // not agree - otherwise this would pass against either field. + test('sorts by last update, most recent first', async () => { + const user = userEvent.setup(); + serverAlbums = timestampedAlbums; + + render(); + await screen.findByText('Legacy'); + + await chooseSort(user, /recently updated/i); + + await waitFor(() => + expect(albumOrder()).toEqual(['MadeFirst', 'MadeLast', 'Legacy']), + ); + }, 30000); + test('shows skeletons while loading instead of a blocking loader', async () => { let releaseAlbums: (value: unknown) => void = () => {}; mockGetAllAlbums.mockImplementation( diff --git a/frontend/src/types/Album.ts b/frontend/src/types/Album.ts index fcf64c242..0115d947f 100644 --- a/frontend/src/types/Album.ts +++ b/frontend/src/types/Album.ts @@ -5,6 +5,10 @@ export interface Album { is_locked: boolean; cover_image_path?: string; image_count: number; + /** Null for albums that predate the backend recording these times. */ + created_at: string | null; + /** Touched by metadata edits and by adding or removing photos. */ + updated_at: string | null; } export interface AlbumFormData { @@ -21,6 +25,16 @@ export interface CreateAlbumRequest { password?: string; } +export interface CreateAlbumFromMemoryRequest { + memory_id: string; + name: string; +} + +export interface CreateAlbumFromMemoryData { + album_id: string; + image_count: number; +} + export interface UpdateAlbumRequest { name?: string; description?: string;