Add pagination & refactoring to reduce the number of queries #3952
Conversation
📝 WalkthroughWalkthroughAdds paginated album endpoints and frontend pagination UI/state, permission-driven photo downgrade flags, repository layers, new resources and requests, pagination config/enum and migrations, SizeVariants/Watermarker refactors, many controller/resource call-site updates, tests, and extensive documentation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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. Comment |
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
…points - Add config property to AlbumHeadResource for frontend rendering - Create album-service methods: getHead(), getAlbums(), getPhotos() - Extend AlbumState store with pagination state and actions: - loadHead(), loadAlbums(), loadPhotos() for new endpoints - loadMorePhotos(), loadMoreAlbums() for infinite scroll/load more - loadPaginated() to orchestrate parallel loading - Add PhotosState.appendPhotos() for timeline-aware photo appending - Create PaginationLoadMore.vue and PaginationInfiniteScroll.vue components - Create usePagination.ts composable for shared pagination logic - Integrate PaginationLoadMore into AlbumPanel for photos and albums - Update tasks.md with implementation progress Spec impact: docs/specs/4-architecture/features/007-pagination/tasks.md updated Co-Authored-By: Claude Opus 4.5 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/specs/4-architecture/open-questions.md (1)
17-1343: Remove resolved questions per coding guidelines.The coding guidelines explicitly state: "Track unresolved high- and medium-impact questions here. Remove each row as soon as it is resolved, ensuring the answer is captured first in the governing spec's normative sections."
Currently, the Question Details section (lines 17-1343) contains numerous resolved questions marked with strikethrough. These should be removed entirely now that they're resolved and captured in the governing specs. Keeping resolved questions in this file:
- Violates the documented process
- Makes the file unnecessarily large and harder to navigate
- Obscures any future active questions
The Active Questions table correctly shows "No active questions" — the Question Details section should reflect this by removing all resolved entries.
Based on coding guidelines: "Remove each row as soon as it is resolved, ensuring the answer is captured first in the governing spec's normative sections."
app/Http/Resources/Models/PhotoResource.php (1)
68-93: Constructor signature change correctly applied; however, fix eager-loading of tags in FlowItemResource.The constructor signature updates are in place across all call sites (PhotoController, HasPrepPhotoCollection, TimelineResource, ResultsResource, GetTagWithPhotos, FrameController). However,
FlowItemResource.setPhotos()at line 110 loads['all_photos', 'all_photos.size_variants', 'all_photos.palette', 'all_photos.statistics']but omits'all_photos.tags'. SincePhotoResource.__construct()accesses$photo->tags->pluck('name')->all(), this will trigger N+1 queries whenflow_include_photos_from_childrenis true. Add'all_photos.tags'to theload()call.resources/js/composables/contextMenus/contextMenu.ts (1)
109-131: Type cast toAlbumResourceshould beHeadAlbumResource.The
Selectors.albumtype was updated to useHeadAlbumResource | HeadTagAlbumResource | HeadSmartAlbumResource, but line 110 still casts toAlbumResource. This type mismatch could cause TypeScript errors or runtime issues.🔧 Suggested fix
if (selectors.config?.value?.is_model_album === true && selectors.album !== undefined) { - const parent_album = selectors.album.value as App.Http.Resources.Models.AlbumResource; + const parent_album = selectors.album.value as App.Http.Resources.Models.HeadAlbumResource;
🧹 Nitpick comments (25)
resources/js/components/forms/album/AlbumVisibility.vue (1)
73-73: Optional: Fix typo in variable name.The variable name
can_pasword_protecthas a typo (missing 's' in 'password'). Consider renaming tocan_password_protectfor correctness.♻️ Suggested refactor
Update the variable name on line 161:
-const can_pasword_protect = ref<boolean>(albumStore.album?.rights.can_pasword_protect ?? false); +const can_password_protect = ref<boolean>(albumStore.album?.rights.can_password_protect ?? false);And update the references on lines 73 and 91:
- v-if="!can_pasword_protect && is_password_required" + v-if="!can_password_protect && is_password_required"- v-else-if="can_pasword_protect" + v-else-if="can_password_protect"Note: This may also require updating the backend property name if it uses the same typo.
Also applies to: 91-91, 161-161
resources/js/views/gallery-panels/Tag.vue (1)
28-28: Remove or annotate commented-out code.Multiple sections are commented out:
PhotoEditcomponent (line 28), its import (line 93),toggleEditfunction (lines 197-203), and privileged keyboard shortcuts (lines 232-234).Leaving commented code in the codebase creates maintenance burden. Consider either:
- Removing the code entirely if no longer needed
- Adding a TODO comment explaining why it's temporarily disabled and when it should be restored
Also applies to: 93-93, 197-203, 232-234
resources/js/stores/PhotosState.ts (1)
42-48: Consider maintaining chronological order for appended timeline groups.When appending new timeline groups that don't match existing headers, they are pushed to the end of the array (line 47). If photos are loaded in paginated batches, this should maintain order. However, if appended photos have timeline dates that precede existing groups, the display order may be incorrect.
If chronological ordering is required, consider sorting
photosTimelineby the timeline date after merging.docs/specs/4-architecture/features/007-pagination/tasks.md (1)
60-64: Consider unit test coverage for maintainability.Multiple layers lack unit test coverage:
- Repository pagination methods (T-007-07)
- Service methods (T-007-21)
- Store pagination logic (T-007-23)
- Pagination UI components (T-007-28)
While integration tests provide some coverage, unit tests improve:
- Faster test execution
- Easier debugging of failures
- Better documentation of expected behavior
- Safer refactoring
Consider creating at least unit tests for the repository and store logic as a recommended follow-up.
Also applies to: 157-161, 171-175, 204-208
app/Http/Resources/Timeline/TimelineResource.php (1)
79-80: Consider injecting ConfigManager instead of resolving on every call.The
resolve()call happens on every invocation offromData. Since this is a static factory method that could be called frequently during pagination, consider either:
- Injecting ConfigManager as a constructor parameter if TimelineResource becomes a service
- Using
request()->configs()directly (already available in this codebase pattern)♻️ Proposed refactor using request()->configs()
- $config_manager = resolve(ConfigManager::class); - $should_downgrade = !$config_manager->getValueAsBool('grants_full_photo_access'); + $should_downgrade = !request()->configs()->getValueAsBool('grants_full_photo_access');resources/js/components/forms/album/ConfirmSharingDialog.vue (1)
4-4: LGTM: Refactored to use Tailwind spacing units.The width classes changed from arbitrary values
md:w-[500px] max-w-[500px]to standard Tailwind spacingmd:w-125 max-w-125. Both resolve to 500px (125 × 4px), so this is functionally equivalent while following Tailwind conventions.app/Http/Controllers/Gallery/AlbumController.php (1)
89-98: Dead assignment on line 89.The
$album_resource = null;assignment is immediately overwritten by the match expression and can be removed.🧹 Suggested cleanup
- $album_resource = null; - - $album_resource = match (true) { + $album_resource = match (true) { $request->album() instanceof BaseSmartAlbum => new SmartAlbumResource($request->album()), $request->album() instanceof TagAlbum => new TagAlbumResource($request->album()), $request->album() instanceof Album => new AlbumResource($request->album()), // @codeCoverageIgnoreStart default => throw new LycheeLogicException('This should not happen'), // @codeCoverageIgnoreEnd };resources/js/lychee.d.ts (1)
582-589: TypeScript generation creates redundantnull | nulltype union.The
statisticsproperty inHeadSmartAlbumResourceandSmartAlbumResourceis generated asnull | null, which is redundant and should be simplified tonull. This occurs because the PHP source intentionally declarespublic null $statistics = null;to unify API responses across resource types, and the Spatie TypeScriptTransformer generates the union type redundantly. While this doesn't cause functional issues, consider:
- Filing an issue with Spatie TypeScriptTransformer if this is a bug in their null type handling, or
- Adding a type replacement rule in
config/typescript-transformer.phpif there's a workaround available.resources/js/components/pagination/PaginationLoadMore.vue (1)
1-5: Consider simplifying Button props.The Button has both
:loading="loading"and:disabled="loading". PrimeVue's Button component typically disables interactions automatically when in loading state, making the explicit:disabledprop potentially redundant.♻️ Potential simplification
- <Button :label="buttonLabel" :loading="loading" :disabled="loading" severity="secondary" class="rounded" @click="emit('loadMore')" /> + <Button :label="buttonLabel" :loading="loading" severity="secondary" class="rounded" @click="emit('loadMore')" />app/Http/Resources/Embed/EmbedAlbumResource.php (1)
37-41: Policy-driven downgrade logic correctly applied.The integration of Gate-based access control for embedded photos is correct and consistent with the PR's broader refactoring pattern.
Minor observation: The negation here uses
!Gate::check(...)whileapp/Actions/Album/PositionData.phpuses=== false. Both are functionally correct for boolean results, but consistent style across the codebase would be slightly cleaner.app/Http/Controllers/Gallery/AlbumChildrenController.php (1)
23-27: Remove misleading constructor comment.The comment "Prevent instantiation" is incorrect. This is a standard Laravel dependency injection pattern that enables instantiation with the injected repository. The constructor should either have no comment or a meaningful one describing its purpose.
📝 Proposed fix
public function __construct( private AlbumRepository $album_repository, ) { - // Prevent instantiation }app/Http/Requests/Album/GetAlbumChildrenRequest.php (1)
68-68: Optional: Remove redundant type coercion.The
intval()call is redundant since the validation rule'integer'already ensures the value is an integer type, and the?? 1fallback provides an integer default.♻️ Simplify type handling
- $this->page = intval($values[RequestAttribute::PAGE_ATTRIBUTE] ?? 1); + $this->page = $values[RequestAttribute::PAGE_ATTRIBUTE] ?? 1;app/Http/Controllers/Gallery/AlbumHeadController.php (2)
48-48: Optional: Remove unnecessary null initialization.The
$album_resourcevariable is initialized tonullbut immediately overwritten by the match expression on line 50. The initialization can be removed since the match expression is exhaustive and always assigns a value.♻️ Simplify variable initialization
$config = new AlbumConfig($request->album()); - $album_resource = null; - - $album_resource = match (true) { + + $album_resource = match (true) { $request->album() instanceof BaseSmartAlbum => new HeadSmartAlbumResource($request->album()), $request->album() instanceof TagAlbum => new HeadTagAlbumResource($request->album()), $request->album() instanceof Album => new HeadAlbumResource($request->album()),
59-59: Optional: Cache album reference to avoid repeated method calls.The
$request->album()method is called three times on line 59. While this is a minor performance consideration, caching the reference improves readability.♻️ Cache album reference
$config = new AlbumConfig($request->album()); + $album = $request->album(); - $album_resource = match (true) { - $request->album() instanceof BaseSmartAlbum => new HeadSmartAlbumResource($request->album()), - $request->album() instanceof TagAlbum => new HeadTagAlbumResource($request->album()), - $request->album() instanceof Album => new HeadAlbumResource($request->album()), + $album_resource = match (true) { + $album instanceof BaseSmartAlbum => new HeadSmartAlbumResource($album), + $album instanceof TagAlbum => new HeadTagAlbumResource($album), + $album instanceof Album => new HeadAlbumResource($album), // @codeCoverageIgnoreStart default => throw new LycheeLogicException('This should not happen'), // @codeCoverageIgnoreEnd }; - AlbumVisit::dispatchIf((MetricsController::shouldMeasure() && $request->album() instanceof BaseAlbum), $this->visitorId(), $request->album()->get_id()); + AlbumVisit::dispatchIf((MetricsController::shouldMeasure() && $album instanceof BaseAlbum), $this->visitorId(), $album->get_id()); return new HeadAbstractAlbumResource($config, $album_resource);database/migrations/2026_01_07_add_pagination_config_keys.php (1)
20-32: Consider usingPOSITIVEtype_range for per-page values.Both
albums_per_pageandphotos_per_pageuseself::INTwhich allows zero or negative values. A value of 0 or negative would likely cause issues with pagination. Consider usingself::POSITIVEto enforce valid positive integers.Proposed fix
[ 'key' => 'albums_per_page', 'value' => '30', 'cat' => self::MOD_GALLERY, - 'type_range' => self::INT, + 'type_range' => self::POSITIVE, 'description' => 'Number of sub-albums per page.',[ 'key' => 'photos_per_page', 'value' => '100', 'cat' => self::MOD_GALLERY, - 'type_range' => self::INT, + 'type_range' => self::POSITIVE, 'description' => 'Number of photos per page.',Also applies to: 59-71
resources/js/components/pagination/Pagination.vue (1)
56-56:totalPagescomputed property is redundant.The
totalPagescomputed is just an alias forprops.lastPage. Consider usingprops.lastPagedirectly in the template condition (Line 22) to reduce indirection.Proposed simplification
-<div v-else-if="mode === 'page_navigation' && totalPages > 1" class="flex justify-center w-full py-4"> +<div v-else-if="mode === 'page_navigation' && lastPage > 1" class="flex justify-center w-full py-4">And remove:
-const totalPages = computed(() => props.lastPage);resources/js/components/gallery/albumModule/AlbumPanel.vue (1)
221-229:goTo*Page()should ideally await loads (or return the Promise) for smoother UI coordination.
IfPagination.vuewants to show loading state based on the handler’s lifetime, returning/awaiting the Promise makes it easier to chain UI behavior (e.g., scroll-to-top after data arrives).Possible tweak
-function goToPhotosPage(page: number) { +function goToPhotosPage(page: number): Promise<void> { photosStore.reset(); - albumStore.loadPhotos(page, false); + return albumStore.loadPhotos(page, false); } -function goToAlbumsPage(page: number) { +function goToAlbumsPage(page: number): Promise<void> { albumsStore.reset(); - albumStore.loadAlbums(page, false); + return albumStore.loadAlbums(page, false); }resources/js/composables/pagination/usePagination.ts (1)
26-88: Solid, minimal pagination composable; consider the “unknown lastPage” UX edge.
IflastPage=0means “unknown yet”,goToPage(1)currently no-ops; that’s fine if initial load is always handled elsewhere, but worth confirming.resources/js/services/album-service.ts (1)
67-76: Fix typo and consider centralizing hard-coded page limits.The comment on line 64 has a typo: "we now" should be "we know". Additionally, hard-coded page ceilings (legacy: 10, new: 50) risk leaving cached entries stale if users browse beyond those limits. The axios-cache-interceptor library doesn't provide a built-in prefix-based eviction API, so consider extracting page limits to named constants (e.g.,
MAX_LEGACY_PAGES = 10,MAX_PAGINATED_PAGES = 50) to make future updates easier, or implement a custom storage adapter if broader prefix-based eviction becomes necessary.tests/Feature_v2/Album/AlbumPhotosEndpointTest.php (1)
26-127: Tests are solid, but hard-coding defaultper_pageand exact validation messages is brittle
per_page => 100and'The album id field is required.'will break if defaults/localization change. If there’s an existing helper inBaseApiWithDataTestfor pagination assertions (or a seeded config accessor), prefer that over hard-coded values/messages.tests/Feature_v2/Album/AlbumChildrenEndpointTest.php (1)
26-108: Same brittleness here: avoid hard-codingper_page => 30and exact validation messageConsider asserting the pagination shape (keys present, ints, totals) or deriving the expected defaults from the seeded config/test fixtures rather than embedding literals.
resources/js/components/pagination/PaginationInfiniteScroll.vue (1)
95-104: Observer isn’t recreated ifthreshold,resourceType, orrootMarginchangeIf any of those props can change during the component lifetime, you’ll keep an observer with stale options. Consider watching them and recreating the observer (disconnect + setup).
app/Http/Resources/Models/HeadAlbumResource.php (1)
52-53: Uninitialized nullable property$editablemay cause issues.The
$editableproperty is declared as nullable but is only assigned when$this->rights->can_editis true. Without explicit initialization tonull, Spatie Data may encounter issues when serializing.♻️ Suggested fix
public ?EditableBaseAlbumResource $editable; + public ?EditableBaseAlbumResource $editable = null;app/Http/Resources/Models/HeadTagAlbumResource.php (1)
47-47: Uninitialized nullable property$editable.Same as
HeadAlbumResource- the nullable$editableproperty should be explicitly initialized tonull.♻️ Suggested fix
- public ?EditableBaseAlbumResource $editable; + public ?EditableBaseAlbumResource $editable = null;resources/js/stores/AlbumState.ts (1)
152-152: Debug logging statements present.Lines 152 and 187 contain
console.debugstatements. These are useful for development but consider removing them before final release or gating behind a debug flag.Also applies to: 187-187
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
database/migrations/2025_12_22_163233_create_xhprof_table.php (1)
28-28: Column name with space violates Laravel and PSR-4 conventions.The column name
'server name'(line 28) contains a space, which violates Laravel's snake_case convention for database columns. While Laravel's Schema builder will quote this identifier automatically, this creates technical debt:
- Raw SQL queries require manual backtick quoting
- External database tools may have issues
- Inconsistent with other columns in the schema
- Violates PSR-4 coding standards specified in the guidelines
As per coding guidelines, variable and column names should use snake_case.
🔧 Rename column to use snake_case
- $table->char('server name', 64)->nullable(); + $table->char('server_name', 64)->nullable();And update the composite index:
- $table->index(['server name', 'timestamp']); + $table->index(['server_name', 'timestamp']);Also applies to: 46-46
resources/js/stores/AlbumState.ts (1)
283-364: Bug:load()setsconfigafter loading photos for tag/smart albums.
That meansloadPhotos()seesthis.configasundefinedand may disable timeline merging incorrectly. Setthis.configbefore callingloadPhotos()(and consider reusingloadHead()to reduce duplication).Proposed fix
if (data.data.config.is_model_album) { this.config = data.data.config; await Promise.all([this.loadAlbums(1, false), this.loadPhotos(1, false)]); this.modelAlbum = data.data.resource as App.Http.Resources.Models.HeadAlbumResource; } else { // Tag/Smart albums: Only load photos (they don't have child albums) - await this.loadPhotos(1, false); this.config = data.data.config; + await this.loadPhotos(1, false); if (data.data.config.is_base_album) { // Tag album: Photos filtered by tag this.tagAlbum = data.data.resource as App.Http.Resources.Models.HeadTagAlbumResource; } else { // Smart album: Recent, Starred, On This Day, Unsorted, Untagged this.smartAlbum = data.data.resource as App.Http.Resources.Models.HeadSmartAlbumResource; } }
🧹 Nitpick comments (16)
app/Relations/HasAlbumThumb.php (1)
226-229: Consider caching the sorting criterion if performance becomes a concern.The change from using a per-instance
$this->sortingproperty to callingPhotoSortingCriterion::createDefault()on eachgetResults()invocation ensures that sorting always reflects the current configuration. However, this means resolving theConfigManagerfrom the service container and creating a newPhotoSortingCriterioninstance on every call.While the
ConfigManagerlikely caches configuration values and this method may not be called frequently, you could optimize by caching the result if profiling indicates this is a hot path.♻️ Optional optimization: cache the default sorting criterion
If performance profiling indicates this is beneficial, you could cache the default sorting criterion as a static property or memoize it within the method:
public function getResults(): ?Thumb { // ... existing code ... static $defaultSorting = null; if ($defaultSorting === null) { $defaultSorting = PhotoSortingCriterion::createDefault(); } return Thumb::createFromQueryable( $this->getRelationQuery(), $defaultSorting ); }Note: Only apply this optimization if the current approach proves to be a performance bottleneck, as the change appears intentional to ensure consistency with the pagination and sorting refactoring in this PR.
app/Http/Middleware/ResolveConfigs.php (1)
33-34: Consider lazy instantiation to avoid unnecessary DB queries.The Watermarker is instantiated eagerly on every request, which may trigger a database query in its constructor (via
check_watermark_image()) if watermarking is enabled—even when the Watermarker is never used in that request. This contradicts the PR's goal of reducing queries.⚡ Proposed fix for lazy instantiation
- $watermarker = new Watermarker(); - app()->scoped(Watermarker::class, fn () => $watermarker); + app()->scoped(Watermarker::class, fn () => new Watermarker());This ensures the Watermarker is only instantiated when first resolved from the container, deferring the database query until it's actually needed.
app/Image/Watermarker.php (2)
186-192: Consider usingcan_watermark()for consistency.The
do()method callsis_watermark_enabled()andcheck_watermark_image()separately, but the newly introduced public methodcan_watermark()combines both checks. Usingcan_watermark()would make the code more consistent and reduce duplication.♻️ Optional refactor
- if (!$this->is_watermark_enabled()) { - return; - } - - if (!$this->check_watermark_image()) { + if (!$this->can_watermark()) { return; }
194-194: Inject ConfigManager via constructor for consistency.The
do()method resolvesConfigManagerdirectly usingresolve(), which is inconsistent with the dependency injection pattern used elsewhere in the codebase (e.g.,ApplyWatermarkconstructor injects bothConfigManagerandWatermarker). Injecting it via the constructor would improve testability and consistency. As per coding guidelines, prefer constructor injection over service location.♻️ Proposed refactor
Add
ConfigManagerto the constructor:+ public function __construct( + private readonly ConfigManager $config_manager, + ) { - public function __construct() - { $this->naming_strategy = new WatermarkGroupedWithRandomSuffixNamingStrategy(); if (!$this->is_watermark_enabled()) { return; }Update method calls to use the injected instance:
- $config_manager = resolve(ConfigManager::class); - $is_enabled = $config_manager->getValueAsBool('watermark_enabled'); + $is_enabled = $this->config_manager->getValueAsBool('watermark_enabled');Apply similar changes to all
resolve(ConfigManager::class)calls in the class (lines 52, 136, 162, 194).database/migrations/2025_12_22_163233_create_xhprof_table.php (1)
17-19: Consider consolidating redundant driver checks.The MySQL driver check at lines 17-19 causes an early return for non-MySQL databases, making the second check at lines 49-53 redundant. While this defensive coding may prevent issues if the early return is removed, it's currently unreachable for non-MySQL drivers.
♻️ Consolidate driver checks
public function up() { if (DB::connection()->getDriverName() !== 'mysql') { return; } Schema::dropIfExists('details'); Schema::create('details', function (Blueprint $table) { $table->id('idcount'); $table->char('id', 64); $table->char('url', 255)->nullable(); $table->char('c_url', 255)->nullable(); $table->timestamp('timestamp')->useCurrent()->useCurrentOnUpdate(); $table->char('server name', 64)->nullable(); $table->binary('perfdata')->nullable(); $table->tinyInteger('type')->nullable(); $table->binary('cookie')->nullable(); $table->binary('post')->nullable(); $table->binary('get')->nullable(); $table->integer('pmu')->nullable(); $table->integer('wt')->nullable(); $table->integer('cpu')->nullable(); $table->char('server_id', 64)->nullable(); $table->char('aggregateCalls_include', 255)->nullable(); $table->index('url'); $table->index('c_url'); $table->index('cpu'); $table->index('wt'); $table->index('pmu'); $table->index('timestamp'); $table->index(['server name', 'timestamp']); }); - if (DB::connection()->getDriverName() === 'mysql') { - DB::statement('ALTER TABLE details MODIFY COLUMN `perfdata` LONGBLOB'); - DB::statement('ALTER TABLE details MODIFY COLUMN `cookie` LONGBLOB'); - DB::statement('ALTER TABLE details MODIFY COLUMN `post` LONGBLOB'); - } + DB::statement('ALTER TABLE details MODIFY COLUMN `perfdata` LONGBLOB'); + DB::statement('ALTER TABLE details MODIFY COLUMN `cookie` LONGBLOB'); + DB::statement('ALTER TABLE details MODIFY COLUMN `post` LONGBLOB'); }Also applies to: 49-53
app/Enum/ColumnSortingType.php (1)
23-26: Clarify semantics of “STRICT” vs non-strict (DB vs natural sorting) to avoid user surprise.Enums are fine, but the comment alone may not be enough for consumers/UI text: ensure it’s clear that
TITLE/DESCRIPTIONmay still use non-DB “natural” sorting (potentially per-page with pagination), while*_STRICTis DB ordering (globally consistent across pages). Based on learnings, this trade-off is acceptable, but it should be documented/config-disclaimed.docs/specs/4-architecture/features/007-pagination/tasks.md (1)
245-259: Duplicate task IDs:T-007-35andT-007-35a..fcollide withT-007-35 – Run full frontend quality gate.Consider renumbering to keep IDs unique and stable for cross-references.
app/Http/Requests/Album/GetAlbumPhotosRequest.php (2)
71-78: Consider excluding 'access_permissions' relation.The code excludes several heavy relations like 'owner' and 'statistics', but doesn't exclude 'access_permissions' which could involve additional joins. Consider whether this relation is needed for the photos endpoint.
39-42: Add PasswordRequiredException handling to match similar request classes.
GetAlbumHeadRequestandGetAlbumRequestboth throwPasswordRequiredExceptionin theirauthorize()methods to display the password dialog when accessing password-protected albums.GetAlbumPhotosRequestshould include the same pattern for consistent UX behavior:public function authorize(): bool { $result = Gate::check(AlbumPolicy::CAN_ACCESS, [AbstractAlbum::class, $this->album]); // In case of a password protected album, we must throw an exception // with a special error message ("Password required") such that the // front-end shows the password dialog if a password is set, but // does not show the dialog otherwise. if ( !$result && $this->album instanceof BaseAlbum && $this->album->public_permissions()?->password !== null ) { throw new PasswordRequiredException(); } return $result; }tests/Unit/Repositories/AlbumRepositoryTest.php (3)
81-98: Pagination coverage is fine, but consider asserting item identity too.
Right now it only asserts counts; optionally assert specific IDs/titles to ensure page boundaries are correct.
133-157: Sorting assertions are OK, but keep in mind title/description sorting may be per-page.
Since you keep results within one page, this won’t conflict with the known “PHP-level natural sort after pagination” trade-off. Based on learnings, consider adding a short comment here to prevent future expansion of this test into multi-page title-sorting expectations.
159-197: Visibility tests are valuable; consider reducing unrelated setup.
$otherAlbumis created but never used—optional to remove to keep the test focused.tests/Feature_v2/PaginationIntegrationTest.php (1)
211-232: Default-page behavior test is fine; consider usingassertSamefor strictness.
Mostly a style/readability improvement for metadata fields (current_pageetc.).resources/js/stores/AlbumState.ts (3)
10-69: State additions are coherent;reset()doesn’t reset legacy paging fields.
If anything still readscurrent_page/last_page/per_page/total, consider resetting them too (or remove them if fully migrated).
82-143:loadHead()looks OK, but error handling is duplicated and can be simplified.
The401/403 + "Password required"branches are identical; consider collapsing with a single condition + optional chaining.
155-191:loadAlbums()append mode risks duplicates if pages overlap/reorder.
If backend sorting can change between calls (e.g., concurrent updates), consider de-duping byidwhen appending.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
app/Actions/Photo/Pipes/Standalone/ApplyWatermark.phpapp/Enum/ColumnSortingAlbumType.phpapp/Enum/ColumnSortingPhotoType.phpapp/Enum/ColumnSortingType.phpapp/Http/Controllers/Gallery/AlbumChildrenController.phpapp/Http/Controllers/Gallery/AlbumHeadController.phpapp/Http/Controllers/Gallery/AlbumPhotosController.phpapp/Http/Middleware/ResolveConfigs.phpapp/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Http/Resources/Models/HeadTagAlbumResource.phpapp/Http/Resources/Rights/ModulesRightsResource.phpapp/Image/Watermarker.phpapp/Models/Extensions/SortingDecorator.phpapp/Relations/HasAlbumThumb.phpapp/Repositories/AlbumRepository.phpconfig/debugbar.phpdatabase/migrations/2025_08_27_203030_create_orders_table.phpdatabase/migrations/2025_12_22_163233_create_xhprof_table.phpdatabase/migrations/2025_12_27_034137_create_photo_ratings_table.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpdatabase/migrations/2026_01_07_add_pagination_config_keys.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.phpdocs/specs/2-how-to/configure-pagination.mddocs/specs/3-reference/api-design.mddocs/specs/4-architecture/features/007-pagination/tasks.mddocs/specs/4-architecture/knowledge-map.mdresources/js/components/pagination/PaginationInfiniteScroll.vueresources/js/composables/pagination/usePagination.tsresources/js/config/constants.tsresources/js/lychee.d.tsresources/js/stores/AlbumState.tsresources/js/stores/PhotosState.tstests/Feature_v2/PaginationIntegrationTest.phptests/Unit/Repositories/AlbumRepositoryTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
- app/Http/Controllers/Gallery/AlbumHeadController.php
- resources/js/composables/pagination/usePagination.ts
- resources/js/components/pagination/PaginationInfiniteScroll.vue
- app/Repositories/AlbumRepository.php
- app/Http/Controllers/Gallery/AlbumChildrenController.php
🧰 Additional context used
📓 Path-based instructions (13)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
app/Actions/Photo/Pipes/Standalone/ApplyWatermark.phpdatabase/migrations/2025_12_22_163233_create_xhprof_table.phpdatabase/migrations/2025_08_27_203030_create_orders_table.phpapp/Enum/ColumnSortingType.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpapp/Relations/HasAlbumThumb.phpapp/Enum/ColumnSortingAlbumType.phpapp/Enum/ColumnSortingPhotoType.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.phptests/Feature_v2/PaginationIntegrationTest.phpdatabase/migrations/2025_12_27_034137_create_photo_ratings_table.phpconfig/debugbar.phptests/Unit/Repositories/AlbumRepositoryTest.phpapp/Http/Resources/Rights/ModulesRightsResource.phpapp/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Models/Extensions/SortingDecorator.phpdatabase/migrations/2026_01_07_add_pagination_config_keys.phpapp/Http/Resources/Models/HeadTagAlbumResource.phpapp/Http/Controllers/Gallery/AlbumPhotosController.phpapp/Image/Watermarker.phpapp/Http/Middleware/ResolveConfigs.php
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.md: Use Markdown format for documentation
At the bottom of documentation files, add an hr line followed by "Last updated: [date of the update]"
Files:
docs/specs/2-how-to/configure-pagination.mddocs/specs/4-architecture/features/007-pagination/tasks.mddocs/specs/4-architecture/knowledge-map.mddocs/specs/3-reference/api-design.md
docs/specs/4-architecture/features/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/*.md: Author new specifications, feature plans, and task checklists using docs/specs/templates/feature-spec-template.md, docs/specs/templates/feature-plan-template.md, and docs/specs/templates/feature-tasks-template.md to keep structure, metadata, and verification notes uniform across features.
For any new UI feature or modification, include an ASCII mock-up in the specification (see docs/specs/4-architecture/spec-guidelines/ui-ascii-mockups.md).
When revising a specification, only document fallback or compatibility behaviour if the user explicitly asked for it; if instructions are unclear, pause and request confirmation instead of assuming a fallback.
For every task, refresh the relevant feature plan and note open questions; only move forward once the plan reflects the desired change. Update specs before code.
Update feature specs, feature plans, and tasks documents as progress is made and sync context to disk.
Capture prompt summaries, command sequences, and rationale in the active feature plan or an appendix referenced from it so downstream reviewers know how the change was produced (intent logging).
Track upcoming additions for contract tests, mutation analysis, and security/red-team prompt suites in the plans until automated jobs exist (quality gates).
Publish prompt and tool usage notes alongside the feature plan update so future agents understand how the iteration unfolded.
Files:
docs/specs/4-architecture/features/007-pagination/tasks.md
docs/specs/4-architecture/features/**/{spec,plan,tasks}.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/{spec,plan,tasks}.md: Start every feature by updating or creating its specification at docs/specs/4-architecture/features/-/spec.md, followed by plan and tasks documents after clarifications are resolved.
Generate or refresh the feature plan only after the specification is current and high-/medium-impact clarifications are resolved and recorded in the spec (plus ADRs where required).
Files:
docs/specs/4-architecture/features/007-pagination/tasks.md
docs/specs/4-architecture/features/**/tasks.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/tasks.md: Maintain a per-feature tasks checklist that mirrors the plan, orders tests before code, and keeps planned increments ≤90 minutes by preferring finer-grained entries and documenting sub-steps when something nears the limit.
Mark each task [x] in the feature's tasks.md as soon as it passes verification. Do not batch task completions—update the checklist after every individual task so progress is always visible.
Before committing, confirm every completed task in tasks.md is marked [x] and the roadmap status reflects current progress (validate task checklist).
Files:
docs/specs/4-architecture/features/007-pagination/tasks.md
tests/Feature_v2/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Feature_v2 directory should extend from BaseApiWithDataTest
Files:
tests/Feature_v2/PaginationIntegrationTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Feature_v2/PaginationIntegrationTest.phptests/Unit/Repositories/AlbumRepositoryTest.php
docs/specs/4-architecture/knowledge-map.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/knowledge-map.md: Skim docs/specs/4-architecture/knowledge-map.md and the up-to-date module snapshot in docs/specs/architecture-graph.json before planning so new work reinforces the architectural relationships already captured there.
Maintain the knowledge map by adding, adjusting, or pruning entries whenever new modules, dependencies, or contracts appear.
Files:
docs/specs/4-architecture/knowledge-map.md
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Unit directory should extend from AbstractTestCase
Files:
tests/Unit/Repositories/AlbumRepositoryTest.php
**/*.{vue,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{vue,ts,js}: For Vue3/TypeScript frontend code, follow coding conventions defined in docs/specs/3-reference/coding-conventions.md for style, naming, and testing practices.
For frontend changes, run npm run check to ensure all frontend tests pass before committing.
Files:
resources/js/config/constants.tsresources/js/stores/AlbumState.tsresources/js/lychee.d.tsresources/js/stores/PhotosState.ts
**/*.{vue,ts,js,css}
📄 CodeRabbit inference engine (AGENTS.md)
For frontend changes, run npm run format to apply frontend code formatting with Prettier before committing.
Files:
resources/js/config/constants.tsresources/js/stores/AlbumState.tsresources/js/lychee.d.tsresources/js/stores/PhotosState.ts
**/*Resource.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Resource classes should extend from Spatie Data instead of JsonResource
Files:
app/Http/Resources/Rights/ModulesRightsResource.phpapp/Http/Resources/Models/HeadTagAlbumResource.php
**/*Request.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*Request.php: In Request classes, if a user is provided by the query, place it in $this->user2
In Request classes, reserve $this->user for the user making the request
Files:
app/Http/Requests/Album/GetAlbumPhotosRequest.php
🧠 Learnings (32)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3718
File: app/Models/Extensions/SortingDecorator.php:191-0
Timestamp: 2025-10-06T21:24:16.072Z
Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/PurchasableService.php:150-168
Timestamp: 2025-09-18T07:09:17.176Z
Learning: In PurchasablePhotoRequest, the validation for photo-album membership is implemented at the request validation level rather than in the service layer, using a count comparison to ensure all photos belong to the target album before creating purchasables.
📚 Learning: 2025-08-22T18:56:08.725Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3642
File: resources/js/components/gallery/albumModule/AlbumHero.vue:173-183
Timestamp: 2025-08-22T18:56:08.725Z
Learning: In the needSizeVariantsWatermark function in resources/js/components/gallery/albumModule/AlbumHero.vue, the original variant is intentionally excluded from the watermark check. This is a deliberate design decision to not consider original files when determining if the watermark button should be shown.
Applied to files:
app/Actions/Photo/Pipes/Standalone/ApplyWatermark.phpapp/Image/Watermarker.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
app/Actions/Photo/Pipes/Standalone/ApplyWatermark.phpdatabase/migrations/2025_12_22_163233_create_xhprof_table.phpdatabase/migrations/2025_08_27_203030_create_orders_table.phpapp/Enum/ColumnSortingType.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpapp/Relations/HasAlbumThumb.phpapp/Enum/ColumnSortingAlbumType.phpapp/Enum/ColumnSortingPhotoType.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.phptests/Feature_v2/PaginationIntegrationTest.phpdatabase/migrations/2025_12_27_034137_create_photo_ratings_table.phpconfig/debugbar.phptests/Unit/Repositories/AlbumRepositoryTest.phpapp/Http/Resources/Rights/ModulesRightsResource.phpapp/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Models/Extensions/SortingDecorator.phpdatabase/migrations/2026_01_07_add_pagination_config_keys.phpapp/Http/Resources/Models/HeadTagAlbumResource.phpapp/Http/Controllers/Gallery/AlbumPhotosController.phpapp/Image/Watermarker.phpapp/Http/Middleware/ResolveConfigs.php
📚 Learning: 2025-08-20T20:32:03.676Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3637
File: database/migrations/2025_08_19_160144_create_renamer_rules_table.php:106-108
Timestamp: 2025-08-20T20:32:03.676Z
Learning: In the Lychee codebase, ildyria intentionally uses destructive migration patterns where up() calls down() to ensure clean state, as indicated by "Working as intended" and "No mercy" comments in migrations like database/migrations/2025_08_19_160144_create_renamer_rules_table.php.
Applied to files:
database/migrations/2025_12_22_163233_create_xhprof_table.phpdatabase/migrations/2025_08_27_203030_create_orders_table.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.phpdatabase/migrations/2025_12_27_034137_create_photo_ratings_table.php
📚 Learning: 2025-10-06T21:24:16.072Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3718
File: app/Models/Extensions/SortingDecorator.php:191-0
Timestamp: 2025-10-06T21:24:16.072Z
Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.
Applied to files:
docs/specs/2-how-to/configure-pagination.mdresources/js/config/constants.tsapp/Models/Extensions/SortingDecorator.phpdocs/specs/3-reference/api-design.md
📚 Learning: 2025-08-20T20:32:36.930Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3637
File: database/migrations/2025_08_19_160144_create_renamer_rules_table.php:133-134
Timestamp: 2025-08-20T20:32:36.930Z
Learning: In the Lychee codebase, ildyria intentionally designs migrations to be destructive where up() calls down() first to ensure a clean state before inserting data. This approach makes migrations idempotent through destruction and recreation rather than through upsert operations, as confirmed by the "Working as intended" response to suggestions about using insertOrIgnore for config seeding.
Applied to files:
database/migrations/2025_08_27_203030_create_orders_table.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.php
📚 Learning: 2025-08-18T10:19:04.946Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3626
File: database/migrations/2025_06_07_144157_photo_tags_to_table.php:87-105
Timestamp: 2025-08-18T10:19:04.946Z
Learning: In the Lychee photo management system, the migration `2025_06_07_144157_photo_tags_to_table.php` runs on data that only contains comma-separated tag names in tag_albums.show_tags - OR/AND expressions do not exist at this migration time, so handling them is unnecessary.
Applied to files:
database/migrations/2026_01_02_203124_create_album_size_statistics_table.phpdatabase/migrations/2026_01_10_212900_add_strict_ordering.phpdatabase/migrations/2025_12_27_034137_create_photo_ratings_table.phpdatabase/migrations/2026_01_07_add_pagination_config_keys.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Applied to files:
app/Relations/HasAlbumThumb.phpapp/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Http/Controllers/Gallery/AlbumPhotosController.php
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Maintain a per-feature tasks checklist that mirrors the plan, orders tests before code, and keeps planned increments ≤90 minutes by preferring finer-grained entries and documenting sub-steps when something nears the limit.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/*.md : For every task, refresh the relevant feature plan and note open questions; only move forward once the plan reflects the desired change. Update specs before code.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Mark each task [x] in the feature's tasks.md as soon as it passes verification. Do not batch task completions—update the checklist after every individual task so progress is always visible.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/*.md : Update feature specs, feature plans, and tasks documents as progress is made and sync context to disk.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Before committing, confirm every completed task in tasks.md is marked [x] and the roadmap status reflects current progress (validate task checklist).
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/roadmap.md : Keep high-level plans in docs/specs/4-architecture/roadmap.md, store each feature's spec/plan/tasks inside docs/specs/4-architecture/features/<NNN>-<feature-name>/, and remove plans once work is complete.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/*.md : Track upcoming additions for contract tests, mutation analysis, and security/red-team prompt suites in the plans until automated jobs exist (quality gates).
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/{spec,plan,tasks}.md : Start every feature by updating or creating its specification at docs/specs/4-architecture/features/<NNN>-<feature-name>/spec.md, followed by plan and tasks documents after clarifications are resolved.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/*.md : Publish prompt and tool usage notes alongside the feature plan update so future agents understand how the iteration unfolded.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/*.md : Author new specifications, feature plans, and task checklists using docs/specs/templates/feature-spec-template.md, docs/specs/templates/feature-plan-template.md, and docs/specs/templates/feature-tasks-template.md to keep structure, metadata, and verification notes uniform across features.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/open-questions.md : Update or close entries in docs/specs/4-architecture/open-questions.md after completing work.
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/features/**/{spec,plan,tasks}.md : Generate or refresh the feature plan only after the specification is current and high-/medium-impact clarifications are resolved and recorded in the spec (plus ADRs where required).
Applied to files:
docs/specs/4-architecture/features/007-pagination/tasks.md
📚 Learning: 2025-12-22T14:11:17.217Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T14:11:17.217Z
Learning: Applies to tests/Feature_v2/**/*.php : Tests in the tests/Feature_v2 directory should extend from BaseApiWithDataTest
Applied to files:
tests/Feature_v2/PaginationIntegrationTest.php
📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR
Repo: LycheeOrg/Lychee PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T14:12:18.082Z
Learning: Applies to docs/specs/4-architecture/knowledge-map.md : Maintain the knowledge map by adding, adjusting, or pruning entries whenever new modules, dependencies, or contracts appear.
Applied to files:
docs/specs/4-architecture/knowledge-map.md
📚 Learning: 2025-09-18T07:09:17.176Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/PurchasableService.php:150-168
Timestamp: 2025-09-18T07:09:17.176Z
Learning: In PurchasablePhotoRequest, the validation for photo-album membership is implemented at the request validation level rather than in the service layer, using a count comparison to ensure all photos belong to the target album before creating purchasables.
Applied to files:
app/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Http/Controllers/Gallery/AlbumPhotosController.php
📚 Learning: 2025-08-12T11:31:09.853Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3605
File: docs/Lifecycle-of-a-request-photo-upload.md:144-151
Timestamp: 2025-08-12T11:31:09.853Z
Learning: In Lychee, the BaseApiRequest class has been modified to change Laravel's default execution order - processValidatedValues() runs BEFORE authorize(), not after. This means that in Lychee request classes, $this->album and other processed properties are available during authorization checks, unlike standard Laravel behavior.
Applied to files:
app/Http/Requests/Album/GetAlbumPhotosRequest.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Applied to files:
app/Http/Requests/Album/GetAlbumPhotosRequest.phpapp/Http/Controllers/Gallery/AlbumPhotosController.php
📚 Learning: 2025-09-22T12:35:38.842Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3706
File: tests/Unit/Http/Controllers/ImportFromServerControllerTest.php:59-66
Timestamp: 2025-09-22T12:35:38.842Z
Learning: In the Lychee codebase, the Configs class extends Laravel's Model, making config values database-backed. Tests using the DatabaseTransactions trait automatically have config changes rolled back without requiring manual restoration in tearDown() methods.
Applied to files:
database/migrations/2026_01_07_add_pagination_config_keys.php
📚 Learning: 2025-09-28T12:04:53.277Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/stores/TagState.ts:34-36
Timestamp: 2025-09-28T12:04:53.277Z
Learning: In the Lychee codebase, different API services have different response structures. TagsService.get() returns TagWithPhotosResource directly with fields id, name, and photos (no "resource" wrapper field), while AlbumService.get() returns a response with data.resource that can be null. Always check the actual TypeScript definitions before assuming response structure.
Applied to files:
app/Http/Resources/Models/HeadTagAlbumResource.phpresources/js/stores/AlbumState.tsresources/js/lychee.d.ts
📚 Learning: 2025-09-28T12:43:29.852Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/views/gallery-panels/Album.vue:200-204
Timestamp: 2025-09-28T12:43:29.852Z
Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.
Applied to files:
app/Http/Resources/Models/HeadTagAlbumResource.phpresources/js/stores/AlbumState.tsresources/js/stores/PhotosState.ts
📚 Learning: 2025-08-14T10:17:35.082Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3616
File: app/Actions/Album/PositionData.php:38-39
Timestamp: 2025-08-14T10:17:35.082Z
Learning: PositionDataResource uses toPhotoResources() method to convert photos to PhotoResource instances, and PhotoResource accesses the tags relationship ($photo->tags->pluck('name')->all()), making the 'tags' eager-loading in PositionData necessary to avoid N+1 queries.
Applied to files:
app/Http/Resources/Models/HeadTagAlbumResource.phpapp/Http/Controllers/Gallery/AlbumPhotosController.phpresources/js/lychee.d.ts
📚 Learning: 2025-09-28T08:27:38.332Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/composables/album/photoActions.ts:39-43
Timestamp: 2025-09-28T08:27:38.332Z
Learning: The user ildyria (repository maintainer) prefers to skip error handling for the rotate photo functions (rotatePhotoCW and rotatePhotoCCW) in resources/js/composables/album/photoActions.ts.
Applied to files:
app/Http/Resources/Models/HeadTagAlbumResource.php
📚 Learning: 2025-09-13T14:01:20.039Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:0-0
Timestamp: 2025-09-13T14:01:20.039Z
Learning: In PurchasableService::getEffectivePurchasableForPhoto(), both the photo-specific and album-level SQL queries include ->where('is_active', true), so the returned purchasable is guaranteed to be active and doesn't need additional is_active checks in calling code.
Applied to files:
app/Http/Controllers/Gallery/AlbumPhotosController.php
📚 Learning: 2025-09-28T08:46:37.299Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/stores/UserState.ts:7-11
Timestamp: 2025-09-28T08:46:37.299Z
Learning: In Lychee codebase, the type `App.Enum.OauthProvidersType` is properly defined within the `App.Enum` namespace in `resources/js/lychee.d.ts` file. References to `App.Enum.OauthProvidersType` in TypeScript files are valid and resolve correctly.
Applied to files:
resources/js/lychee.d.ts
🧬 Code graph analysis (10)
app/Actions/Photo/Pipes/Standalone/ApplyWatermark.php (1)
app/Image/Watermarker.php (1)
can_watermark(108-111)
app/Relations/HasAlbumThumb.php (3)
resources/js/lychee.d.ts (1)
PhotoSortingCriterion(6-9)app/DTO/AlbumSortingCriterion.php (1)
createDefault(23-35)app/DTO/PhotoSortingCriterion.php (1)
createDefault(23-35)
tests/Feature_v2/PaginationIntegrationTest.php (2)
app/Models/Configs.php (1)
Configs(61-234)tests/Feature_v2/Base/BaseApiWithDataTest.php (1)
BaseApiWithDataTest(93-266)
tests/Unit/Repositories/AlbumRepositoryTest.php (3)
app/Repositories/AlbumRepository.php (2)
AlbumRepository(24-67)getChildrenPaginated(43-66)tests/Traits/RequiresEmptyUsers.php (2)
setUpRequiresEmptyUsers(27-31)tearDownRequiresEmptyUsers(33-37)database/factories/AccessPermissionFactory.php (2)
public(55-63)visible(146-153)
resources/js/config/constants.ts (1)
resources/js/lychee.d.ts (1)
PaginationMode(128-128)
app/Http/Resources/Rights/ModulesRightsResource.php (1)
app/Image/Watermarker.php (2)
Watermarker(23-245)can_watermark(108-111)
app/Http/Requests/Album/GetAlbumPhotosRequest.php (4)
app/Factories/AlbumFactory.php (2)
AlbumFactory(28-236)createSmartAlbum(224-235)app/Http/Requests/BaseApiRequest.php (1)
BaseApiRequest(30-172)app/Rules/AlbumIDRule.php (1)
AlbumIDRule(15-45)app/Http/Requests/Album/GetAlbumHeadRequest.php (3)
authorize(29-46)rules(51-56)processValidatedValues(61-65)
app/Http/Resources/Models/HeadTagAlbumResource.php (6)
resources/js/lychee.d.ts (5)
EditableBaseAlbumResource(260-278)AlbumProtectionPolicy(841-849)PreFormattedAlbumData(859-870)AlbumRightsResource(905-918)AlbumStatisticsResource(544-548)app/Models/TagAlbum.php (1)
TagAlbum(58-180)app/Http/Resources/Models/HeadSmartAlbumResource.php (3)
TypeScript(19-47)__construct(31-41)fromModel(43-46)app/Http/Resources/Models/HeadAlbumResource.php (3)
TypeScript(23-100)__construct(57-94)fromModel(96-99)app/Http/Resources/Models/Utils/AlbumProtectionPolicy.php (1)
ofBaseAlbum(51-62)app/Http/Resources/Traits/HasHeaderUrl.php (1)
getHeaderUrl(22-38)
app/Image/Watermarker.php (4)
app/Assets/WatermarkGroupedWithRandomSuffixNamingStrategy.php (1)
WatermarkGroupedWithRandomSuffixNamingStrategy(22-70)app/Actions/Photo/Pipes/Standalone/ApplyWatermark.php (1)
__construct(23-27)app/Actions/Diagnostics/Pipes/Checks/WatermarkerEnabledCheck.php (1)
__construct(23-26)app/Image/Handlers/ImageHandler.php (1)
__construct(41-48)
app/Http/Middleware/ResolveConfigs.php (1)
app/Image/Watermarker.php (1)
Watermarker(23-245)
🪛 LanguageTool
docs/specs/2-how-to/configure-pagination.md
[style] ~160-~160: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...UX 3. Monitor browser memory usage with very large collections 4. Consider `page_navigatio...
(EN_WEAK_ADJECTIVE)
docs/specs/4-architecture/features/007-pagination/tasks.md
[style] ~252-~252: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s/components/settings/ConfigGroup.vue. Added translation keys in lang/en/gallery.ph...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~436-~436: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...re: Explore cursor-based pagination for very large albums (10000+ photos)
(EN_WEAK_ADJECTIVE)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 2️⃣ PHP dist / 8.5 - sqlite
- GitHub Check: 2️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.4 - mariadb -- Install
- GitHub Check: 1️⃣ PHP tests / 8.4 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/ImageProcessing/Image/WatermarkerTest.php (2)
62-62: Use strict boolean assertions instead ofassertEquals.The method call conversion looks correct. However, per coding guidelines requiring strict comparison (===), boolean assertions should use
assertTrue/assertFalseinstead ofassertEqualswith boolean literals, asassertEqualsperforms loose comparison by default.♻️ Refactor to use strict boolean assertions
- self::assertEquals(false, $watermarker->can_watermark()); + self::assertFalse($watermarker->can_watermark());Apply this pattern to all boolean assertions at lines 62, 70, 79, 88, 118, and 184. For
trueassertions:- self::assertEquals(true, $watermarker->can_watermark()); + self::assertTrue($watermarker->can_watermark());As per coding guidelines, PHP code should use strict comparison (===) instead of loose comparison (==).
Also applies to: 70-70, 79-79, 88-88, 118-118, 184-184
113-114: Remove duplicateConfigs::setcalls.The same configuration key is set twice consecutively. The second call is redundant.
♻️ Remove duplicate configuration
At lines 113-114:
Configs::set('watermark_photo_id', $photoId); - Configs::set('watermark_photo_id', $photoId);At lines 178-179:
Configs::set('watermark_photo_id', $photoId); - Configs::set('watermark_photo_id', $photoId);Also applies to: 178-179
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/ImageProcessing/Image/WatermarkerTest.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
tests/ImageProcessing/Image/WatermarkerTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/ImageProcessing/Image/WatermarkerTest.php
🧠 Learnings (2)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3718
File: app/Models/Extensions/SortingDecorator.php:191-0
Timestamp: 2025-10-06T21:24:16.072Z
Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/PurchasableService.php:150-168
Timestamp: 2025-09-18T07:09:17.176Z
Learning: In PurchasablePhotoRequest, the validation for photo-album membership is implemented at the request validation level rather than in the service layer, using a count comparison to ensure all photos belong to the target album before creating purchasables.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
tests/ImageProcessing/Image/WatermarkerTest.php
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: 2️⃣ PHP dist / 8.5 - postgresql
- GitHub Check: 1️⃣ PHP tests / 8.4 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.4 - mariadb -- Install
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
app/Image/Watermarker.php (2)
25-43: Avoid eager DB lookup in the constructor (can add a baseline query per request).
IfWatermarkeris resolved even on requests that never watermark,__construct()callingcheck_watermark_image()can introduce unnecessary DB load. Prefer lazy initialization (keep caching incheck_watermark_image()), and let callers trigger the check viacan_watermark()/do().Proposed diff
public function __construct() { $this->naming_strategy = new WatermarkGroupedWithRandomSuffixNamingStrategy(); - - if (!$this->is_watermark_enabled()) { - return; - } - - if (!$this->check_watermark_image()) { - return; - } }
50-72: Good consolidation of enablement checks; consider caching ConfigManager to reduce repeated resolve() calls.
is_watermark_enabled()is now a single source of truth and uses strict comparisons—nice. Minor maintainability/perf:resolve(ConfigManager::class)is repeated across the class; a constructor-injected (or cached)ConfigManagerproperty would simplify and reduce container lookups.resources/js/stores/AlbumState.ts (3)
156-194: Consider adding_loadingAlbumIdcheck for consistency withloadHead().The
loadAlbums()method only guards against navigation away (this.albumId !== requestedAlbumId) but doesn't track which specific load request is in flight using_loadingAlbumId. This differs from the more robust race condition handling inloadHead()(lines 93, 108, 139).While
loadMoreAlbums()guards against concurrent calls with thealbums_loadingflag,loadAlbums()can still be called directly with different page numbers, potentially causing race conditions if pages load out of order.♻️ Consider consistent race condition handling
loadAlbums(page: number = 1, append: boolean = false): Promise<void> { const albumsStore = useAlbumsStore(); if (this.albumId === ALL || this.albumId === undefined) { return Promise.resolve(); } // Capture current album ID to detect navigation during loading const requestedAlbumId = this.albumId; + const requestToken = `${requestedAlbumId}_${page}`; this.albums_loading = true; + this._loadingPage = page; return AlbumService.getAlbums(requestedAlbumId, page) .then((data) => { // Race condition guard: Don't update state if user navigated away - if (this.albumId !== requestedAlbumId) { + if (this.albumId !== requestedAlbumId || this._loadingPage !== page) { return; }
206-244: Consider adding page-level race condition tracking.Similar to
loadAlbums(), theloadPhotos()method only guards against navigation changes but doesn't track which specific page load is in flight. If multiple page loads are triggered (e.g., user rapidly clicks pagination buttons), responses could arrive out of order and overwrite newer data with older results.Consider tracking the requested page similar to the pattern suggested for
loadAlbums()to ensure that only the most recent page load updates the state.
288-369: Consider refactoringload()to useloadHead()to reduce duplication.The
load()method duplicates significant logic fromloadHead()(lines 83-144):
- Both call
AlbumService.getHead()with the same album ID- Both handle the response data mapping (lines 312-347 vs 97-123)
- Both have identical error handling for password protection (lines 349-362 vs 124-137)
- Both manage race conditions using
_loadingAlbumId♻️ Refactor to eliminate duplication
Consider refactoring
load()to callloadHead()first, then load children/photos:async load(): Promise<void> { const togglableState = useTogglablesStateStore(); const photosState = usePhotosStore(); const albumsStore = useAlbumsStore(); if (this.albumId === ALL || this.albumId === undefined) { return Promise.resolve(); } // Do not reload fully if we are already on the right album. if (this.albumId === this.album?.id && this.isLoaded) { return Promise.resolve(); } // Exit early if we are already loading this album if (this._loadingAlbumId === this.albumId) { return Promise.resolve(); } const requestedAlbumId = this.albumId; // Clear existing data to avoid showing stale content albumsStore.reset(); photosState.reset(); // Load head metadata first await this.loadHead(); // If navigation occurred during loadHead, abort if (this.albumId !== requestedAlbumId) { return; } // Load children and/or photos based on album type if (this.config?.is_model_album) { await Promise.all([this.loadAlbums(1, false), this.loadPhotos(1, false)]); } else { await this.loadPhotos(1, false); } }This approach:
- Eliminates ~70 lines of duplicated code
- Makes the loading flow more explicit and easier to maintain
- Reuses the robust race condition handling from
loadHead()- Maintains the same functionality
resources/js/components/pagination/PaginationInfiniteScroll.vue (2)
14-20: Consider adding validation or documentation for therootMarginprop format.The
rootMarginprop accepts any string, but the parsing logic at line 158 usesparseInt(getRootMargin(), 10), which assumes a format like"{number}px". If a user provides a different format (e.g.,"50%"or"1em"), the parsing will fail silently or produce incorrect results.🛡️ Suggestions to improve robustness
Option 1: Add JSDoc or TypeScript comment documenting the expected format:
const props = defineProps<{ loading: boolean; hasMore: boolean; + /** Root margin in pixels (e.g., "300px"). Other units are not supported. */ rootMargin?: string; threshold?: number; resourceType?: "photos" | "albums"; }>();Option 2: Add runtime validation when parsing (at line 158):
function parseRootMarginPx(margin: string): number { const parsed = parseInt(margin, 10); if (isNaN(parsed)) { console.warn(`[InfiniteScroll] Invalid rootMargin format: ${margin}, falling back to 0`); return 0; } return parsed; } // Then use: const rootMarginPx = parseRootMarginPx(getRootMargin());
172-181: Optional: Extract duplicate emission pattern into a helper function.The emission logic at lines 176-180 duplicates the pattern from lines 49-55 (setting
isEmitting, emittingloadMore, and resetting viarequestAnimationFrame). Extracting this into a helper function would improve maintainability.♻️ Refactoring suggestion
+function emitLoadMore() { + isEmitting = true; + emit("loadMore"); + requestAnimationFrame(() => { + isEmitting = false; + }); +} + function handleIntersect(entries: IntersectionObserverEntry[]) { const entry = entries[0]; console.debug( `[InfiniteScroll] intersect: isIntersecting=${entry.isIntersecting}, hasMore=${props.hasMore}, loading=${props.loading}, isEmitting=${isEmitting}`, ); if (entry.isIntersecting && props.hasMore && !props.loading && !isEmitting) { - isEmitting = true; - emit("loadMore"); - requestAnimationFrame(() => { - isEmitting = false; - }); + emitLoadMore(); } }Then apply the same change at lines 176-180 in the post-load watcher.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/Image/Watermarker.phpapp/Models/Extensions/SortingDecorator.phpconfig/debugbar.phpdatabase/migrations/2026_01_07_add_pagination_config_keys.phpresources/js/components/pagination/PaginationInfiniteScroll.vueresources/js/stores/AlbumState.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- database/migrations/2026_01_07_add_pagination_config_keys.php
- config/debugbar.php
🧰 Additional context used
📓 Path-based instructions (4)
**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.vue: Use TypeScript in composition API for Vue3 and use PrimeVue for UI components
Do not use await async calls in Vue3; use .then() instead
Do not use const function = () => {} syntax; use function functionName() {} instead in Vue3
In Vue3 components, the structure should be first, then <script lang="ts">, then <style>Files:
resources/js/components/pagination/PaginationInfiniteScroll.vue**/*.{vue,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{vue,ts,js}: For Vue3/TypeScript frontend code, follow coding conventions defined in docs/specs/3-reference/coding-conventions.md for style, naming, and testing practices.
For frontend changes, run npm run check to ensure all frontend tests pass before committing.Files:
resources/js/components/pagination/PaginationInfiniteScroll.vueresources/js/stores/AlbumState.ts**/*.{vue,ts,js,css}
📄 CodeRabbit inference engine (AGENTS.md)
For frontend changes, run npm run format to apply frontend code formatting with Prettier before committing.
Files:
resources/js/components/pagination/PaginationInfiniteScroll.vueresources/js/stores/AlbumState.ts**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...Files:
app/Image/Watermarker.phpapp/Models/Extensions/SortingDecorator.php🧠 Learnings (10)
📓 Common learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3718 File: app/Models/Extensions/SortingDecorator.php:191-0 Timestamp: 2025-10-06T21:24:16.072Z Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/OrderService.php:65-73 Timestamp: 2025-09-16T21:56:01.607Z Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/OrderService.php:65-73 Timestamp: 2025-09-16T21:56:01.607Z Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/PurchasableService.php:150-168 Timestamp: 2025-09-18T07:09:17.176Z Learning: In PurchasablePhotoRequest, the validation for photo-album membership is implemented at the request validation level rather than in the service layer, using a count comparison to ensure all photos belong to the target album before creating purchasables.📚 Learning: 2025-08-16T10:57:51.480Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/views/gallery-panels/Albums.vue:11-13 Timestamp: 2025-08-16T10:57:51.480Z Learning: In resources/js/views/gallery-panels/Albums.vue, the SelectDrag component is intentionally configured with :with-scroll="false" even though the #galleryView container has overflow-y-auto. This is working as intended according to the maintainer, who suggested that overflow-y-auto might be removed instead.Applied to files:
resources/js/components/pagination/PaginationInfiniteScroll.vue📚 Learning: 2026-01-10T17:13:49.763Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3952 File: resources/js/components/forms/album/AlbumProperties.vue:20-20 Timestamp: 2026-01-10T17:13:49.763Z Learning: PrimeVue v4.0+ deprecated inputId for Select and related form components in favor of labelId. In Vue templates, use kebab-case label-id (label-id="...") to set the labelId prop. Also replace inputStyle and inputClass with labelStyle and labelClass. Apply these changes to all Vue components that interact with PrimeVue form controls, not just a single file.Applied to files:
resources/js/components/pagination/PaginationInfiniteScroll.vue📚 Learning: 2025-09-28T12:43:29.852Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/views/gallery-panels/Album.vue:200-204 Timestamp: 2025-09-28T12:43:29.852Z Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.Applied to files:
resources/js/stores/AlbumState.ts📚 Learning: 2025-09-28T12:04:53.277Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/stores/TagState.ts:34-36 Timestamp: 2025-09-28T12:04:53.277Z Learning: In the Lychee codebase, different API services have different response structures. TagsService.get() returns TagWithPhotosResource directly with fields id, name, and photos (no "resource" wrapper field), while AlbumService.get() returns a response with data.resource that can be null. Always check the actual TypeScript definitions before assuming response structure.Applied to files:
resources/js/stores/AlbumState.ts📚 Learning: 2025-09-28T08:39:34.280Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/stores/NsfwConsentedState.ts:3-5 Timestamp: 2025-09-28T08:39:34.280Z Learning: In Lychee codebase, all Pinia store files follow the pattern of declaring the type alias before the store definition (e.g., `export type StoreType = ReturnType<typeof useStore>; export const useStore = defineStore(...)`). This pattern compiles successfully with TypeScript 5.9 and vue-tsc.Applied to files:
resources/js/stores/AlbumState.ts📚 Learning: 2025-11-30T17:19:00.773Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3837 File: resources/js/composables/checkout/useStepTwo.ts:8-16 Timestamp: 2025-11-30T17:19:00.773Z Learning: In the Lychee project's checkout composables (resources/js/composables/checkout/), module-level refs that cause state sharing across component instances are intentional architectural choices, not issues to flag.Applied to files:
resources/js/stores/AlbumState.ts📚 Learning: 2025-08-22T18:56:08.725Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3642 File: resources/js/components/gallery/albumModule/AlbumHero.vue:173-183 Timestamp: 2025-08-22T18:56:08.725Z Learning: In the needSizeVariantsWatermark function in resources/js/components/gallery/albumModule/AlbumHero.vue, the original variant is intentionally excluded from the watermark check. This is a deliberate design decision to not consider original files when determining if the watermark button should be shown.Applied to files:
app/Image/Watermarker.php📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3901 File: app/Providers/AppServiceProvider.php:0-0 Timestamp: 2025-12-28T18:12:55.752Z Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).Applied to files:
app/Image/Watermarker.phpapp/Models/Extensions/SortingDecorator.php📚 Learning: 2025-10-06T21:24:16.072Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3718 File: app/Models/Extensions/SortingDecorator.php:191-0 Timestamp: 2025-10-06T21:24:16.072Z Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.Applied to files:
app/Models/Extensions/SortingDecorator.php🧬 Code graph analysis (2)
resources/js/stores/AlbumState.ts (5)
resources/js/config/constants.ts (1)
ALL(2-2)resources/js/stores/AlbumsState.ts (1)
useAlbumsStore(14-101)resources/js/stores/LycheeState.ts (1)
useLycheeStateStore(6-221)resources/js/stores/PhotosState.ts (1)
usePhotosStore(7-84)resources/js/lychee.d.ts (3)
HeadAlbumResource(580-600)HeadSmartAlbumResource(601-608)HeadTagAlbumResource(609-621)app/Image/Watermarker.php (2)
app/Models/SizeVariant.php (1)
SizeVariant(71-239)app/Actions/Diagnostics/Pipes/Checks/WatermarkerEnabledCheck.php (1)
__construct(23-26)🪛 GitHub Check: 1️⃣ PHP 8.4 - PHPStan
app/Models/Extensions/SortingDecorator.php
[failure] 214-214:
Variable $column_sorting_name might not be defined.⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 2️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.4 - sqlite -- Install
🔇 Additional comments (6)
app/Models/Extensions/SortingDecorator.php (2)
152-157: LGTM! Correct implementation of strict suffix stripping.The SQL sorting logic correctly strips the
_strictsuffix before applying theorderByclause.
175-189: LGTM! PHP-level sorting correctly handles strict variants.The two-step sanitization (removing
photos.prefix, then_strictsuffix) correctly prepares the column name for type resolution and sorting.app/Image/Watermarker.php (1)
108-111:can_watermark()API looks clean and correctly composes the two checks.
This is a straightforward replacement for the old public flag and should read well at call sites.resources/js/components/pagination/PaginationInfiniteScroll.vue (3)
1-6: LGTM: Template structure is clean and correct.The sentinel element and conditional loading indicator are properly implemented. The sentinel's fixed height (
h-4) is minimal and appropriate for an invisible trigger element.
41-57: LGTM: Race condition prevention is well-implemented.The
isEmittingflag combined with theloadingprop check provides solid protection against duplicate emissions during rapid intersection callbacks. The use ofrequestAnimationFrameto reset the flag is appropriate.
65-74: LGTM: Observer lifecycle and edge case handling are excellent.The implementation demonstrates strong attention to detail:
findScrollContainercorrectly identifies the scroll parent by traversing the DOM tree- Observer lifecycle management properly sets up on mount, cleans up on unmount, and responds to
hasMorechanges- Post-load watcher (lines 149-184) elegantly handles the critical edge case where loaded content is insufficient to fill the viewport, preventing the user from needing to manually scroll to trigger more loading
The verbose debug logging is well-gated behind
LycheeState.is_debug_enabledand will be valuable for troubleshooting in production.Also applies to: 94-133, 149-184
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
resources/js/stores/AlbumState.ts (1)
285-366: Consider extracting duplicated getHead() logic.The
load()method duplicates significant logic fromloadHead()(lines 80-141), including:
- AlbumService.getHead() call and response handling
- Race condition checks
- Password protection handling
- Type casting to Head*Resource variants
While the duplication may be intentional for different orchestration patterns, extracting the common logic into a shared helper would improve maintainability and reduce the risk of inconsistent behavior if one is updated without the other.
♻️ Refactoring approach
Consider extracting the shared getHead() call and response handling into a private method, or having
load()callthis.loadHead()followed by conditional loading of albums/photos. The race condition tracking would need careful consideration in either approach.resources/js/components/gallery/albumModule/AlbumPanel.vue (1)
223-229: Consider adding user feedback for page navigation errors.While the store methods handle errors internally, providing user-facing feedback at the component level (e.g., a toast notification) would improve UX if a page load fails. Currently, users would see no indication of failure.
📋 Optional enhancement
function goToPhotosPage(page: number) { - albumStore.loadPhotos(page, false); + albumStore.loadPhotos(page, false).catch(() => { + toast.add({ severity: 'error', summary: 'Error', detail: 'Failed to load photos page', life: 3000 }); + }); } function goToAlbumsPage(page: number) { - albumStore.loadAlbums(page, false); + albumStore.loadAlbums(page, false).catch(() => { + toast.add({ severity: 'error', summary: 'Error', detail: 'Failed to load albums page', life: 3000 }); + }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Models/Extensions/SortingDecorator.phpresources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.vue: Use TypeScript in composition API for Vue3 and use PrimeVue for UI components
Do not use await async calls in Vue3; use .then() instead
Do not use const function = () => {} syntax; use function functionName() {} instead in Vue3
In Vue3 components, the structure should be first, then <script lang="ts">, then <style>Files:
resources/js/components/gallery/albumModule/AlbumPanel.vue**/*.{vue,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{vue,ts,js}: For Vue3/TypeScript frontend code, follow coding conventions defined in docs/specs/3-reference/coding-conventions.md for style, naming, and testing practices.
For frontend changes, run npm run check to ensure all frontend tests pass before committing.Files:
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts**/*.{vue,ts,js,css}
📄 CodeRabbit inference engine (AGENTS.md)
For frontend changes, run npm run format to apply frontend code formatting with Prettier before committing.
Files:
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...Files:
app/Models/Extensions/SortingDecorator.php🧠 Learnings (14)
📓 Common learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3718 File: app/Models/Extensions/SortingDecorator.php:191-0 Timestamp: 2025-10-06T21:24:16.072Z Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.📚 Learning: 2025-08-16T10:57:51.480Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/views/gallery-panels/Albums.vue:11-13 Timestamp: 2025-08-16T10:57:51.480Z Learning: In resources/js/views/gallery-panels/Albums.vue, the SelectDrag component is intentionally configured with :with-scroll="false" even though the #galleryView container has overflow-y-auto. This is working as intended according to the maintainer, who suggested that overflow-y-auto might be removed instead.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vue📚 Learning: 2025-08-16T10:55:08.753Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/components/gallery/albumModule/thumbs/AlbumThumbImage.vue:41-42 Timestamp: 2025-08-16T10:55:08.753Z Learning: In AlbumThumbImage.vue, the isSelectable prop is intentionally optional without a default value. When undefined, the template condition `isDragging && !isSelectable ? '' : props.class` will strip props.class during dragging by design. Components should explicitly pass isSelectable={true} if they want to preserve styling during drag operations.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vue📚 Learning: 2025-08-16T10:56:04.248Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/composables/album/dragAndSelect.ts:68-74 Timestamp: 2025-08-16T10:56:04.248Z Learning: In the Lychee drag-and-select implementation (resources/js/composables/album/dragAndSelect.ts), the X offset handling is intentionally not implemented as it's not needed for the functionality. The current coordinate system mixing pageY with viewport-based rects has been tested and works as expected.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vue📚 Learning: 2025-11-30T17:19:00.773Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3837 File: resources/js/composables/checkout/useStepTwo.ts:8-16 Timestamp: 2025-11-30T17:19:00.773Z Learning: In the Lychee project's checkout composables (resources/js/composables/checkout/), module-level refs that cause state sharing across component instances are intentional architectural choices, not issues to flag.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts📚 Learning: 2025-08-16T14:00:53.808Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/composables/selections/selections.ts:145-147 Timestamp: 2025-08-16T14:00:53.808Z Learning: In the Lychee codebase, the maintainer ildyria has indicated that bounds checking for array access in selectables.albums.value[idx] within the albumClick function in resources/js/composables/selections/selections.ts is not necessary, suggesting there are adequate safeguards elsewhere in the code or the access pattern is guaranteed to be safe.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts📚 Learning: 2025-09-28T12:43:29.852Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/views/gallery-panels/Album.vue:200-204 Timestamp: 2025-09-28T12:43:29.852Z Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts📚 Learning: 2026-01-10T17:13:49.763Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3952 File: resources/js/components/forms/album/AlbumProperties.vue:20-20 Timestamp: 2026-01-10T17:13:49.763Z Learning: PrimeVue v4.0+ deprecated inputId for Select and related form components in favor of labelId. In Vue templates, use kebab-case label-id (label-id="...") to set the labelId prop. Also replace inputStyle and inputClass with labelStyle and labelClass. Apply these changes to all Vue components that interact with PrimeVue form controls, not just a single file.Applied to files:
resources/js/components/gallery/albumModule/AlbumPanel.vue📚 Learning: 2025-10-06T21:24:16.072Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3718 File: app/Models/Extensions/SortingDecorator.php:191-0 Timestamp: 2025-10-06T21:24:16.072Z Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.Applied to files:
app/Models/Extensions/SortingDecorator.php📚 Learning: 2025-09-21T20:07:11.825Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3702 File: app/Http/Resources/Shop/CatalogResource.php:11-16 Timestamp: 2025-09-21T20:07:11.825Z Learning: Spatie\LaravelData\DataCollection does NOT extend Illuminate\Support\Collection - it only implements some collection methods. To use Collection type, you must explicitly collect with Collection::class as the second parameter.Applied to files:
app/Models/Extensions/SortingDecorator.php📚 Learning: 2025-09-21T20:07:11.825Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3702 File: app/Http/Resources/Shop/CatalogResource.php:11-16 Timestamp: 2025-09-21T20:07:11.825Z Learning: Spatie\LaravelData\DataCollection extends Illuminate\Support\Collection, so DataCollection instances can be assigned to Collection-typed properties without runtime errors.Applied to files:
app/Models/Extensions/SortingDecorator.php📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3901 File: app/Providers/AppServiceProvider.php:0-0 Timestamp: 2025-12-28T18:12:55.752Z Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).Applied to files:
app/Models/Extensions/SortingDecorator.php📚 Learning: 2025-09-28T12:04:53.277Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/stores/TagState.ts:34-36 Timestamp: 2025-09-28T12:04:53.277Z Learning: In the Lychee codebase, different API services have different response structures. TagsService.get() returns TagWithPhotosResource directly with fields id, name, and photos (no "resource" wrapper field), while AlbumService.get() returns a response with data.resource that can be null. Always check the actual TypeScript definitions before assuming response structure.Applied to files:
resources/js/stores/AlbumState.ts📚 Learning: 2025-09-28T08:39:34.280Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3721 File: resources/js/stores/NsfwConsentedState.ts:3-5 Timestamp: 2025-09-28T08:39:34.280Z Learning: In Lychee codebase, all Pinia store files follow the pattern of declaring the type alias before the store definition (e.g., `export type StoreType = ReturnType<typeof useStore>; export const useStore = defineStore(...)`). This pattern compiles successfully with TypeScript 5.9 and vue-tsc.Applied to files:
resources/js/stores/AlbumState.ts🧬 Code graph analysis (2)
app/Models/Extensions/SortingDecorator.php (3)
app/Models/Order.php (1)
items(121-124)resources/js/lychee.d.ts (2)
ColumnSortingType(41-52)OrderSortingType(127-127)app/Enum/Traits/DecorateBackedEnum.php (1)
values(46-49)resources/js/stores/AlbumState.ts (5)
resources/js/config/constants.ts (1)
ALL(2-2)resources/js/stores/AlbumsState.ts (1)
useAlbumsStore(14-101)resources/js/stores/LycheeState.ts (1)
useLycheeStateStore(6-221)resources/js/stores/PhotosState.ts (1)
usePhotosStore(7-101)resources/js/lychee.d.ts (3)
HeadAlbumResource(580-600)HeadSmartAlbumResource(601-608)HeadTagAlbumResource(609-621)⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 2️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.4 - sqlite -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
🔇 Additional comments (14)
app/Models/Extensions/SortingDecorator.php (5)
17-17: LGTM - Clear alias for Collection types.The
BaseCollectionalias appropriately distinguishesIlluminate\Support\CollectionfromEloquent\Collection, improving type clarity in method signatures.
140-166: LGTM - Clean extraction of SQL sorting logic.The
applySqlSorting()method correctly:
- Strips the
_strictsuffix before SQL ordering (line 151), as SQL columns don't carry this marker- Processes criteria from
pivot_idx + 1onwards, ensuring only SQL-compatible sorts are applied- Handles the edge case where
pivot_idx === -1by processing all criteria
168-195: LGTM - PHP sorting correctly implements post-pagination natural sort.The
applyPhpSorting()method correctly:
- Applies sorts in reverse order (lines 180-192) for stable sorting behavior
- Strips both
photos.prefix (line 182) and_strictsuffix (line 183) to match model properties- Uses
SORT_NATURAL | SORT_FLAG_CASEfor title/description columns (line 186) to enable human-friendly ordering- Returns the sorted collection with reset keys via
values()Based on learnings, PHP-level sorting occurs after pagination is an intentional trade-off to avoid memory issues from loading all items before pagination.
197-218: LGTM - Clean refactoring with proper delegation.The
get()method now cleanly delegates toapplySqlSorting()andapplyPhpSorting(), improving separation of concerns while preserving the existing behavior.
232-245: LGTM - Paginate correctly applies PHP sorting post-pagination.The
paginate()method:
- Delegates SQL sorting (line 237), then paginates (line 240)
- Correctly wraps
$result->items()incollect()(line 242) to produce aBaseCollectionforapplyPhpSorting()- Constructs a new
LengthAwarePaginatorwith sorted items while preserving pagination metadata (line 244)Based on learnings, applying PHP sorting after pagination is an intentional trade-off to avoid loading all items into memory, which means sorting is per-page rather than globally correct across all pages.
resources/js/stores/AlbumState.ts (7)
14-16: LGTM - Resource type updates align with pagination refactor.The migration to
Head*Resourcetypes is consistent with the PR's goal to separate metadata from child collections, enabling efficient paginated loading.
24-42: Well-structured pagination state with backward compatibility.The separate pagination states for photos and albums, along with retained legacy state, provide clean separation while maintaining compatibility with existing smart album pagination.
69-141: Excellent race condition handling in loadHead().The method properly guards against multiple concurrent loads and user navigation during fetch by:
- Capturing the requested album ID before the async call
- Checking consistency at multiple points (lines 96, 105, 122, 136)
- Preventing state corruption from stale responses
143-191: LGTM - Clean pagination implementation with append/replace modes.The append parameter elegantly handles both infinite scroll (append=true) and page navigation (append=false) scenarios. Race condition guards prevent stale data from overwriting current state.
193-241: LGTM - Proper delegation to PhotosState for timeline handling.The method correctly delegates timeline merging logic to
PhotosState.appendPhotos()andPhotosState.setPhotos(), maintaining separation of concerns while supporting both pagination modes.
243-283: LGTM - Well-guarded convenience methods for infinite scroll.Both
loadMorePhotos()andloadMoreAlbums()properly prevent:
- Duplicate requests during ongoing loads
- Loading beyond the last available page
These guards are essential for reliable infinite scroll UX.
390-410: LGTM - Comprehensive pagination getters.The new getters provide all necessary pagination state:
hasMore*: Navigation guards for "load more" buttons*RemainingCount: UI feedback for remaining itemshas*Pagination: Feature detection for conditional renderingThe
Math.max(0, ...)guard prevents negative remaining counts.resources/js/components/gallery/albumModule/AlbumPanel.vue (2)
55-69: LGTM - Well-integrated albums pagination component.The Pagination component properly binds to albumStore's pagination state and responds to both "load more" (infinite scroll) and "go to page" events. The conditional rendering ensures the UI only appears when both albums exist and pagination metadata is available.
83-97: LGTM - Consistent photos pagination implementation.The photos pagination mirrors the albums implementation with appropriate state bindings and event handlers. The dual condition ensures proper rendering only when photos are loaded and pagination is initialized.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lang/fa/gallery.php (1)
245-263: “Strict/Lexico” sort option labels are English infa.
Same concern as pagination: these will show up untranslated in the Persian UI.Proposed update (placeholder translations; please adjust to preferred terminology)
- 'photo_select_3_strict' => 'Title (Lexico)', - 'photo_select_4_strict' => 'Description (Lexico)', + 'photo_select_3_strict' => 'عنوان (Lexico)', + 'photo_select_4_strict' => 'توضیحات (Lexico)', ... - 'album_select_2_strict' => 'Title (Lexico)', - 'album_select_3_strict' => 'Description (Lexico)', + 'album_select_2_strict' => 'عنوان (Lexico)', + 'album_select_3_strict' => 'توضیحات (Lexico)',
🧹 Nitpick comments (7)
lang/it/gallery.php (2)
70-78: Pagination strings are in English, not Italian.The new
paginationblock contains English strings (e.g., "Infinite scroll", "Load more button", "Loading...") rather than Italian translations. Other parts of this file use Italian (e.g.,'pinned_albums' => 'Album fissati').Based on learnings, you may not prioritize strict localization consistency, but flagging this for awareness in case Italian translations are intended:
'infinite_scroll' => 'Scorrimento infinito''load_more_button' => 'Pulsante carica altro''page_navigation' => 'Navigazione pagina''loading' => 'Caricamento...''load_more' => 'Carica altro''load_more_photos' => 'Carica altro (%s foto rimanenti)''load_more_albums' => 'Carica altro (%s album rimanenti)'
248-260: Sort option labels use English abbreviations.The sort options use English abbreviations "(Nat)" and "(Lexico)" which may not be clear to Italian users. Consider using Italian equivalents like "(Naturale)" and "(Lessicografico)" for consistency with the target locale.
That said, technical abbreviations are sometimes left in English intentionally for clarity across locales. Based on learnings regarding localization preferences, this is optional.
lang/fr/gallery.php (1)
248-260: Sort labels: consider standardizing the abbreviations (“Nat”, “Lexico”)Not a blocker, but “Nat”/“Lexico” are a bit opaque in French; consider “Nat.”/“Lex.” or “Naturel”/“Lexicographique” if you want maximum clarity.
lang/ru/all_settings.php (1)
306-325: Consider clarifying the description forwebshop_manual_fulfill_enabled.Line 325's description "Enable auto-fulfillment of orders on manual action." could be confusing since the key name contains "manual_fulfill" but the description mentions "auto-fulfillment". The detailed description at line 642 explains this better.
Consider rewording to something like:
📝 Suggested wording
- 'webshop_manual_fulfill_enabled' => 'Enable auto-fulfillment of orders on manual action.', + 'webshop_manual_fulfill_enabled' => 'Enable automatic content delivery when manually marking orders as delivered.',lang/nl/all_settings.php (1)
306-326: Verify language: English content in Dutch language file path.The file path
lang/nl/all_settings.phpsuggests this should contain Dutch translations, but all content (both existing and newly added) is in English. While the new translation keys are well-written with clear descriptions, appropriate security warnings, and performance considerations, please confirm whether:
- This is intentional (English is the standard for this file despite the
nldirectory)- These entries should be translated to Dutch
- The
nldirectory name refers to something other than "Nederlands"The new keys follow the established pattern in the file consistently, covering pagination settings (albums/photos per page, UI modes, infinite scroll thresholds), rating features, unlock propagation, and webshop configurations. The documentation is clear and professional.
Minor note: Lines 641-642 both mention content being "automatically made available to the user when possible" - consider slightly varying the wording to distinguish between auto-fulfillment (on payment completion) and manual fulfillment (on admin action).
Also applies to: 623-643
lang/zh_TW/all_settings.php (1)
325-325: Clarify potentially confusing wording.The description "Enable auto-fulfillment of orders on manual action" contains contradictory terms. The phrase combines "auto-fulfillment" (suggesting automatic) with "on manual action" (requiring manual trigger).
Based on Line 642's details, the feature automatically makes content available when manually marked as delivered. Consider rewording for clarity:
📝 Suggested wording improvements
- 'webshop_manual_fulfill_enabled' => 'Enable auto-fulfillment of orders on manual action.', + 'webshop_manual_fulfill_enabled' => 'Enable fulfillment of orders on manual action.',Or, if emphasizing the automatic content delivery aspect:
- 'webshop_manual_fulfill_enabled' => 'Enable auto-fulfillment of orders on manual action.', + 'webshop_manual_fulfill_enabled' => 'Enable automatic content delivery when manually marking orders as delivered.',lang/el/gallery.php (1)
70-78: Greek locale adds English pagination strings; also fix minor consistency (ellipsis/capitalization).If
lang/elis intended to be Greek, these new values should be translated (or at least kept stylistically consistent: ellipsis, sentence case). Also consider pluralization for “photo(s)/album(s)” if the i18n system supports it.Possible consistency-only tweak (no translation)
'pagination' => [ 'infinite_scroll' => 'Infinite scroll', 'load_more_button' => 'Load more button', 'page_navigation' => 'Page navigation', - 'loading' => 'Loading...', - 'load_more' => 'Load More', + 'loading' => 'Loading…', + 'load_more' => 'Load more', 'load_more_photos' => 'Load More (%s photos remaining)', 'load_more_albums' => 'Load More (%s albums remaining)', ],
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
lang/ar/all_settings.phplang/ar/gallery.phplang/cz/all_settings.phplang/cz/gallery.phplang/de/all_settings.phplang/de/gallery.phplang/el/all_settings.phplang/el/gallery.phplang/en/all_settings.phplang/en/gallery.phplang/es/all_settings.phplang/es/gallery.phplang/fa/all_settings.phplang/fa/gallery.phplang/fr/all_settings.phplang/fr/gallery.phplang/hu/all_settings.phplang/hu/gallery.phplang/it/all_settings.phplang/it/gallery.phplang/ja/all_settings.phplang/ja/gallery.phplang/nl/all_settings.phplang/nl/gallery.phplang/no/all_settings.phplang/no/gallery.phplang/pl/all_settings.phplang/pl/gallery.phplang/pt/all_settings.phplang/pt/gallery.phplang/ru/all_settings.phplang/ru/gallery.phplang/sk/all_settings.phplang/sk/gallery.phplang/sv/all_settings.phplang/sv/gallery.phplang/vi/all_settings.phplang/vi/gallery.phplang/zh_CN/all_settings.phplang/zh_CN/gallery.phplang/zh_TW/all_settings.phplang/zh_TW/gallery.phpresources/js/components/pagination/PaginationLoadMore.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/js/components/pagination/PaginationLoadMore.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
lang/ar/all_settings.phplang/en/gallery.phplang/it/gallery.phplang/ru/gallery.phplang/nl/gallery.phplang/no/all_settings.phplang/no/gallery.phplang/cz/all_settings.phplang/el/all_settings.phplang/pt/gallery.phplang/cz/gallery.phplang/fr/gallery.phplang/es/all_settings.phplang/sv/all_settings.phplang/de/all_settings.phplang/ru/all_settings.phplang/fa/gallery.phplang/zh_TW/gallery.phplang/fa/all_settings.phplang/zh_TW/all_settings.phplang/sk/gallery.phplang/ja/gallery.phplang/es/gallery.phplang/ar/gallery.phplang/vi/gallery.phplang/vi/all_settings.phplang/it/all_settings.phplang/de/gallery.phplang/nl/all_settings.phplang/pt/all_settings.phplang/fr/all_settings.phplang/en/all_settings.phplang/sk/all_settings.phplang/zh_CN/all_settings.phplang/el/gallery.phplang/sv/gallery.phplang/pl/all_settings.phplang/pl/gallery.phplang/hu/all_settings.phplang/ja/all_settings.phplang/zh_CN/gallery.phplang/hu/gallery.php
🧠 Learnings (7)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3718
File: app/Models/Extensions/SortingDecorator.php:191-0
Timestamp: 2025-10-06T21:24:16.072Z
Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.
📚 Learning: 2025-12-19T21:01:32.168Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/pl/webshop.php:1-2
Timestamp: 2025-12-19T21:01:32.168Z
Learning: In the Lychee repository, PHP files under the lang/ directory (and its subdirectories) do not require the standard license header. This is an exception to the general PHP license header rule. Ensure all non-lang PHP files continue to include the license header.
Applied to files:
lang/ar/all_settings.phplang/en/gallery.phplang/it/gallery.phplang/ru/gallery.phplang/nl/gallery.phplang/no/all_settings.phplang/no/gallery.phplang/cz/all_settings.phplang/el/all_settings.phplang/pt/gallery.phplang/cz/gallery.phplang/fr/gallery.phplang/es/all_settings.phplang/sv/all_settings.phplang/de/all_settings.phplang/ru/all_settings.phplang/fa/gallery.phplang/zh_TW/gallery.phplang/fa/all_settings.phplang/zh_TW/all_settings.phplang/sk/gallery.phplang/ja/gallery.phplang/es/gallery.phplang/ar/gallery.phplang/vi/gallery.phplang/vi/all_settings.phplang/it/all_settings.phplang/de/gallery.phplang/nl/all_settings.phplang/pt/all_settings.phplang/fr/all_settings.phplang/en/all_settings.phplang/sk/all_settings.phplang/zh_CN/all_settings.phplang/el/gallery.phplang/sv/gallery.phplang/pl/all_settings.phplang/pl/gallery.phplang/hu/all_settings.phplang/ja/all_settings.phplang/zh_CN/gallery.phplang/hu/gallery.php
📚 Learning: 2025-12-19T21:01:45.910Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/ar/webshop.php:1-2
Timestamp: 2025-12-19T21:01:45.910Z
Learning: Do not require license headers for language translation files under the lang/ directory (e.g., lang/ar/webshop.php). These resource files are exempt from header checks; apply header enforcement to other PHP source files in the repo.
Applied to files:
lang/ar/all_settings.phplang/en/gallery.phplang/it/gallery.phplang/ru/gallery.phplang/nl/gallery.phplang/no/all_settings.phplang/no/gallery.phplang/cz/all_settings.phplang/el/all_settings.phplang/pt/gallery.phplang/cz/gallery.phplang/fr/gallery.phplang/es/all_settings.phplang/sv/all_settings.phplang/de/all_settings.phplang/ru/all_settings.phplang/fa/gallery.phplang/zh_TW/gallery.phplang/fa/all_settings.phplang/zh_TW/all_settings.phplang/sk/gallery.phplang/ja/gallery.phplang/es/gallery.phplang/ar/gallery.phplang/vi/gallery.phplang/vi/all_settings.phplang/it/all_settings.phplang/de/gallery.phplang/nl/all_settings.phplang/pt/all_settings.phplang/fr/all_settings.phplang/en/all_settings.phplang/sk/all_settings.phplang/zh_CN/all_settings.phplang/el/gallery.phplang/sv/gallery.phplang/pl/all_settings.phplang/pl/gallery.phplang/hu/all_settings.phplang/ja/all_settings.phplang/zh_CN/gallery.phplang/hu/gallery.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
lang/ar/all_settings.phplang/en/gallery.phplang/it/gallery.phplang/ru/gallery.phplang/nl/gallery.phplang/no/all_settings.phplang/no/gallery.phplang/cz/all_settings.phplang/el/all_settings.phplang/pt/gallery.phplang/cz/gallery.phplang/fr/gallery.phplang/es/all_settings.phplang/sv/all_settings.phplang/de/all_settings.phplang/ru/all_settings.phplang/fa/gallery.phplang/zh_TW/gallery.phplang/fa/all_settings.phplang/zh_TW/all_settings.phplang/sk/gallery.phplang/ja/gallery.phplang/es/gallery.phplang/ar/gallery.phplang/vi/gallery.phplang/vi/all_settings.phplang/it/all_settings.phplang/de/gallery.phplang/nl/all_settings.phplang/pt/all_settings.phplang/fr/all_settings.phplang/en/all_settings.phplang/sk/all_settings.phplang/zh_CN/all_settings.phplang/el/gallery.phplang/sv/gallery.phplang/pl/all_settings.phplang/pl/gallery.phplang/hu/all_settings.phplang/ja/all_settings.phplang/zh_CN/gallery.phplang/hu/gallery.php
📚 Learning: 2025-08-22T06:11:18.329Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3641
File: lang/no/settings.php:9-9
Timestamp: 2025-08-22T06:11:18.329Z
Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.
Applied to files:
lang/en/gallery.phplang/no/all_settings.phplang/el/all_settings.phplang/cz/gallery.phplang/es/all_settings.phplang/fa/all_settings.phplang/it/all_settings.phplang/de/gallery.phplang/pt/all_settings.phplang/pl/gallery.phplang/hu/all_settings.phplang/ja/all_settings.php
📚 Learning: 2025-08-27T08:48:32.956Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3654
File: lang/pl/gallery.php:210-210
Timestamp: 2025-08-27T08:48:32.956Z
Learning: The user ildyria does not prioritize strict localization consistency for new menu items in language files, as indicated by their "Lang = don't care" response when suggested to translate 'Import from Server' to Polish in lang/pl/gallery.php.
Applied to files:
lang/it/gallery.phplang/ru/gallery.phplang/cz/gallery.phplang/de/gallery.phplang/pl/gallery.php
📚 Learning: 2025-10-06T21:24:16.072Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3718
File: app/Models/Extensions/SortingDecorator.php:191-0
Timestamp: 2025-10-06T21:24:16.072Z
Learning: In the Lychee codebase (LycheeOrg/Lychee), when implementing pagination with the SortingDecorator class, it's acceptable for PHP-level sorting (used for natural sorting of title/description columns) to occur after pagination rather than before, even though this means sorting is per-page rather than globally correct across all pages. This is an intentional trade-off to avoid memory issues from loading all items before pagination. A config disclaimer should document this limitation.
Applied to files:
lang/fr/gallery.phplang/fa/gallery.phplang/zh_TW/gallery.phplang/ja/gallery.phplang/de/gallery.phplang/hu/gallery.php
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 1️⃣ PHP tests / 8.4 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
Summary by CodeRabbit
New Features
Configuration
UI/UX
Tests & Docs
✏️ Tip: You can customize this high-level summary in your review settings.