Fix product status color matching in products table#2292
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a default case to the color state resolution in ProductResource::getTableColumns() so any unrecognized status maps to "primary". Existing handling for known statuses remains; no public signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
🔇 Additional comments (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/admin/src/Filament/Resources/ProductResource.php (1)
266-275: Harden against null/unknown states and missing translations.If
$record->statuscan be null, the typedfn (string $state)may throw. Also, missing i18n keys will render the raw key. Consider a small defensive tweak:Apply this diff in-place:
- ->getStateUsing( - fn (Model $record) => $record->deleted_at ? 'deleted' : $record->status - ) - ->formatStateUsing(fn ($state) => __('lunarpanel::product.table.status.states.'.$state)) - ->color(fn (string $state): string => match ($state) { + ->getStateUsing( + fn (Model $record) => $record->deleted_at ? 'deleted' : ($record->status ?? 'unknown') + ) + ->formatStateUsing(function ($state) { + $key = 'lunarpanel::product.table.status.states.'.(string) $state; + return \Illuminate\Support\Facades\Lang::has($key) + ? __($key) + : \Illuminate\Support\Str::headline((string) $state); + }) + ->color(fn ($state): string => match ((string) $state) { 'draft' => 'warning', 'published' => 'success', 'deleted' => 'danger', default => 'primary', }),Add these imports at the top of the file if not present:
use Illuminate\Support\Facades\Lang; use Illuminate\Support\Str;Please confirm whether
statuscan be null in your dataset. If it cannot, feel free to ignore the null-guard and keep the current signature.packages/core/src/Facades/StorefrontSession.php (1)
8-27: Add @mixin for the manager to reduce drift and improve IDE support.Laravel facades commonly use
@mixinto inherit method signatures from the underlying class and avoid duplicating many@methodtags.Apply this diff to the docblock header:
/** - * @method static \Lunar\Models\Contracts\Channel getChannel() + * @mixin \Lunar\Managers\StorefrontSessionManager + * @method static \Lunar\Models\Contracts\Channel getChannel() * @method static \Lunar\Managers\StorefrontSessionManager setChannel(\Lunar\Models\Contracts\Channel $channel) * @method static \Illuminate\Support\Collection getCustomerGroups() * @method static \Lunar\Managers\StorefrontSessionManager setCustomerGroups(\Illuminate\Support\Collection $customerGroups) * @method static \Lunar\Managers\StorefrontSessionManager setCustomerGroup(\Lunar\Models\Contracts\CustomerGroup $customerGroup) * @method static \Lunar\Managers\StorefrontSessionManager resetCustomerGroups() * @method static \Lunar\Models\Contracts\Currency getCurrency() * @method static \Lunar\Managers\StorefrontSessionManager setCurrency(\Lunar\Models\Contracts\Currency $currency) * @method static \Lunar\Models\Contracts\Customer|null getCustomer() * @method static \Lunar\Managers\StorefrontSessionManager setCustomer(\Lunar\Models\Contracts\Customer $customer) * @method static void initChannel() * @method static void initCustomerGroups() * @method static void initCurrency() * @method static void initCustomer() * @method static void forget() * @method static string getSessionKey() * * @see \Lunar\Managers\StorefrontSessionManager */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/admin/src/Filament/Resources/ProductResource.php(1 hunks)packages/core/src/Facades/StorefrontSession.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: core - PHP 8.4 - L12.* ↑
- GitHub Check: core - PHP 8.4 - L12.* ↑ E
- GitHub Check: admin - PHP 8.4 - L12.* ↑ E
- GitHub Check: admin - PHP 8.4 - L11.* ↑ E
- GitHub Check: core - PHP 8.4 - L11.* ↑ E
- GitHub Check: core - PHP 8.4 - L11.* ↑
- GitHub Check: core - PHP 8.3 - L12.* ↑
- GitHub Check: core - PHP 8.3 - L11.* ↑ E
- GitHub Check: core - PHP 8.3 - L12.* ↑ E
- GitHub Check: core - PHP 8.3 - L11.* ↑
🔇 Additional comments (1)
packages/admin/src/Filament/Resources/ProductResource.php (1)
270-275: Good fix: add default color to avoid UnhandledMatchError.The default => 'primary' makes the match exhaustive and prevents crashes on unknown statuses. Looks correct.
|
Thanks @repl6669 Looks like some formatting changes to the |
972a9f6 to
c1062c9
Compare
|
Hello @alecritson, I cleaned up the formatting changes. Is everything ok now? |
If we were to use a different status than
published | draft | deleted, we get an exception in products table. This PR just addsdefaultstatus color handling.Summary by CodeRabbit