Skip to content

Convert a memory into an album, plus album sorting and grid updates - #1456

Merged
rohan-pandeyy merged 10 commits into
AOSSIE-Org:mainfrom
rohan-pandeyy:feat/memory-to-album
Aug 4, 2026
Merged

Convert a memory into an album, plus album sorting and grid updates#1456
rohan-pandeyy merged 10 commits into
AOSSIE-Org:mainfrom
rohan-pandeyy:feat/memory-to-album

Conversation

@rohan-pandeyy

@rohan-pandeyy rohan-pandeyy commented Aug 4, 2026

Copy link
Copy Markdown
Member

Memories can now be turned into albums.

Hovering a memory card brings up the same three dot menu the album cards already have, with a Convert to Album action in it.
It opens a small dialog with the name prefilled from the memory title, and drops you on the new album once it is created.
The conversion is a single call to POST /albums/from-memory, which copies the memory's photos into a new album in one transaction, so a partial failure cannot leave an empty album behind.
The memory itself is left alone and can be converted again later under a different name.
Videos are skipped since albums only hold photos, and the dialog says so when the memory has any.

Grid layout

The album and memory grids changed their column count at breakpoints, so every card resized whenever the window did.
They now use the same auto fill grid the chronological and ranked galleries already use, where the cards keep a steady width and only the number of columns follows the window.
That class string was repeated across the three gallery files, so it now lives in a single shared constant.

Album sorting

Albums can be sorted by date created and by recently updated, alongside the existing name and photo count.
This adds two columns to the albums table, both migrated onto existing databases.
created_at is written once when the album is made and is never touched again.
updated_at moves when the album is renamed, when its description or lock changes, and when photos are added or removed.

Albums that predate these columns have no timestamps rather than invented ones.
They read as oldest, and the listing now orders by rowid so their fallback order matches the order they were actually created in.

The sort choice also survives a reload now, stored per page in localStorage the same way the theme already is.
The AI Tagging page picks it up through the same hook, since it uses the same dropdown and reset the same way.

Tests

Covers the new endpoint and its failure cases, the timestamp behaviour against a real database including the legacy migration path, the memory card menu, both new sorts, and the persistence hook.

Summary by CodeRabbit

New Features

  • Convert a memory’s photos into a new album from the memory actions menu; videos are excluded.
  • Album creation validates names, reports errors, and opens the new album after success.
  • Albums now display creation and last-updated dates.
  • Sort albums by creation date or recent activity, with preferences remembered.
  • Persist sorting preferences for albums and AI tagging.

Bug Fixes

  • Improved album ordering and handling of older albums without timestamps.
  • Standardized media grid layouts across galleries and memory views.

Adds a three-dot menu to the memory grid tiles, matching the album
cards, with a Convert to Album action. POST /albums/from-memory copies
the memory's photos into a new album in one transaction; the memory is
left untouched and clips are skipped, since albums hold images only.
The album and memory tiles already grow as the window widens, but the
actions menu and the lock badge were a fixed size, so they looked heavy
on a small window. They now step at lg and xl alongside the grid, and
keep their current size at xl.

Icons switch to size-* because the button variant forces size-4 onto any
svg without a size- class, which silently overrode the old h-5 w-5.
Both grids stepped their column count at breakpoints, so every card
resized as the window changed. They now use the same auto-fill grid the
chronological and ranked galleries use: cards hold a near-constant width
and the column count follows the window instead.

The class string was already repeated across the three galleries, so it
moves to a constant they all share.
Adds created_at and updated_at to albums, both migrated onto existing
databases. created_at is written once and never touched again; updated_at
moves on a rename, a description or lock change, and on photos being
added or removed, since to a user that is the album changing.

Albums predating the columns have no timestamps rather than fabricated
ones. They read as oldest, and the listing now orders by rowid so their
fallback order is the order they were made in.

The sort choice also survives a reload now, stored per surface in
localStorage like the theme. AI Tagging gets it from the same hook: it
uses the same dropdown and reset the same way.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 91317672-e3e5-4178-82d0-953d3ee19d1b

📥 Commits

Reviewing files that changed from the base of the PR and between 77cbc8c and 3ae563a.

📒 Files selected for processing (2)
  • backend/tests/test_album_utils.py
  • backend/tests/test_albums.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/tests/test_album_utils.py
  • backend/tests/test_albums.py

Walkthrough

Album storage now tracks lifecycle timestamps and supports transactional creation from memory photos. The frontend adds conversion controls, typed API access, persisted sorting, timestamp-based album ordering, and shared media-grid styling. Tests and OpenAPI documentation cover the new behavior.

Changes

Album enhancements

Layer / File(s) Summary
Album timestamps and lifecycle
backend/app/database/albums.py, backend/app/routes/albums.py, backend/app/schemas/album.py, backend/tests/test_albums.py, backend/tests/test_albums_db.py
Album rows now use typed dictionaries with nullable timestamps. Existing tables migrate timestamp columns. Album and image changes update updated_at when data changes.
Memory-to-album backend flow
backend/app/routes/albums.py, backend/app/utils/albums.py, backend/app/schemas/album.py, backend/tests/test_albums.py, backend/tests/test_album_utils.py, docs/backend/backend_python/openapi.json
POST /albums/from-memory validates the request and memory, creates an album with memory photos, and returns the album ID and image count.
Memory conversion interface
frontend/src/components/Memories/*, frontend/src/api/api-functions/albums.ts, frontend/src/api/apiEndpoints.ts, frontend/src/types/Album.ts, frontend/src/pages/Memories/Memories.tsx, frontend/src/components/Memories/__tests__/*
The frontend adds a conversion dialog, MemoryCard action, typed API call, page wiring, and interaction tests.
Persisted album and tagging sorting
frontend/src/hooks/usePersistedSort.ts, frontend/src/pages/Album/*, frontend/src/pages/AITagging/AITagging.tsx, frontend/src/components/GallerySortDropdown.tsx, frontend/src/components/Media/*, frontend/src/constants/layout.ts, frontend/src/pages/__tests__/Album.test.tsx, frontend/src/hooks/__tests__/usePersistedSort.test.tsx
Album sorting supports creation and update timestamps. Sort selections persist and reject invalid stored values. Legacy albums without timestamps sort last. Shared media-grid classes replace duplicated grid declarations.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MemoryCard
  participant ConvertMemoryToAlbumDialog
  participant createAlbumFromMemory
  participant albums_endpoint
  participant album_util_create_from_memory
  MemoryCard->>ConvertMemoryToAlbumDialog: choose Convert to Album
  ConvertMemoryToAlbumDialog->>createAlbumFromMemory: submit memory_id and album name
  createAlbumFromMemory->>albums_endpoint: POST /albums/from-memory
  albums_endpoint->>album_util_create_from_memory: validate memory and create album
  album_util_create_from_memory-->>albums_endpoint: return album ID and image count
  albums_endpoint-->>ConvertMemoryToAlbumDialog: return creation result
Loading

Possibly related PRs

Suggested labels: Python, TypeScript/JavaScript, Documentation

Poem

A rabbit taps “Convert” with care,
Photos hop to albums there.
Timestamps bloom, sorts remember,
Old rows rest with values tender.
Grids align from row to row—
Carrots cheer the steady flow.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: memory-to-album conversion, album sorting, and grid layout updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/app/database/albums.py (1)

304-308: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Update updated_at only after an album-image row changes.

Both operations can affect zero rows but still call _touch_album. This records a false album update and changes update-date sorting.

  • backend/app/database/albums.py#L304-L308: call _touch_album only when INSERT OR IGNORE inserts at least one row.
  • backend/app/database/albums.py#L336-L340: call _touch_album only when the bulk deletion removes at least one row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/albums.py` around lines 304 - 308, In
backend/app/database/albums.py lines 304-308, update the album-image insertion
flow to call _touch_album only when cursor.executemany inserts at least one row,
using the database row-change result. Apply the same conditional behavior to the
bulk deletion flow at lines 336-340, calling _touch_album only when at least one
row is removed.
🧹 Nitpick comments (1)
backend/tests/test_albums.py (1)

602-612: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Verify that the persisted album ID matches the response ID.

The test validates both values independently but does not compare them. A regression could pass one UUID to db_create_album_with_images and return another UUID to the client.

Proposed fix
             album_id, name, description, image_ids = mock_create.call_args.args
+            assert album_id == json_response["data"]["album_id"]
             assert name == "Paris 2022"

As per path instructions, ensure that critical functionality has comprehensive automated tests. <path_instructions>

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_albums.py` around lines 602 - 612, Update the album
creation test around mock_create.call_args.args to capture the persisted
album_id and assert it equals json_response["data"]["album_id"], while retaining
UUID validation and the existing name, description, and image_ids assertions.

Sources: Path instructions, Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/app/database/albums.py`:
- Around line 90-101: Define an AlbumRow TypedDict for the eight album columns,
then update db_get_all_albums and the two related database functions to return
AlbumRow records with accurate list return annotations. Map each fetched row by
column name rather than exposing positional tuples, while preserving the
existing query ordering and values.

In `@backend/app/routes/albums.py`:
- Around line 103-104: Complete the route contract on create_album_from_memory
by adding the CreateAlbumFromMemoryResponse return annotation and documenting
400, 404, 409, and 500 responses with ErrorResponse models. Preserve the
existing Pydantic request body signature without adding redundant body metadata.

In `@backend/app/schemas/album.py`:
- Around line 37-39: Update CreateAlbumFromMemoryRequest with a validator that
trims memory_id and name and rejects values that are blank after stripping,
while preserving the normalized values. Add a backend test covering
whitespace-only input for both fields.

In `@frontend/src/components/Memories/ConvertMemoryToAlbumDialog.tsx`:
- Around line 40-45: Update the useEffect that initializes name and error so it
depends on both memory and isOpen, and performs the reset only on an opening
transition while a memory is present. Preserve existing behavior for memory
changes, and add a dialog test covering close/reopen with the same memory
instance to verify stale state is cleared.

In `@frontend/src/hooks/__tests__/usePersistedSort.test.ts`:
- Around line 1-46: Rename the test file containing the usePersistedSort suite
to use the required .test.tsx extension, preserving its contents and test
behavior.

---

Outside diff comments:
In `@backend/app/database/albums.py`:
- Around line 304-308: In backend/app/database/albums.py lines 304-308, update
the album-image insertion flow to call _touch_album only when cursor.executemany
inserts at least one row, using the database row-change result. Apply the same
conditional behavior to the bulk deletion flow at lines 336-340, calling
_touch_album only when at least one row is removed.

---

Nitpick comments:
In `@backend/tests/test_albums.py`:
- Around line 602-612: Update the album creation test around
mock_create.call_args.args to capture the persisted album_id and assert it
equals json_response["data"]["album_id"], while retaining UUID validation and
the existing name, description, and image_ids assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 237f8e0a-89ce-4c84-82f5-9ff3da7f1ac0

📥 Commits

Reviewing files that changed from the base of the PR and between 78e7dc3 and 0a5b79e.

📒 Files selected for processing (24)
  • backend/app/database/albums.py
  • backend/app/routes/albums.py
  • backend/app/schemas/album.py
  • backend/tests/test_albums.py
  • backend/tests/test_albums_db.py
  • docs/backend/backend_python/openapi.json
  • frontend/src/api/api-functions/albums.ts
  • frontend/src/api/apiEndpoints.ts
  • frontend/src/components/GallerySortDropdown.tsx
  • frontend/src/components/Media/ChronologicalGallery.tsx
  • frontend/src/components/Media/ChronologicalVideoGallery.tsx
  • frontend/src/components/Media/RankedGallery.tsx
  • frontend/src/components/Memories/ConvertMemoryToAlbumDialog.tsx
  • frontend/src/components/Memories/MemoryCard.tsx
  • frontend/src/components/Memories/__tests__/MemoryCard.test.tsx
  • frontend/src/constants/layout.ts
  • frontend/src/hooks/__tests__/usePersistedSort.test.ts
  • frontend/src/hooks/usePersistedSort.ts
  • frontend/src/pages/AITagging/AITagging.tsx
  • frontend/src/pages/Album/Album.tsx
  • frontend/src/pages/Album/AlbumDetail.tsx
  • frontend/src/pages/Memories/Memories.tsx
  • frontend/src/pages/__tests__/Album.test.tsx
  • frontend/src/types/Album.ts

Comment thread backend/app/database/albums.py Outdated
Comment thread backend/app/routes/albums.py Outdated
Comment thread backend/app/schemas/album.py
Comment thread frontend/src/components/Memories/ConvertMemoryToAlbumDialog.tsx Outdated
Comment thread frontend/src/hooks/__tests__/usePersistedSort.test.tsx
Album reads now return an AlbumRow TypedDict instead of positional
tuples, so the routes look columns up by name rather than by index.

The from-memory route declares its error responses and return type, and
its request rejects names and ids that are blank once trimmed.

Adding images that are already in an album, or removing ones that are
not, no longer marks the album as updated, matching what the single
image removal already did.

The convert dialog resets on opening rather than only when the memory
changes, so a name typed and abandoned does not come back.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

1 similar comment
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
backend/app/schemas/album.py (1)

41-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Annotate the validator signature completely.

check_not_blank leaves cls untyped. Annotate cls with the model class type to meet the backend signature requirement. Confirm that the selected annotation is accepted by the installed Pydantic version.

As per coding guidelines, “Annotate function signatures and return types accurately.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/schemas/album.py` around lines 41 - 48, Update the
check_not_blank validator signature to annotate cls with the enclosing album
schema model class, while preserving the existing value and return annotations
and validation behavior. Use the class annotation form supported by the
installed Pydantic version.

Source: Coding guidelines

frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx (1)

10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a complete typed memory fixture.

as MemoryCard hides missing required fields. Define memory as a complete MemoryCard, or use a typed fixture factory that returns MemoryCard.

As per coding guidelines, “do not use as to silence real type errors.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx`
around lines 10 - 16, Replace the casted memory fixture in
ConvertMemoryToAlbumDialog tests with a complete, type-safe MemoryCard object,
including every required field, or use an existing typed fixture factory that
returns MemoryCard. Remove the `as MemoryCard` assertion while preserving the
fixture values needed by the tests.

Source: Coding guidelines

backend/tests/test_albums.py (1)

23-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Type the album-row fixture.

album_row accepts and returns bare dictionaries. Define or reuse a TypedDict for this database-row shape. Annotate the optional timestamp and cover-path parameters too. This prevents fixture keys from drifting from the database contract.

As per coding guidelines, “represent table rows with TypedDict classes rather than bare dictionaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_albums.py` around lines 23 - 34, Update the album_row
fixture to use a TypedDict representing the complete album database-row shape,
including album_id, album_name, description, is_locked, password_hash,
cover_image_path, created_at, and updated_at. Annotate album_row’s album
parameter and return type with that TypedDict, and give cover_image_path,
created_at, and updated_at explicit optional types consistent with the row
contract.

Source: Coding guidelines

backend/app/database/albums.py (1)

2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace deprecated typing.List and typing.Tuple aliases.

ruff check --select UP035 flags these imports. Use tuple[Any, ...] for _to_album_row(row: ...), list[AlbumRow] for db_get_all_albums(), and remove List and Tuple from the import line. The module already has supported runtime annotations for these generics.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/albums.py` at line 2, Replace deprecated List and Tuple
annotations in _to_album_row and db_get_all_albums with built-in generic forms
tuple[Any, ...] and list[AlbumRow], respectively, and remove List and Tuple from
the typing imports.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/app/database/albums.py`:
- Around line 126-136: Replace the direct sqlite3.connect(DATABASE_PATH) calls
in db_get_all_albums, db_get_album_by_name, and db_get_album with the
module-private _connect() helper, preserving each function’s existing query
logic and finally-based connection cleanup.

In `@backend/tests/test_albums.py`:
- Around line 342-351: Update the password fixtures in album_row to pass byte
literals directly to bcrypt.hashpw, replacing the redundant "oldpass".encode()
and "correctpass".encode() calls with b"oldpass" and b"correctpass".

In `@docs/backend/backend_python/openapi.json`:
- Around line 499-537: The error response schemas for statuses 400, 404, 409,
and 500 currently describe ErrorResponse at the root, but HTTPException
responses wrap it under detail. Update the referenced
app__schemas__album__ErrorResponse schema or these response definitions so the
documented envelope requires a detail property containing success, message, and
error, preserving the existing error payload fields.

---

Nitpick comments:
In `@backend/app/database/albums.py`:
- Line 2: Replace deprecated List and Tuple annotations in _to_album_row and
db_get_all_albums with built-in generic forms tuple[Any, ...] and
list[AlbumRow], respectively, and remove List and Tuple from the typing imports.

In `@backend/app/schemas/album.py`:
- Around line 41-48: Update the check_not_blank validator signature to annotate
cls with the enclosing album schema model class, while preserving the existing
value and return annotations and validation behavior. Use the class annotation
form supported by the installed Pydantic version.

In `@backend/tests/test_albums.py`:
- Around line 23-34: Update the album_row fixture to use a TypedDict
representing the complete album database-row shape, including album_id,
album_name, description, is_locked, password_hash, cover_image_path, created_at,
and updated_at. Annotate album_row’s album parameter and return type with that
TypedDict, and give cover_image_path, created_at, and updated_at explicit
optional types consistent with the row contract.

In
`@frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx`:
- Around line 10-16: Replace the casted memory fixture in
ConvertMemoryToAlbumDialog tests with a complete, type-safe MemoryCard object,
including every required field, or use an existing typed fixture factory that
returns MemoryCard. Remove the `as MemoryCard` assertion while preserving the
fixture values needed by the tests.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 42bc85a9-d1e7-4f4a-8a78-21f00bfcebf0

📥 Commits

Reviewing files that changed from the base of the PR and between 0a5b79e and 4a3371c.

📒 Files selected for processing (9)
  • backend/app/database/albums.py
  • backend/app/routes/albums.py
  • backend/app/schemas/album.py
  • backend/tests/test_albums.py
  • backend/tests/test_albums_db.py
  • docs/backend/backend_python/openapi.json
  • frontend/src/components/Memories/ConvertMemoryToAlbumDialog.tsx
  • frontend/src/components/Memories/__tests__/ConvertMemoryToAlbumDialog.test.tsx
  • frontend/src/hooks/__tests__/usePersistedSort.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/app/routes/albums.py
  • frontend/src/components/Memories/ConvertMemoryToAlbumDialog.tsx

Comment thread backend/app/database/albums.py
Comment thread backend/tests/test_albums.py
Comment thread docs/backend/backend_python/openapi.json
All album reads now go through a module-private _connect(), matching the
videos module, so the foreign-key pragma is never skipped. The three
select statements are built once from the shared column list rather than
interpolated at each call site.

The from-memory route documents its errors with an envelope model, since
HTTPException nests the payload under detail and the generated spec was
describing a shape no client receives. Its duplicated conflict and
internal-error blocks collapse into two helpers, and the raises inside
except blocks now chain the original error.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

1 similar comment
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
backend/tests/test_albums.py (1)

23-36: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Define a TypedDict for album database rows.

album_row creates database row-shaped mappings that route mocks consume. Define an AlbumRow(TypedDict) and annotate the helper input and return value. This prevents fixture keys from drifting from the database-row contract.

As per coding guidelines, “In Python, annotate function signatures and return types; represent table rows with TypedDict classes rather than bare dictionaries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_albums.py` around lines 23 - 36, Define an AlbumRow
TypedDict for the database-row keys returned by album_row, including appropriate
value types and nullable fields for cover_image_path, created_at, and
updated_at. Update album_row’s album parameter and return annotation to use the
relevant typed structures while preserving the existing row contents.

Source: Coding guidelines

backend/app/routes/albums.py (1)

115-168: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Move the conversion workflow to app/utils/.

The route performs memory lookup, image selection, duplicate detection, and album-creation coordination. Extract this workflow to a utility helper. Keep the route limited to request parsing, response mapping, and HTTP error translation.

As per coding guidelines, “Routes handle HTTP and validation only; business logic belongs in app/utils/ and data access belongs in app/database/.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/routes/albums.py` around lines 115 - 168, The
create_album_from_memory route currently contains the full conversion workflow.
Extract the memory lookup, image selection, duplicate detection, UUID
generation, and album creation coordination into a dedicated helper under
app/utils, preserving its existing behavior and database calls; keep
create_album_from_memory limited to invoking the helper, mapping its result to
CreateAlbumFromMemoryResponse, and translating domain failures into HTTP errors.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@backend/app/routes/albums.py`:
- Around line 115-168: The create_album_from_memory route currently contains the
full conversion workflow. Extract the memory lookup, image selection, duplicate
detection, UUID generation, and album creation coordination into a dedicated
helper under app/utils, preserving its existing behavior and database calls;
keep create_album_from_memory limited to invoking the helper, mapping its result
to CreateAlbumFromMemoryResponse, and translating domain failures into HTTP
errors.

In `@backend/tests/test_albums.py`:
- Around line 23-36: Define an AlbumRow TypedDict for the database-row keys
returned by album_row, including appropriate value types and nullable fields for
cover_image_path, created_at, and updated_at. Update album_row’s album parameter
and return annotation to use the relevant typed structures while preserving the
existing row contents.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ace9df07-982e-4401-b1b8-5ea8edb5bfc2

📥 Commits

Reviewing files that changed from the base of the PR and between 4a3371c and e37efa7.

📒 Files selected for processing (6)
  • backend/app/database/albums.py
  • backend/app/routes/albums.py
  • backend/app/schemas/album.py
  • backend/tests/test_albums.py
  • docs/backend/backend_python/openapi.json
  • frontend/src/hooks/usePersistedSort.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/hooks/usePersistedSort.ts
  • backend/app/database/albums.py

The route was doing the memory lookup, the image selection, the
duplicate-name check and the album creation itself. That work moves to
album_util_create_from_memory, which raises a small set of domain errors
the route translates into status codes, leaving it to handle HTTP only.

Isolating it exposed an untested branch: the insert can still hit the
unique constraint after the name check passes, and only a name that is
taken on re-check means a conflict. Anything else has to surface rather
than be reported as a duplicate. Both paths now have tests.

The row helper in the album route tests builds the production AlbumRow
instead of a look-alike dict, so a column added to the table fails there
rather than drifting.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

1 similar comment
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/tests/test_albums.py (1)

565-667: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required backend test annotations.

  • backend/tests/test_albums.py#L565-L667: Add exact fixture parameter types and -> None to every method in TestCreateAlbumFromMemory.
  • backend/tests/test_album_utils.py#L18-42: Import and use AlbumFromMemoryResult for run_with_integrity_error, and add -> None to both new test methods.

Run pre-commit run --config .pre-commit-config.yaml --all-files from the repository root after updating these annotations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_albums.py` around lines 565 - 667, Update every test
method in backend/tests/test_albums.py lines 565-667 within
TestCreateAlbumFromMemory to use the exact fixture parameter types and an
explicit -> None return annotation. In backend/tests/test_album_utils.py lines
18-42, import and apply AlbumFromMemoryResult as the return type for
run_with_integrity_error, and add -> None to both new test methods. Run
pre-commit with the repository’s .pre-commit-config.yaml across all files.

Sources: Coding guidelines, Path instructions

🧹 Nitpick comments (1)
backend/tests/test_albums.py (1)

9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the Python 3.9 built-in generic.

Replace Dict[str, Any] with dict[str, Any] in album_row and remove Dict from from typing import Any, Dict, Optional. Keep Optional[str] because this module does not use postponed annotations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_albums.py` at line 9, Update album_row to use the Python
3.9 built-in generic dict[str, Any] instead of Dict[str, Any], and remove Dict
from the typing import while retaining Any and Optional for the module’s
existing annotations.

Sources: Coding guidelines, Path instructions, Learnings, Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/app/utils/albums.py`:
- Around line 7-8: Move the database-orchestration workflow currently
implemented by album_util_create_from_memory from app/utils/albums.py into
app/database/, including its memory/image reads, album-state checks, album
creation, and transaction ownership. Update callers and imports to use the
database-layer function, leaving app/utils/albums.py with only pure
transformations.

---

Outside diff comments:
In `@backend/tests/test_albums.py`:
- Around line 565-667: Update every test method in backend/tests/test_albums.py
lines 565-667 within TestCreateAlbumFromMemory to use the exact fixture
parameter types and an explicit -> None return annotation. In
backend/tests/test_album_utils.py lines 18-42, import and apply
AlbumFromMemoryResult as the return type for run_with_integrity_error, and add
-> None to both new test methods. Run pre-commit with the repository’s
.pre-commit-config.yaml across all files.

---

Nitpick comments:
In `@backend/tests/test_albums.py`:
- Line 9: Update album_row to use the Python 3.9 built-in generic dict[str, Any]
instead of Dict[str, Any], and remove Dict from the typing import while
retaining Any and Optional for the module’s existing annotations.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c3672d01-ff3c-4e23-8576-1b21f93660c1

📥 Commits

Reviewing files that changed from the base of the PR and between e37efa7 and 77cbc8c.

📒 Files selected for processing (4)
  • backend/app/routes/albums.py
  • backend/app/utils/albums.py
  • backend/tests/test_album_utils.py
  • backend/tests/test_albums.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/app/routes/albums.py

Comment thread backend/app/utils/albums.py
Exact fixture parameter types and return annotations on the conversion
tests and the two util tests, and the row helper takes a built-in
dict generic.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ No issue was linked in the PR description.
Please make sure to link an issue (e.g., 'Fixes #issue_number')

@rohan-pandeyy
rohan-pandeyy merged commit 5b4c41d into AOSSIE-Org:main Aug 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant