Skip to content

Conversation

@ildyria
Copy link
Member

@ildyria ildyria commented Jan 4, 2026

On async mode, the value returned is -1, which lead to not seeing the Maintenance tab.

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Adjusted maintenance feature card visibility to display in broader scenarios
  • Refined button availability conditions to better align with card visibility
  • Improved edge case handling in maintenance-related interface components

✏️ Tip: You can customize this high-level summary in your review settings.

@ildyria ildyria requested a review from a team as a code owner January 4, 2026 15:48
@ildyria ildyria added the ignore-for-release Do not publish this PR in the release summary. label Jan 4, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

Two maintenance UI components have updated visibility conditions. Card rendering now triggers when data is non-zero rather than positive, and button visibility logic is simplified by removing explicit undefined checks while relying on parent card gating. One component adds an early return for description computation when data equals -1.

Changes

Cohort / File(s) Summary
Maintenance component visibility refactoring
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue, resources/js/components/maintenance/MaintenanceFulfillPrecompute.vue
Card visibility condition changed from data > 0 to data !== 0, expanding when cards render. Button visibility simplified by removing explicit undefined check, relying on card-level gating instead. In MaintenanceFulfillPrecompute only: description computation now returns empty string early when data.value === -1.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 With whiskers twitched and hoppy delight,
Zero or not, we show the sight!
Cards now dance when data ain't flat,
And buttons bow—imagine that!
Minus one means hush, no word to say,
Cleaner logic wins the day!

Pre-merge checks

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

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

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
resources/js/components/maintenance/MaintenanceFulfillPrecompute.vue (1)

43-45: Consider providing feedback for async mode instead of empty description.

When data.value === -1 (async mode), the card and button are visible but the description is empty. This creates a potentially confusing UX where users see a clickable button without understanding what it does or the current state.

🔎 Suggested improvement for async mode messaging
 const description = computed(() => {
 	if (data.value === -1) {
-		return "";
+		return trans("maintenance.fulfill-precompute.async_processing");
 	}
 	return sprintf(trans("maintenance.fulfill-precompute.description"), data.value);
 });

This would require adding a translation key like "maintenance.fulfill-precompute.async_processing": "Processing in background. Click to refresh status." to provide context to users about the async state.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a346e50 and 7be7dc1.

📒 Files selected for processing (2)
  • resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue
  • resources/js/components/maintenance/MaintenanceFulfillPrecompute.vue
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/MaintenanceFulfillPrecompute.vue
  • resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.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/MaintenanceFulfillPrecompute.vue
  • resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.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/MaintenanceFulfillPrecompute.vue
  • resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue
⏰ 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). (10)
  • GitHub Check: 3️⃣ Dockerfile Lint
  • GitHub Check: 2️⃣ PHP dist / 8.5 - sqlite
  • GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
  • GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
  • GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
  • GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
  • GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
  • GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
  • GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
  • GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
🔇 Additional comments (2)
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue (1)

3-3: LGTM: Visibility conditions now align with existing async mode handling.

The changes correctly update the card and button visibility logic to be consistent with the existing description guard at lines 43-45. The component now properly handles async mode (-1) throughout, and the button condition is appropriately simplified since the card's v-if already prevents rendering when data is undefined.

The changes maintain consistency with MaintenanceFulfillPrecompute.vue and properly address the PR objective.

Also applies to: 19-19

resources/js/components/maintenance/MaintenanceFulfillPrecompute.vue (1)

3-3: LGTM: Card and button visibility logic correctly handles async mode.

The changes from data > 0 to data !== 0 appropriately show the maintenance card when the API returns -1 (async mode), addressing the PR objective. The button visibility logic is also correctly simplified since the card's v-if already gates against undefined values.

However, please verify whether there are other maintenance components in the codebase that should receive the same treatment:

#!/bin/bash
# Description: Find other maintenance components that might need similar async mode handling

# Search for maintenance components with similar patterns
fd -e vue . resources/js/components/maintenance/ --exec echo "=== {} ===" \; --exec cat {}

Also applies to: 19-19

@codecov
Copy link

codecov bot commented Jan 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.11%. Comparing base (af8359a) to head (711a50c).
⚠️ Report is 1 commits behind head on master.

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

@ildyria ildyria merged commit 856f372 into master Jan 4, 2026
42 checks passed
@ildyria ildyria deleted the add-maintenance-recompute branch January 4, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore-for-release Do not publish this PR in the release summary.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants