Skip to content

feat: search other pages when photo not found in suggested page#4311

Merged
ildyria merged 2 commits intomasterfrom
copilot/fetch-previous-and-next-photo-pages
Apr 22, 2026
Merged

feat: search other pages when photo not found in suggested page#4311
ildyria merged 2 commits intomasterfrom
copilot/fetch-previous-and-next-photo-pages

Conversation

@ildyria
Copy link
Copy Markdown
Member

@ildyria ildyria commented Apr 21, 2026

Summary by CodeRabbit

  • New Features

    • Enabled direct photo permalinks within albums. The app will load the necessary pages to display a specific photo regardless of its paginated position.
    • Loading now accepts photo context so album fetches prioritize locating a requested photo.
  • Bug Fixes / Improvements

    • Added navigation race guards to prevent stale loads when switching albums.
    • Optimized background preloading to avoid duplicate page requests during targeted photo searches.

@ildyria ildyria requested a review from a team as a code owner April 21, 2026 22:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

The album loader now accepts an optional photoId; when given and not found on the initial page, the store sequentially loads other pages (backwards then forwards) until the photo appears, with albumId checks to abort on navigation and skips background prepends to avoid duplicate loads.

Changes

Cohort / File(s) Summary
Album Store State Management
resources/js/stores/AlbumState.ts
Added new async action _searchPhotoInPages(photoId, startPage, requestedAlbumId) to sequentially load missing pages until a photo is found. Extended load(startPage = 1)load(startPage = 1, photoId?). Adjusted load to enqueue the new search when needed and to skip background prepends during an active search; added navigation race guards.
Album View Integration
resources/js/views/gallery-panels/Album.vue
Call to albumStore.load updated to pass the current photoId along with startPage, enabling the store-side sequential search behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hop through pages, one by one,
Backwards then forwards until it's done.
I guard the path and skip the repeat,
Till the hidden photo and I finally meet. 📸

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
resources/js/stores/AlbumState.ts (1)

351-375: Sequential full-album scan can be an expensive fallback; consider a server-side photo→page lookup.

For large albums where the hinted ?page=N is wrong, this walks every page serially, with each loadPhotos awaiting the network round-trip before deciding whether to continue. On a 50-page album with the photo on page 50, that's 49 sequential requests — visible latency plus extra DB/IO load server-side.

Options worth weighing:

  • Add a cheap endpoint like GET /Album/{id}/photos/locate?photoId=X&perPage=Y returning { page }, then load only that page. This collapses the worst case from O(pages) round-trips to 2.
  • Alternatively, parallelize the scan with a small concurrency window and AbortController once the photo is found (still O(N) server work, but wall-clock better).
  • Capping the search with a sensible max (e.g., search ±K pages, then give up and fall back to the initial page view) would at least bound the waterfall.

Happy to sketch an implementation if useful.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afa74599-44d6-4181-9852-94b6f528d649

📥 Commits

Reviewing files that changed from the base of the PR and between 808efd0 and 6dc4f00.

📒 Files selected for processing (2)
  • resources/js/stores/AlbumState.ts
  • resources/js/views/gallery-panels/Album.vue

Comment thread resources/js/stores/AlbumState.ts
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.51%. Comparing base (bda7afb) to head (5e1d5d7).
⚠️ Report is 2 commits behind head on master.

🚀 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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 497ef3b9-90ef-43e2-8a09-d3bfb67c5b51

📥 Commits

Reviewing files that changed from the base of the PR and between 6dc4f00 and 5e1d5d7.

📒 Files selected for processing (1)
  • resources/js/stores/AlbumState.ts

Comment thread resources/js/stores/AlbumState.ts
@ildyria ildyria merged commit 3d0cfac into master Apr 22, 2026
45 checks passed
@ildyria ildyria deleted the copilot/fetch-previous-and-next-photo-pages branch April 22, 2026 20:03
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.

2 participants