-
-
Notifications
You must be signed in to change notification settings - Fork 362
Do not load duplicates directly, it crashes the DB... #3920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughA new translation key Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks✅ Passed checks (1 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resources/js/components/maintenance/MaintenanceDuplicateChecker.vue (1)
1-64: Run frontend quality checks before committing.Ensure you run
npm run formatto format the code andnpm run checkto verify TypeScript type checking passes.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
lang/ar/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/en/maintenance.phplang/es/maintenance.phplang/fa/maintenance.phplang/fr/maintenance.phplang/hu/maintenance.phplang/it/maintenance.phplang/ja/maintenance.phplang/nl/maintenance.phplang/no/maintenance.phplang/pl/maintenance.phplang/pt/maintenance.phplang/ru/maintenance.phplang/sk/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/zh_CN/maintenance.phplang/zh_TW/maintenance.phpresources/js/components/maintenance/MaintenanceDuplicateChecker.vue
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/fr/maintenance.phplang/cz/maintenance.phplang/fa/maintenance.phplang/sk/maintenance.phplang/pl/maintenance.phplang/no/maintenance.phplang/ar/maintenance.phplang/nl/maintenance.phplang/zh_TW/maintenance.phplang/ja/maintenance.phplang/zh_CN/maintenance.phplang/pt/maintenance.phplang/hu/maintenance.phplang/ru/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/en/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/es/maintenance.phplang/it/maintenance.php
**/*.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/maintenance/MaintenanceDuplicateChecker.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/maintenance/MaintenanceDuplicateChecker.vue**/*.{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/maintenance/MaintenanceDuplicateChecker.vue🧠 Learnings (5)
📓 Common learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3702 File: database/migrations/2025_08_27_203030_create_purchasable.php:35-39 Timestamp: 2025-09-21T20:09:31.762Z Learning: In the Lychee codebase, ildyria has decided to ignore the unique constraint issue with UNIQUE(album_id, photo_id) in the purchasables table where NULL values in MySQL/MariaDB don't enforce uniqueness as expected. This is acceptable for their multi-database support strategy (sqlite, pgsql, mariadb) and likely handled at the application level.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3644 File: database/migrations/2025_08_24_122020_fix_timeline_config.php:25-29 Timestamp: 2025-08-24T14:33:37.747Z Learning: In the Lychee codebase, ildyria prefers not to add null checks in migrations when they can guarantee that database keys exist, as indicated by their "Key exists, don't worry about it" response to null-safety suggestions in database/migrations/2025_08_24_122020_fix_timeline_config.php.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3637 File: lang/nl/renamer.php:10-94 Timestamp: 2025-08-20T20:35:04.474Z Learning: In Lychee, translation files are initially created with English strings as placeholders, and actual translations are handled through Weblate (a web-based translation management system). This means finding English text in non-English locale files (like lang/nl/, lang/de/, etc.) is expected and part of their translation workflow, not an issue to flag.📚 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/fr/maintenance.phplang/cz/maintenance.phplang/fa/maintenance.phplang/sk/maintenance.phplang/pl/maintenance.phplang/no/maintenance.phplang/ar/maintenance.phplang/nl/maintenance.phplang/zh_TW/maintenance.phplang/ja/maintenance.phplang/zh_CN/maintenance.phplang/pt/maintenance.phplang/hu/maintenance.phplang/ru/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/en/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/es/maintenance.phplang/it/maintenance.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/fr/maintenance.phplang/cz/maintenance.phplang/fa/maintenance.phplang/sk/maintenance.phplang/pl/maintenance.phplang/no/maintenance.phplang/ar/maintenance.phplang/nl/maintenance.phplang/zh_TW/maintenance.phplang/ja/maintenance.phplang/zh_CN/maintenance.phplang/pt/maintenance.phplang/hu/maintenance.phplang/ru/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/en/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/es/maintenance.phplang/it/maintenance.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/fr/maintenance.phplang/cz/maintenance.phplang/fa/maintenance.phplang/sk/maintenance.phplang/pl/maintenance.phplang/no/maintenance.phplang/ar/maintenance.phplang/nl/maintenance.phplang/zh_TW/maintenance.phplang/ja/maintenance.phplang/zh_CN/maintenance.phplang/pt/maintenance.phplang/hu/maintenance.phplang/ru/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/en/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/es/maintenance.phplang/it/maintenance.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/cz/maintenance.phplang/pl/maintenance.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: 3️⃣ Dockerfile Lint
- GitHub Check: 2️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.5 - sqlite
- GitHub Check: 2️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- 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 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Install
🔇 Additional comments (24)
lang/fr/maintenance.php (1)
24-24: LGTM - French translation added.The translation key for the Load button is correctly added to support the explicit load functionality that prevents automatic DB-heavy duplicate queries.
lang/no/maintenance.php (1)
23-23: LGTM - Translation key added.The translation key is correctly positioned. English placeholder text will be translated through Weblate as per the standard workflow.
Based on learnings, English placeholders in non-English locale files are expected and part of the Weblate translation workflow.
lang/cz/maintenance.php (1)
23-23: LGTM - Translation key added.The new translation entry is correctly placed and will be translated through Weblate.
lang/es/maintenance.php (1)
23-23: LGTM - Translation key added.The translation key is properly positioned. The English placeholder will be localized through the Weblate translation system.
lang/vi/maintenance.php (1)
24-24: LGTM - Translation key added.The new translation entry for the Load button is correctly positioned and follows the expected workflow for localization.
lang/nl/maintenance.php (1)
23-23: LGTM! Translation key added to support explicit load action.The addition of the 'load' translation key aligns with the UI changes that introduce an explicit Load button to prevent automatic duplicate counting that could crash the database. The English placeholder follows the established Weblate translation workflow.
Based on learnings, translation files use English placeholders that are later translated through Weblate.
lang/el/maintenance.php (1)
24-24: LGTM! Translation key added correctly.The 'load' key addition is syntactically correct and consistent with the changes across all locale files to support the explicit load functionality.
lang/hu/maintenance.php (1)
24-24: LGTM! Translation key added correctly.The addition is syntactically correct and consistent with the uniform changes across all locale files.
lang/ar/maintenance.php (1)
23-23: LGTM! Translation key added correctly.The addition is syntactically correct and follows the established pattern for introducing new translation keys that will be localized through Weblate.
lang/pl/maintenance.php (1)
24-24: LGTM! Translation key added correctly.The addition is syntactically correct and consistent with the changes across all locale files to support the safety fix that prevents automatic duplicate loading.
lang/sv/maintenance.php (1)
24-24: LGTM! Translation key addition is correct.The new 'load' translation key correctly extends the duplicate-finder section to support the explicit Load button functionality. The English placeholder text will be translated through Weblate as per the established workflow.
Based on learnings, English strings in translation files are expected placeholders handled via Weblate.
lang/sk/maintenance.php (1)
24-24: LGTM! Translation key addition is correct.The new 'load' translation key correctly extends the duplicate-finder section to support the explicit Load button functionality. The English placeholder text will be translated through Weblate as per the established workflow.
Based on learnings, English strings in translation files are expected placeholders handled via Weblate.
lang/pt/maintenance.php (1)
24-24: LGTM! Translation key addition is correct.The new 'load' translation key correctly extends the duplicate-finder section to support the explicit Load button functionality. The English placeholder text will be translated through Weblate as per the established workflow.
Based on learnings, English strings in translation files are expected placeholders handled via Weblate.
lang/zh_CN/maintenance.php (1)
24-24: LGTM! Translation key addition is correct.The new 'load' translation key correctly extends the duplicate-finder section to support the explicit Load button functionality. The English placeholder text will be translated through Weblate as per the established workflow.
Based on learnings, English strings in translation files are expected placeholders handled via Weblate.
lang/ru/maintenance.php (1)
23-23: LGTM! Translation key addition is correct.The new 'load' translation key correctly extends the duplicate-finder section to support the explicit Load button functionality. The English placeholder text will be translated through Weblate as per the established workflow.
Based on learnings, English strings in translation files are expected placeholders handled via Weblate.
lang/ja/maintenance.php (1)
24-24: LGTM!The new 'load' translation key is correctly added to support the Load button functionality. The English placeholder value is expected per the Weblate translation workflow.
lang/en/maintenance.php (2)
23-23: LGTM!The new 'load' translation key is correctly added to support the explicit Load button functionality introduced in this PR.
20-20: Fix the typo in the translation value.The text "Title duplicates duplicate-finderper album" appears to have "duplicate-finder" accidentally inserted into the string. This should likely be "Title duplicates per album".
🔎 Proposed fix for the typo
- 'duplicates-title' => 'Title duplicates duplicate-finderper album', + 'duplicates-title' => 'Title duplicates per album',⛔ Skipped due to learnings
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.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/es/gallery.php:210-210 Timestamp: 2025-08-27T08:48:45.672Z Learning: The project maintainer ildyria has indicated that language localization consistency is not a priority ("Lang = don't care"), meaning English text in non-English language files is acceptable and should not be flagged as an issue.lang/de/maintenance.php (1)
23-23: LGTM!The new 'load' translation key is correctly added. The English placeholder value is expected per the Weblate translation workflow.
lang/it/maintenance.php (1)
24-24: LGTM!The new 'load' translation key is correctly added. The English placeholder value is expected per the Weblate translation workflow.
lang/zh_TW/maintenance.php (1)
24-24: LGTM!The new 'load' translation key is correctly added. The English placeholder value is expected per the Weblate translation workflow.
lang/fa/maintenance.php (1)
23-23: LGTM! Translation key added correctly.The new
loadtranslation key aligns with the explicit load functionality introduced in the Vue component. English placeholders in translation files are expected and will be translated through Weblate.Based on learnings, English text in non-English locale files is part of the translation workflow.
resources/js/components/maintenance/MaintenanceDuplicateChecker.vue (2)
43-43: LGTM! State management correctly implements explicit loading.The removal of
onMountedand addition ofisLoadedstate correctly shifts from automatic to explicit loading behavior, addressing the database crash issue mentioned in the PR title.Also applies to: 50-50
21-21: LGTM! Template logic correctly implements the loading UI flow.The conditional rendering ensures:
- Load button appears initially (
!isLoaded)- Progress spinner shows only during active loading (
data === undefined && isLoaded)- Results display once data is fetched
Also applies to: 34-36
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.