Skip to content

Also SoA on root - #4710

Open
ildyria wants to merge 16 commits into
feature-61/bucket-album-approachfrom
feature-62/bucket-album-approach-tags-owners-root
Open

Also SoA on root#4710
ildyria wants to merge 16 commits into
feature-61/bucket-album-approachfrom
feature-62/bucket-album-approach-tags-owners-root

Conversation

@ildyria

@ildyria ildyria commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added root album listings with own/shared scopes, buckets, and rights.
    • Added smart, tag, person, and pinned album listings.
    • Added owner information to shared album results.
    • Smart-album covers and photo assets now support cached computed thumbnails.
  • Improvements

    • Album listing routes are now shorter and consolidated.
    • Album bucket data is recomputed after relevant settings changes.
    • Updated album access-permission response naming.
  • Breaking Changes

    • Removed album sorting by owner.
    • Replaced /children album endpoints with consolidated routes.

@coderabbitai

coderabbitai Bot commented Sep 2, 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: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 1556ba04-0de9-47e6-ad73-714b5a1f728d

📥 Commits

Reviewing files that changed from the base of the PR and between cd42112 and 5b0773d.

📒 Files selected for processing (26)
  • app/Actions/Album/StructOfArrays/BuildAlbumDataResource.php
  • app/Actions/Album/StructOfArrays/QueryChildrenForAlbum.php
  • app/Actions/Album/StructOfArrays/QueryChildrenForPerson.php
  • app/Actions/Album/StructOfArrays/QueryChildrenForTag.php
  • app/Actions/Album/StructOfArrays/QueryRightsForAlbum.php
  • app/Actions/Album/StructOfArrays/QueryRightsForMatchingAlbums.php
  • app/Actions/Album/StructOfArrays/Traits/BuildsAlbumCategoryResource.php
  • app/Actions/Album/StructOfArrays/Traits/GrantsAlbumRights.php
  • app/Http/Controllers/Gallery/AlbumAccessPermissionListController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumChildrenController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumPersonController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumPinnedController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumRootController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumSmartController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumTagController.php
  • app/Http/Requests/Album/GetAlbumCategoryRequest.php
  • app/Http/Resources/V3/AlbumAccessPermissionResource.php
  • app/Http/Resources/V3/AlbumCategoryResource.php
  • app/Http/Resources/V3/AlbumCategoryRightsResource.php
  • app/Http/Resources/V3/AlbumDataResource.php
  • app/Http/Resources/V3/AlbumRightsResource.php
  • database/migrations/2026_09_03_120000_fix_album_user_thumbs_album_id_type.php
  • resources/js/lychee.d.ts
  • resources/js/services/album-list-v3-service.ts
  • resources/js/v8/views/Sharing.vue
  • routes/api_v3.php
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/Http/Resources/V3/AlbumCategoryRightsResource.php
  • app/Http/Requests/Album/GetAlbumCategoryRequest.php

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Album listing API

Layer / File(s) Summary
Root and category listing flows
app/Http/Controllers/Gallery/AlbumListing/*, app/Http/Requests/Album/*, app/Http/Resources/V3/*
Adds scoped root listings, flat smart/tag/person/pinned listings, rights responses, and shared struct-of-arrays resources.
Sub-album endpoint consolidation
app/Http/Controllers/Gallery/AlbumListing/AlbumChildrenController.php, routes/api_v3.php, app/Actions/Album/StructOfArrays/*
Moves child data and rights queries into actions and renames /children routes to /Albums/{album_id}, /buckets, and /rights.
Sorting and root bucket recomputation
app/Enum/*, app/Jobs/RecomputeRootAlbumBucketsJob.php, app/Http/Controllers/Admin/SettingsController.php, database/migrations/*
Removes album owner sorting, migrates stored values to created_at, recomputes root buckets, and dispatches recomputation after relevant setting changes.
Caching and client contracts
app/Services/Cache/CacheKeyProvider.php, resources/js/lychee.d.ts, resources/js/services/*, resources/js/v7/*, resources/js/v8/*
Adds scope-aware cache keys, updates generated resource types, and aligns frontend fallback configuration and permission type names.
Validation and regression coverage
app/Http/Requests/Photo/GetPhotoAssetRequest.php, tests/Feature_v3/*, tests/Precomputing/*, tests/Unit/*
Allows cached smart-album covers during asset validation and tests routes, scopes, permissions, caching, migrations, and recomputation.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🔵 Low · up to 5b077

The PR is mergeable with minor follow-up for avoidable bucket-recomputation load and an inaccurate cache invalidation test fixture.

Poem

A rabbit reviews the albums in rows
New scopes hop where old children-paths rose
Buckets retune by year, month, and name
Cached covers now join the game
Routes rearrange with a soft carrot cheer
Tests guard every burrow near
“Ship it,” says Bunny, “the structure is clear!”

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 51.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 241 functions across 58 files. (1 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Docstring Coverage

Explanation

Docstring coverage is 51.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 241 functions across 58 files. (1 skipped: 1 unsupported.)


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.

@ildyria
ildyria marked this pull request as ready for review September 3, 2026 10:50
@ildyria
ildyria requested a review from a team as a code owner September 3, 2026 10:50

@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: 4

🧹 Nitpick comments (1)
app/Http/Controllers/Admin/SettingsController.php (1)

62-62: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Remove sorting_albums_order from ROOT_ALBUM_BUCKET_RECOMPUTE_CONFIGS.

AlbumBucketComputer::compute() uses the sorting column and granularity, not the sort order. An order-only change cannot alter bucket_id, but it can still queue a read and bulk upsert for every root album.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c24ce167-bc00-4ad6-80bf-b7d5ccdea9f4

📥 Commits

Reviewing files that changed from the base of the PR and between 3b56194 and 31d1451.

📒 Files selected for processing (48)
  • app/Actions/Albums/Top.php
  • app/Contracts/Http/Requests/RequestAttribute.php
  • app/Enum/AlbumListingScope.php
  • app/Enum/ColumnSortingAlbumType.php
  • app/Enum/ColumnSortingType.php
  • app/Http/Controllers/Admin/SettingsController.php
  • app/Http/Controllers/Gallery/AlbumBucketController.php
  • app/Http/Controllers/Gallery/AlbumChildrenDataController.php
  • app/Http/Controllers/Gallery/AlbumChildrenRightsController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumCategoryController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumChildrenController.php
  • app/Http/Controllers/Gallery/AlbumListing/AlbumRootController.php
  • app/Http/Requests/Album/GetAlbumCategoryRequest.php
  • app/Http/Requests/Album/GetScopedAlbumsRequest.php
  • app/Http/Requests/Photo/GetPhotoAssetRequest.php
  • app/Http/Resources/V3/AlbumCategoryListResource.php
  • app/Http/Resources/V3/AlbumCategoryRightsResource.php
  • app/Http/Resources/V3/AlbumChildrenDataResource.php
  • app/Http/Resources/V3/AlbumChildrenRightsResource.php
  • app/Jobs/RecomputeRootAlbumBucketsJob.php
  • app/Services/Cache/CacheKeyProvider.php
  • database/migrations/2026_09_02_120000_remove_owner_id_album_sorting.php
  • docs/specs/3-reference/api-design.md
  • docs/specs/3-reference/database-schema.md
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/spec.md
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/tasks.md
  • docs/specs/4-architecture/features/062-root-album-listing-struct-of-arrays/plan.md
  • docs/specs/4-architecture/features/062-root-album-listing-struct-of-arrays/spec.md
  • docs/specs/4-architecture/features/062-root-album-listing-struct-of-arrays/tasks.md
  • docs/specs/4-architecture/knowledge-map.md
  • docs/specs/4-architecture/open-questions.md
  • docs/specs/4-architecture/roadmap.md
  • resources/js/lychee.d.ts
  • resources/js/v7/views/gallery-panels/Search.vue
  • resources/js/v8/views/gallery-panels/Search.vue
  • routes/api_v3.php
  • tests/Feature_v2/Settings/OwnerIdSortingMigrationTest.php
  • tests/Feature_v2/Settings/UpdateSettingsTest.php
  • tests/Feature_v3/Album/AlbumBucketsV3Test.php
  • tests/Feature_v3/Album/AlbumCategoryV3Test.php
  • tests/Feature_v3/Album/AlbumChildrenDataV3Test.php
  • tests/Feature_v3/Album/AlbumChildrenRightsV3Test.php
  • tests/Feature_v3/Album/AlbumRootV3Test.php
  • tests/Feature_v3/Photo/PhotoAssetV3Test.php
  • tests/Precomputing/CoverSelection/RecomputeAlbumBucketsCommandTest.php
  • tests/Precomputing/CoverSelection/RecomputeRootAlbumBucketsJobTest.php
  • tests/Unit/Listeners/ManagedCacheAlbumListingInvalidatorTest.php
  • tests/Unit/Services/Cache/CacheKeyProviderTest.php
💤 Files with no reviewable changes (3)
  • app/Http/Controllers/Gallery/AlbumChildrenRightsController.php
  • app/Http/Controllers/Gallery/AlbumBucketController.php
  • app/Http/Controllers/Gallery/AlbumChildrenDataController.php

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread app/Http/Controllers/Gallery/AlbumListing/AlbumRootController.php
Comment thread app/Jobs/RecomputeRootAlbumBucketsJob.php

@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)
tests/Unit/Listeners/ManagedCacheAlbumListingInvalidatorTest.php (1)

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

Use the production tag set in this cache test.

rootAlbumBucketsKey(...) is used by AlbumRootController::buckets(), which adds albumListingGlobalTag() at app/Http/Controllers/Gallery/AlbumListing/AlbumRootController.php Line 256. This fixture omits that tag, so assertNotEvicted() tests a cache entry that production does not create. Add the global tag and assert eviction, or use a clearly synthetic key for the narrower tag-behavior test.

docs/specs/2-how-to/admin-dashboard.md (1)

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

Maintain the required documentation footer in every edited Markdown file.

The edited files do not all contain a current Last updated footer.

  • docs/specs/2-how-to/admin-dashboard.md#L58-L58: change the date from 2026-04-22 to 2026-09-03.
  • docs/specs/2-how-to/customize-v8-nuxt-ui.md#L88-L88: append --- and *Last updated: 2026-09-03*.
  • docs/specs/3-reference/managed-cache-service.md#L240-L240: append --- and *Last updated: 2026-09-03*.

As per coding guidelines, documentation files must end with an hr line followed by *Last updated: [date of the update]*.

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 8ec903cb-c8db-4ed6-b225-95c8fa9798c9

📥 Commits

Reviewing files that changed from the base of the PR and between dc60fa6 and cd42112.

📒 Files selected for processing (12)
  • app/Http/Controllers/Gallery/AlbumListing/AlbumRootController.php
  • app/Jobs/RecomputeRootAlbumBucketsJob.php
  • docs/specs/2-how-to/admin-dashboard.md
  • docs/specs/2-how-to/customize-v8-nuxt-ui.md
  • docs/specs/3-reference/api-design.md
  • docs/specs/3-reference/database-schema.md
  • docs/specs/3-reference/image-processing.md
  • docs/specs/3-reference/managed-cache-service.md
  • routes/api_v2.php
  • routes/api_v3.php
  • tests/Feature_v3/Album/AlbumRootV3Test.php
  • tests/Unit/Listeners/ManagedCacheAlbumListingInvalidatorTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
  • docs/specs/3-reference/database-schema.md
  • tests/Feature_v3/Album/AlbumRootV3Test.php
  • docs/specs/3-reference/api-design.md
  • routes/api_v3.php
  • app/Jobs/RecomputeRootAlbumBucketsJob.php

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

@ildyria

ildyria commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.09069% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.75%. Comparing base (c70babb) to head (52d8f20).

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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