Skip to content

feat: implement albums feature with create, edit, delete, and image management UI #610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SiddharthJiyani wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: implement albums feature with create, edit, delete, and image management UI #610

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
afcecea
feat: implement albums feature with create, edit, delete, and image m…
SiddharthJiyani Nov 3, 2025
5841ada
test: update album tests to use is_locked instead of is_hidden
SiddharthJiyani Nov 3, 2025
67e9937
refactor: address coderabbit review feedback for albums feature
SiddharthJiyani Nov 3, 2025
634140f
refactor code to address errors
SiddharthJiyani Nov 3, 2025
3078e9d
refactor: use useMutationFeedback for album mutations
SiddharthJiyani Feb 15, 2026
6ab8ede
fix: remove unused imports from album dialogs
SiddharthJiyani Feb 15, 2026
a25088f
fix: address code review feedback
SiddharthJiyani Feb 15, 2026
262874a
fix: address code review issues - htmlFor mismatch, stale closure, re…
SiddharthJiyani Feb 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 70 additions & 17 deletions backend/app/database/albums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ def db_create_albums_table() -> None:
album_id TEXT PRIMARY KEY,
album_name TEXT UNIQUE,
description TEXT,
is_hidden BOOLEAN DEFAULT 0,
password_hash TEXT
is_locked BOOLEAN DEFAULT 0,
password_hash TEXT,
cover_image_path TEXT
)
"""
)
Expand All @@ -61,6 +62,26 @@ def db_create_albums_table() -> None:
conn.close()


def db_migrate_add_cover_image_column() -> None:
"""Add cover_image_path column to existing albums table if it doesn't exist"""
conn = None
try:
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()

# Check if column exists
cursor.execute("PRAGMA table_info(albums)")
columns = [column[1] for column in cursor.fetchall()]

if "cover_image_path" not in columns:
cursor.execute("ALTER TABLE albums ADD COLUMN cover_image_path TEXT")
conn.commit()
print("Added cover_image_path column to albums table")
finally:
if conn is not None:
conn.close()
Comment on lines +65 to +82
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify migration function is invoked.

The migration function logic is correct, but ensure it's being called during application initialization or in a migration script to actually apply the schema change to existing databases.

Run this script to check if the migration function is invoked:

🏁 Script executed:

#!/bin/bash
# Check if db_migrate_add_cover_image_column is called anywhere
rg -n "db_migrate_add_cover_image_column" --type=py -C 3

Length of output: 421


---

🏁 Script executed:

```shell
#!/bin/bash
# Check for migration-related patterns in initialization and main files
rg -n "(migrate|initialization|startup)" --type=py -C 2 backend/app/__init__.py backend/app/database/__init__.py backend/app/main.py 2>/dev/null || echo "No direct matches"

# Also check if there's any file that imports this migration function
rg -n "from.*albums.*import|import.*db_migrate" --type=py

Length of output: 423


---

🏁 Script executed:

```shell
#!/bin/bash
# Check where db_create functions are called in main.py to understand the initialization flow
sed -n '15,50p' backend/main.py

Length of output: 1412


---

🏁 Script executed:

```shell
#!/bin/bash
# Get the full lifespan function to see complete initialization
sed -n '40,70p' backend/main.py

Length of output: 921

Add migration function to application startup sequence.

The db_migrate_add_cover_image_column function is defined but never called, so the cover_image_path column won't be added to existing databases. Import and invoke it during application initialization in backend/main.py within the lifespan context manager, after db_create_albums_table() is called (around line 54).

🤖 Prompt for AI Agents 
In backend/main.py around line 54 inside the lifespan context manager, the
migration function db_migrate_add_cover_image_column from
backend.app.database.albums is not invoked; import
db_migrate_add_cover_image_column at the top of the file and call it immediately
after the existing db_create_albums_table() invocation so the migration runs on
startup for existing databases (keep it synchronous, handle/let exceptions
propagate as with db_create_albums_table()).

Comment on lines +65 to +82
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The migration function db_migrate_add_cover_image_column is defined but there's no evidence it's being called during application startup in the modified code. This migration is necessary for existing databases that don't have the cover_image_path column. For backward compatibility with existing databases, this function should be called after table creation but before the application starts using album functionality.

Copilot uses AI. Check for mistakes.


def db_create_album_images_table() -> None:
conn = None
try:
Expand All @@ -83,14 +104,14 @@ def db_create_album_images_table() -> None:
conn.close()


def db_get_all_albums(show_hidden: bool = False):
def db_get_all_albums():
"""Get all albums (both locked and unlocked)."""
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
try:
if show_hidden:
cursor.execute("SELECT * FROM albums")
else:
cursor.execute("SELECT * FROM albums WHERE is_hidden = 0")
cursor.execute(
"SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums"
)
albums = cursor.fetchall()
return albums
finally:
Expand All @@ -101,7 +122,10 @@ def db_get_album_by_name(name: str):
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
try:
cursor.execute("SELECT * FROM albums WHERE album_name = ?", (name,))
cursor.execute(
"SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums WHERE album_name = ?",
(name,),
)
album = cursor.fetchone()
return album if album else None
finally:
Expand All @@ -112,7 +136,10 @@ def db_get_album(album_id: str):
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
try:
cursor.execute("SELECT * FROM albums WHERE album_id = ?", (album_id,))
cursor.execute(
"SELECT album_id, album_name, description, is_locked, password_hash, cover_image_path FROM albums WHERE album_id = ?",
(album_id,),
)
album = cursor.fetchone()
return album if album else None
finally:
Expand All @@ -123,7 +150,7 @@ def db_insert_album(
album_id: str,
album_name: str,
description: str = "",
is_hidden: bool = False,
is_locked: bool = False,
password: str = None,
):
conn = sqlite3.connect(DATABASE_PATH)
Expand All @@ -136,10 +163,10 @@ def db_insert_album(
).decode("utf-8")
cursor.execute(
"""
INSERT INTO albums (album_id, album_name, description, is_hidden, password_hash)
INSERT INTO albums (album_id, album_name, description, is_locked, password_hash)
VALUES (?, ?, ?, ?, ?)
""",
(album_id, album_name, description, int(is_hidden), password_hash),
(album_id, album_name, description, int(is_locked), password_hash),
)
conn.commit()
finally:
Expand All @@ -150,7 +177,7 @@ def db_update_album(
album_id: str,
album_name: str,
description: str,
is_hidden: bool,
is_locked: bool,
password: str = None,
):
conn = sqlite3.connect(DATABASE_PATH)
Expand All @@ -164,20 +191,20 @@ def db_update_album(
cursor.execute(
"""
UPDATE albums
SET album_name = ?, description = ?, is_hidden = ?, password_hash = ?
SET album_name = ?, description = ?, is_locked = ?, password_hash = ?
WHERE album_id = ?
""",
(album_name, description, int(is_hidden), password_hash, album_id),
(album_name, description, int(is_locked), password_hash, album_id),
)
else:
# Update without changing password
cursor.execute(
"""
UPDATE albums
SET album_name = ?, description = ?, is_hidden = ?
SET album_name = ?, description = ?, is_locked = ?
WHERE album_id = ?
""",
(album_name, description, int(is_hidden), album_id),
(album_name, description, int(is_locked), album_id),
)
conn.commit()
finally:
Expand All @@ -190,6 +217,20 @@ def db_delete_album(album_id: str):
cursor.execute("DELETE FROM albums WHERE album_id = ?", (album_id,))


def db_update_album_cover_image(album_id: str, cover_image_path: str):
"""Update the cover image path for an album"""
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
try:
cursor.execute(
"UPDATE albums SET cover_image_path = ? WHERE album_id = ?",
(cover_image_path, album_id),
)
conn.commit()
finally:
conn.close()


def db_get_album_images(album_id: str):
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
Expand Down Expand Up @@ -267,3 +308,15 @@ def verify_album_password(album_id: str, password: str) -> bool:
return bcrypt.checkpw(password.encode("utf-8"), row[0].encode("utf-8"))
finally:
conn.close()


def db_get_image_path(image_id: str) -> str | None:
"""Get the path of an image by its ID."""
conn = sqlite3.connect(DATABASE_PATH)
cursor = conn.cursor()
try:
cursor.execute("SELECT path FROM images WHERE id = ?", (image_id,))
result = cursor.fetchone()
return result[0] if result else None
finally:
conn.close()
99 changes: 86 additions & 13 deletions backend/app/routes/albums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from fastapi import APIRouter, HTTPException, status, Query, Body, Path
from fastapi import APIRouter, HTTPException, status, Body, Path
import uuid
from app.schemas.album import (
GetAlbumsResponse,
Expand All @@ -11,6 +11,7 @@
SuccessResponse,
ErrorResponse,
ImageIdsRequest,
SetCoverImageRequest,
Album,
)
from app.database.albums import (
Expand All @@ -24,24 +25,33 @@
db_add_images_to_album,
db_remove_image_from_album,
db_remove_images_from_album,
db_update_album_cover_image,
verify_album_password,
db_get_image_path,
)

router = APIRouter()


# GET /albums/ - Get all albums
# GET /albums/ - Get all albums (including locked ones)
@router.get("/", response_model=GetAlbumsResponse)
def get_albums(show_hidden: bool = Query(False)):
albums = db_get_all_albums(show_hidden)
def get_albums():
"""Get all albums. Always returns both locked and unlocked albums."""
albums = db_get_all_albums()
album_list = []
for album in albums:
# Get image count for each album
image_ids = db_get_album_images(album[0])
image_count = len(image_ids)

album_list.append(
Album(
album_id=album[0],
album_name=album[1],
description=album[2] or "",
is_hidden=bool(album[3]),
is_locked=bool(album[3]),
cover_image_path=album[5] if len(album) > 5 else None,
image_count=image_count,
)
)
Comment on lines 42 to 56
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_albums endpoint calls db_get_album_images() for every album in a loop (line 44), which results in N+1 database queries. For users with many albums, this could cause significant performance degradation. Consider optimizing this by either: (1) adding a database query that counts images per album in a single query using a JOIN or GROUP BY, or (2) caching the image counts in the albums table itself and updating it when images are added/removed.

Copilot uses AI. Check for mistakes.
return GetAlbumsResponse(success=True, albums=album_list)
Expand All @@ -64,7 +74,7 @@ def create_album(body: CreateAlbumRequest):
album_id = str(uuid.uuid4())
try:
db_insert_album(
album_id, body.name, body.description, body.is_hidden, body.password
album_id, body.name, body.description, body.is_locked, body.password
)
return CreateAlbumResponse(success=True, album_id=album_id)
except Exception as e:
Expand All @@ -91,11 +101,17 @@ def get_album(album_id: str = Path(...)):
)

try:
# Get image count for the album
image_ids = db_get_album_images(album_id)
image_count = len(image_ids)

album_obj = Album(
album_id=album[0],
album_name=album[1],
description=album[2] or "",
is_hidden=bool(album[3]),
is_locked=bool(album[3]),
cover_image_path=album[5] if len(album) > 5 else None,
image_count=image_count,
)
return GetAlbumResponse(success=True, data=album_obj)
except Exception as e:
Expand Down Expand Up @@ -127,11 +143,11 @@ def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...)
"album_id": album[0],
"album_name": album[1],
"description": album[2],
"is_hidden": bool(album[3]),
"is_locked": bool(album[3]),
"password_hash": album[4],
}

if album_dict["password_hash"]:
if album_dict["is_locked"]:
if not body.current_password:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
Expand All @@ -154,7 +170,7 @@ def update_album(album_id: str = Path(...), body: UpdateAlbumRequest = Body(...)

try:
db_update_album(
album_id, body.name, body.description, body.is_hidden, body.password
album_id, body.name, body.description, body.is_locked, body.password
)
return SuccessResponse(success=True, msg="Album updated successfully")
except Exception as e:
Expand Down Expand Up @@ -215,18 +231,18 @@ def get_album_images(
"album_id": album[0],
"album_name": album[1],
"description": album[2],
"is_hidden": bool(album[3]),
"is_locked": bool(album[3]),
"password_hash": album[4],
}

if album_dict["is_hidden"]:
if album_dict["is_locked"]:
if not body.password:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail=ErrorResponse(
success=False,
error="Password Required",
message="Password is required to access this hidden album.",
message="Password is required to access this locked album.",
).model_dump(),
)
if not verify_album_password(album_id, body.password):
Expand Down Expand Up @@ -355,3 +371,60 @@ def remove_images_from_album(
success=False, error="Failed to Remove Images", message=str(e)
).model_dump(),
)


# PUT /albums/{album_id}/cover - Set album cover image
@router.put("/{album_id}/cover", response_model=SuccessResponse)
def set_album_cover_image(
album_id: str = Path(...), body: SetCoverImageRequest = Body(...)
):
"""Set or update the cover image for an album"""
album = db_get_album(album_id)
if not album:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=ErrorResponse(
success=False,
error="Album Not Found",
message="No album exists with the provided ID.",
).model_dump(),
)

# Verify the image exists in the album
album_image_ids = db_get_album_images(album_id)
if body.image_id not in album_image_ids:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=ErrorResponse(
success=False,
error="Image Not In Album",
message="The specified image is not in this album.",
).model_dump(),
)

try:
# Get the image path from the database
image_path = db_get_image_path(body.image_id)

if not image_path:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=ErrorResponse(
success=False,
error="Image Not Found",
message="The specified image does not exist.",
).model_dump(),
)

db_update_album_cover_image(album_id, image_path)

return SuccessResponse(
success=True, msg="Album cover image updated successfully"
)
except Exception as e:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=ErrorResponse(
success=False, error="Failed to Set Cover Image", message=str(e)
).model_dump(),
)
Loading
Loading