[IP-98]: Report Builder #349
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces a comprehensive Report Builder system for InvoicePlane v2, including data transfer objects (BlockDTO, GridPositionDTO), enums for block configuration (ReportBlockType, ReportBlockWidth, ReportBand, ReportDataSource, ReportTemplateType), Eloquent models (ReportTemplate, ReportBlock), services for managing templates and rendering reports, Filament admin resources with drag-and-drop designer interface, block handlers for rendering invoice sections, transformers for data conversion, and extensive test coverage spanning unit and feature tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces substantial new functionality with multiple interacting subsystems (services, DTOs, handlers, UI components), extensive test coverage across unit and feature tests, and broad architectural changes affecting the admin panel and styling. While individual components are reasonably well-structured, the heterogeneity across DTOs, services, views, handlers, and tests, combined with logic density in ReportRenderer and ReportTemplateService, and numerous new public APIs warrant careful review. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
resources/css/filament/company/orange.css (2)
59-63: CRITICAL: Update important modifier syntax for Tailwind v4.Line 61 uses the old Tailwind v3 important syntax (
!bg-orange-700). In Tailwind CSS v4, the important modifier must come at the END of the utility class name.🐛 Proposed fix for Tailwind v4 syntax
.fi-sidebar-item.fi-active { @apply text-white; - @apply !bg-orange-700; + @apply bg-orange-700!; @apply rounded-lg; }Based on library documentation, this is a breaking change that will prevent the important modifier from working correctly in Tailwind v4.
73-77: Add missing semicolon for consistency.Line 76 is missing a semicolon after the
@applystatement. While this may not cause a parse error, all other@applystatements in the file include semicolons for consistency.🔧 Proposed fix
.fi-checkbox-input { @apply ring-orange-700; @apply focus:ring-orange-500; - @apply hover:ring-orange-500 + @apply hover:ring-orange-500; }
🤖 Fix all issues with AI agents
In
@Modules/Core/Filament/Admin/Resources/ReportBlocks/Tables/ReportBlocksTable.php:
- Around line 39-47: Replace the use of ->recordActions(...) with ->actions(...)
on the table definition and remove the unused $data parameter from the
DeleteAction callback; specifically change the ActionGroup block that currently
uses ReportBlocksTable::recordActions to use ->actions([...]) and update the
DeleteAction::make('delete')->action(function (ReportBlock $record, array $data)
{ ... }) to function (ReportBlock $record) {
app(ReportBlockService::class)->deleteReportBlock($record); } so the closure
signature matches usage and the record-level actions API is correct.
- Around line 5-7: The file imports ActionGroup, DeleteAction, and EditAction
from Filament\Actions\* which are page-level actions and will error for table
records; update the use statements to import these three symbols from
Filament\Tables\Actions (i.e., replace Filament\Actions\ActionGroup,
Filament\Actions\DeleteAction, Filament\Actions\EditAction with
Filament\Tables\Actions\ActionGroup, Filament\Tables\Actions\DeleteAction,
Filament\Tables\Actions\EditAction) so the table record actions in
ReportBlocksTable.php resolve correctly.
In @Modules/Core/Services/GridSnapperService.php:
- Around line 25-33: In GridSnapperService::snap the width calculation uses the
original unclamped $position->getX() instead of the clamped $x; update the
$width expression to use $x (i.e. max(1, min($position->getWidth(),
$this->gridSize - $x))) so the available width is computed from the clamped x;
keep the existing min/max guards and return statement using
GridPositionDTO::create($x, $y, $width, $height).
In @Modules/Core/Tests/Unit/ReportTemplateServiceTest.php:
- Around line 230-240: The test mocks file repository using method name load but
the actual repository defines get; update the mock on $this->fileRepository in
ReportTemplateServiceTest to expect and stub the get method (e.g.,
->method('get')->with(1, 'test-template')->willReturn(...)) so it matches
ReportTemplateFileRepository::get, or alternatively change the repository
reference used by the service to a mock that exposes load; ensure the unique
symbols referenced ($this->fileRepository, ReportTemplateFileRepository, get)
are aligned between test and implementation.
In @resources/css/filament/company/invoiceplane-blue.css:
- Line 217: The file ends with a missing closing brace causing a parse error;
inspect the open CSS blocks (start with the .fi-topbar and .fi-sidebar selectors
and any recently modified Tailwind v4 blocks) and add the missing closing "}" to
properly terminate the innermost unclosed rule (ensure every "{" has a matching
"}" for nested rules), then re-run Biome/Prettier to confirm the syntax error is
resolved.
In @resources/css/filament/company/invoiceplane.css:
- Line 1: Update the Tailwind v4 important modifier usage in invoiceplane.css by
moving the "!" from before the utility name to after it for any @apply rules
(e.g., change occurrences like "@apply !bg-primary-700;" to the v4 form "@apply
bg-primary-700!;"); search for other @apply lines using the old "!utility"
pattern and convert them similarly so all important modifiers follow the utility
name.
🟠 Major comments (16)
Modules/Invoices/Tests/Feature/TempInvoicesTest.php-174-215 (1)
174-215: Missing section comment markers.Per coding guidelines, tests must have inline comment blocks above sections (
/* Arrange */,/* Act */,/* Assert */). This test is missing/* act */and/* assert */comments.Proposed fix
$payload = [ 'invoice_number' => 'INV-987654', 'customer_id' => $customer->getKey(), 'numbering_id' => $documentGroup->getKey(), 'user_id' => $this->user->id, 'invoiced_at' => '2025-05-10', 'invoice_due_at' => '2025-06-09', 'invoiceItems' => [ [ 'product_id' => $product->getKey(), 'quantity' => 3, 'price' => 150, 'discount' => 0, ], ], ]; + /* act */ $component = Livewire::actingAs($this->user) ->test(ListInvoices::class) ->mountAction('create') ->fillForm($payload) ->callMountedAction(); + /* assert */ $component->assertHasFormErrors(['invoice_status' => 'required']); }Modules/Invoices/Tests/Feature/TempInvoicesTest.php-312-358 (1)
312-358: Missing/* arrange */section comment.Per coding guidelines, this test is missing the
/* arrange */comment block at the start of the test setup.Proposed fix
public function it_creates_an_invoice_with_items(): void { + /* arrange */ $customer = Relation::factory()->for($this->company)->customer()->create(); $documentGroup = Numbering::factory()->for($this->company)->create();Modules/Invoices/Tests/Feature/TempInvoicesTest.php-404-444 (1)
404-444: Missing/* act */and/* assert */section comments.Per coding guidelines, this test is missing the
/* act */comment before the Livewire call and the/* assert */comment before the assertion.Proposed fix
$payload = [ 'customer_id' => $customer->id, ... ]; + /* act */ $component = Livewire::actingAs($this->user) ->test(CreateInvoice::class) ->fillForm($payload) ->call('create'); + /* assert */ $component->assertHasFormErrors(['invoice_status' => 'required']); }Modules/Invoices/Tests/Feature/InvoicesExportImportTest.php-20-236 (1)
20-236: Refactor to use data providers to eliminate duplication.All eight tests follow the same pattern with only minor variations (action name, column configuration, test data). Per coding guidelines: "Use data providers for testing multiple scenarios with the same logic in tests."
♻️ Refactor using data provider
public static function exportActionProvider(): array { return [ 'csv export' => [ 'action' => 'exportCsv', 'invoiceCount' => 3, 'columnMap' => [ 'number' => ['isEnabled' => true, 'label' => 'Invoice Number'], 'total' => ['isEnabled' => true, 'label' => 'Total'], ], 'markIncomplete' => true, ], 'excel export' => [ 'action' => 'exportExcel', 'invoiceCount' => 3, 'columnMap' => [ 'number' => ['isEnabled' => true, 'label' => 'Invoice Number'], ], 'markIncomplete' => true, ], 'csv v2 export' => [ 'action' => 'exportCsvV2', 'invoiceCount' => 3, 'columnMap' => [ 'number' => ['isEnabled' => true, 'label' => 'Number'], ], 'markIncomplete' => false, ], // ... other variations ]; } #[Test] #[Group('export')] #[DataProvider('exportActionProvider')] public function it_dispatches_export_job( string $action, int $invoiceCount, array $columnMap, bool $markIncomplete ): void { if ($markIncomplete) { $this->markTestIncomplete(); } /* Arrange */ Queue::fake(); Storage::fake('local'); if ($invoiceCount > 0) { Invoice::factory()->for($this->company)->count($invoiceCount)->create(); } /* Act */ Livewire::actingAs($this->user) ->test(ListInvoices::class) ->callAction($action, data: ['columnMap' => $columnMap]); /* Assert */ Bus::assertChained([ function ($batch) { return $batch instanceof \Illuminate\Bus\PendingBatch; }, ]); }Don't forget to add the import:
use PHPUnit\Framework\Attributes\DataProvider;Modules/Invoices/Tests/Feature/InvoicesExportImportTest.php-24-24 (1)
24-24: Complete or remove incomplete tests before merging.Five of the eight tests are marked incomplete. Tests in a PR should be ready for execution and validation. Either complete these tests to verify the export functionality or remove them until they're ready.
Also applies to: 52-52, 79-79, 106-106, 215-215
Modules/Invoices/Tests/Feature/InvoicesExportImportTest.php-41-45 (1)
41-45: Strengthen assertions to validate actual export behavior.The assertions only verify that a
PendingBatchinstance was created, but don't validate:
- The correct export job class was dispatched
- The columnMap data was passed correctly
- The batch contains the expected number of jobs
- The correct file format/type is being exported
As per coding guidelines: "Tests must be meaningful - avoid simple 'ok' checks; validate actual behavior and data."
💡 Example of stronger assertions
/* Assert */ Bus::assertChained([ function ($batch) use ($invoices) { $jobs = $batch->jobs->all(); // Verify batch structure if (!$batch instanceof \Illuminate\Bus\PendingBatch) { return false; } // Verify correct job type if (!isset($jobs[0]) || !$jobs[0] instanceof ExpectedExportJob::class) { return false; } // Verify data was passed correctly $job = $jobs[0]; if ($job->columnMap !== ['number' => [...], 'total' => [...]]) { return false; } return true; }, ]); // Additionally verify the export file was queued for storage Storage::disk('local')->assertExists('path/to/export.csv');Also applies to: 68-72, 95-99, 126-130, 152-156, 178-182, 204-208, 231-235
Modules/Core/Traits/FormatsCurrency.php-18-23 (1)
18-23: Use locale-aware currency formatting instead of hardcoded format.The current implementation has several issues:
- Hardcodes 2 decimal places (JPY uses 0, KWD uses 3)
- Hardcodes decimal/thousands separators (varies by locale)
- Uses currency code instead of symbol ("USD" vs "$")
- Doesn't respect currency position conventions
💱 Recommended solution using Laravel's Number facade
+use Illuminate\Support\Number; + trait FormatsCurrency { /** * Format a currency amount. * * @param float $amount The amount to format * @param string|null $currency The currency code (defaults to USD) * * @return string The formatted currency string */ private function formatCurrency(float $amount, ?string $currency = null): string { - $currency ??= 'USD'; - - return $currency . ' ' . number_format($amount, 2, '.', ','); + return Number::currency($amount, $currency ?? 'USD'); } }This respects locale settings, currency symbols, decimal places, and formatting conventions automatically.
Modules/Core/Handlers/FooterQrCodeBlockHandler.php-34-36 (1)
34-36: Replace external QR API with self-hosted library to eliminate privacy and reliability risks.The current implementation sends invoice
url_keyidentifiers to a third-party service (api.qrserver.com) with no error handling or fallback, creating three issues:
- Privacy: Sensitive invoice identifiers are transmitted to an external service
- Reliability: QR codes fail silently if the external service is unavailable
- Performance: External HTTP requests add latency during report generation
Replace with a self-hosted QR library. Both
endroid/qr-code(feature-rich with multiple output formats) andbacon/bacon-qr-code(lightweight core encoder) are actively maintained alternatives that generate QR codes locally.Modules/Core/Filament/Admin/Resources/ReportTemplates/Pages/ListReportTemplates.php-18-31 (1)
18-31: CreateAction missing form definition and has potential issues.Several concerns with this action:
- The
$templatevariable is assigned but never used (flagged by static analysis).- The action references
$data['name']and$data['template_type']but no->form()is defined on the CreateAction, so these fields won't exist.- If
auth()->user()returns null or the user has no companies, line 22 will throw an error.Proposed fix
return [ CreateAction::make() + ->form([ + \Filament\Forms\Components\TextInput::make('name') + ->required(), + \Filament\Forms\Components\Select::make('template_type') + ->options(\Modules\Core\Enums\ReportTemplateType::class) + ->required(), + ]) ->action(function (array $data) { $company = Company::query()->find(session('current_company_id')); if ( ! $company) { - $company = auth()->user()->companies()->first(); + $company = auth()->user()?->companies()->first(); + } + + if ( ! $company) { + throw new \RuntimeException('No company found for current user'); } - $template = app(ReportTemplateService::class)->createTemplate( + app(ReportTemplateService::class)->createTemplate( $company, $data['name'], $data['template_type'], [] ); }) ->modalWidth('full'), ];Modules/Core/Filament/Admin/Resources/ReportBlocks/Pages/EditReportBlock.php-36-52 (1)
36-52: Extract duplicate data normalization logic.The
mountUsingandfillFormclosures contain identical logic for normalizingis_activeandwidthfields. This duplication violates the DRY principle and makes maintenance harder.♻️ Refactor to eliminate duplication
+ private function normalizeRecordData(): array + { + $data = $this->record->toArray(); + $data['is_active'] = (bool) ($data['is_active'] ?? true); + if (isset($data['width']) && $data['width'] instanceof BackedEnum) { + $data['width'] = $data['width']->value; + } + return $data; + } + public function editAction(): Action { return Action::make('edit') ->label('Edit Modal') ->schema(fn (Schema $schema) => ReportBlockForm::configure($schema)) - ->mountUsing(function (Schema $schema) { - $data = $this->record->toArray(); - $data['is_active'] = (bool) ($data['is_active'] ?? true); - if (isset($data['width']) && $data['width'] instanceof BackedEnum) { - $data['width'] = $data['width']->value; - } - $schema->fill($data); - }) - ->fillForm(function () { - $data = $this->record->toArray(); - $data['is_active'] = (bool) ($data['is_active'] ?? true); - if (isset($data['width']) && $data['width'] instanceof BackedEnum) { - $data['width'] = $data['width']->value; - } - - return $data; - }) + ->mountUsing(fn (Schema $schema) => $schema->fill($this->normalizeRecordData())) + ->fillForm(fn () => $this->normalizeRecordData()) ->action(function (array $data) { $this->record->update($data); $this->record->refresh(); }); }Modules/Core/Tests/Unit/BlockDTOTest.php-251-272 (1)
251-272: Test method name should followit_prefix convention and include section comments.Per coding guidelines, unit test functions must be prefixed with
it_and include inline comment blocks (/* Arrange */,/* Act */,/* Assert */).Apply naming and comment conventions
#[Test] #[Group('unit')] - public function setters_return_self_for_method_chaining(): void + public function it_returns_self_from_setters_for_method_chaining(): void { + /* Arrange */ $position = new GridPositionDTO(); $position->setX(0)->setY(0)->setWidth(6)->setHeight(4); + /* Act */ $dto = (new BlockDTO()) ->setId('block_test') ->setType('test_type') ->setPosition($position) ->setConfig(['key' => 'value']) ->setLabel('Test Label') ->setIsCloneable(true) ->setDataSource('test_source') ->setIsCloned(false) ->setClonedFrom(null); + /* Assert */ $this->assertInstanceOf(BlockDTO::class, $dto); $this->assertEquals('block_test', $dto->getId()); $this->assertEquals('test_type', $dto->getType()); }Based on coding guidelines, unit test functions must be prefixed with
it_and include section comments.Modules/Core/Database/Seeders/ReportBlocksSeeder.php-78-93 (1)
78-93: Seeder is not idempotent — re-running creates duplicate blocks.The random slug suffix (
Str::random(8)) ensures uniqueness but makes the seeder non-idempotent. Re-running will create duplicate "Company Header", "Customer Header", etc. blocks. Consider usingfirstOrCreateorupdateOrCreatewith a stable identifier.Use updateOrCreate for idempotency
- $baseSlug = Str::slug($block['name']); - $slug = $baseSlug . '-' . Str::random(8); - $filename = $slug; - - ReportBlock::create([ + $baseSlug = Str::slug($block['name']); + + ReportBlock::updateOrCreate( + [ + 'name' => $block['name'], + 'is_system' => true, + ], + [ 'is_active' => true, - 'is_system' => true, 'block_type' => $block['block_type'], - 'name' => $block['name'], - 'slug' => $slug, - 'filename' => $filename, + 'slug' => $baseSlug, + 'filename' => $baseSlug, 'width' => $block['width'], 'data_source' => $block['data_source'], 'default_band' => $block['default_band'], - ]); + ] + );Modules/Core/DTOs/GridPositionDTO.php-42-48 (1)
42-48: Invalid default state in constructor.The no-arg constructor initializes
widthandheightto 0, which violates the validation rules enforced by thecreate()factory method (requires width > 0 and height > 0). This creates an inconsistent state where a directly instantiated DTO is invalid.If code uses
new GridPositionDTO()without immediately setting valid values, it will contain invalid data that could cause logic errors downstream.🔧 Recommended fix
Initialize to valid default values:
public function __construct() { $this->x = 0; $this->y = 0; - $this->width = 0; - $this->height = 0; + $this->width = 1; + $this->height = 1; }Modules/Core/Filament/Admin/Resources/ReportTemplates/Pages/CreateReportTemplate.php-44-57 (1)
44-57: Add safeguard for users with no companies.If
session('current_company_id')is not set and the authenticated user has no companies,companies()->first()will returnnull(line 48), which would then be passed tocreateTemplate()causing an error.While authorization guards typically ensure users have at least one company, consider adding an explicit check or throwing a meaningful exception.
🛡️ Proposed safeguard
protected function handleRecordCreation(array $data): Model { $company = Company::find(session('current_company_id')); if ( ! $company) { $company = auth()->user()->companies()->first(); } + + if ( ! $company) { + throw new \RuntimeException('User must be associated with at least one company to create a report template.'); + } return app(ReportTemplateService::class)->createTemplate( $company, $data['name'], $data['template_type'], [] ); }Modules/Core/Services/ReportBlockService.php-99-109 (1)
99-109: Use importedLogfacade instead of inline FQCN.Per coding guidelines, use full namespace imports. Add
use Illuminate\Support\Facades\Log;at the top and replace\Illuminate\Support\Facades\Log::withLog::throughout.Suggested fix
Add to imports (after line 8):
use Illuminate\Support\Facades\Log;Then replace all occurrences:
- \Illuminate\Support\Facades\Log::warning('Failed to decode existing block config, starting fresh', [ + Log::warning('Failed to decode existing block config, starting fresh', [Committable suggestion skipped: line range outside the PR's diff.
Modules/Core/Services/ReportTemplateService.php-353-382 (1)
353-382: Services should use Transformers to build DTOs.Per coding guidelines, "Services must not build DTOs manually; they must use Transformers directly." The
createSystemBlockmethod manually constructsBlockDTOinstances. Consider moving this logic toBlockTransformeror using an existing transformer method.♻️ Suggested approach
Add a method to
BlockTransformerthat creates aBlockDTOfrom aReportBlockmodel:// In BlockTransformer public static function fromReportBlock(ReportBlock $block, int $x = 0, int $y = 0): BlockDTO { $position = GridPositionDTO::create($x, $y, $block->width->getGridWidth(), 4); $dto = new BlockDTO(); $dto->setId('block_' . $block->block_type->value) ->setType($block->block_type) ->setSlug($block->slug) ->setPosition($position) ->setConfig($block->config ?: []) ->setLabel($block->name) ->setIsCloneable(true) ->setDataSource($block->data_source->value) ->setBand($block->default_band->value) ->setIsCloned(false) ->setClonedFrom(null); return $dto; }Then in
getSystemBlocks:$blocks[$dbBlock->block_type->value] = BlockTransformer::fromReportBlock($dbBlock);Based on coding guidelines, services should delegate DTO construction to transformers.
🟡 Minor comments (23)
.github/REPORT_BUILDER_ENHANCEMENTS.md-22-22 (1)
22-22: Fix: Use two-word phrasal verb "look up".Line 22 should read "properly look up blocks" rather than "lookup blocks".
Proposed fix
-- Fixed `configureBlockAction()` in `ReportBuilder.php` to properly lookup blocks using `block_type` +- Fixed `configureBlockAction()` in `ReportBuilder.php` to properly look up blocks using `block_type`.github/workflows/pint.yml-37-37 (1)
37-37: Clarify the PHP version downgrade in pint.yml and align workflow versions.The change from PHP 8.4 to 8.2 is compatible with
composer.json(requires^8.2), but the PR lacks explanation for this downgrade. Additionally, the inconsistency with other workflows is notable:
- release.yml still uses PHP 8.4
- pint.yml (after this change) would use PHP 8.2
- Other workflows vary: 8.3 (quickstart, phpunit), 8.2 (phpstan, composer-update, yarn-update)
Either justify the version choice for pint specifically, or align workflow versions consistently across the project.
.junie/guidelines.md-400-476 (1)
400-476: Remove duplicated Internationalization section.According to the AI summary, the Internationalization & Translations section appears twice in this document. Documentation duplication can lead to maintenance issues and inconsistent updates.
Please verify and remove the duplicate section, keeping only one instance of the internationalization guidelines.
Modules/Core/Tests/Unit/ReportTemplateTest.php-105-236 (1)
105-236: Add required Arrange/Act/Assert comment blocks to all test methods.Six test methods are missing the mandatory
/* Arrange */,/* Act */, and/* Assert */section comments required by the coding guidelines. These comments improve test readability and ensure proper test structure.Based on coding guidelines, all tests must have these inline comment blocks.
📝 Example fix for is_cloneable_returns_true_when_active
#[Test] #[Group('unit')] public function is_cloneable_returns_true_when_active(): void { + /* Arrange */ $template = ReportTemplate::create([ 'company_id' => $this->company->id, 'name' => 'Active Template', 'slug' => 'active_template', 'template_type' => 'invoice', 'is_system' => false, 'is_active' => true, ]); + /* Act */ + $result = $template->isCloneable(); + + /* Assert */ - $this->assertTrue($template->isCloneable()); + $this->assertTrue($result); }Apply similar changes to the other five test methods:
is_cloneable_returns_false_when_inactive,is_system_returns_true_for_system_templates,is_system_returns_false_for_user_templates,get_file_path_returns_correct_path,slug_must_be_unique_within_company, andsame_slug_can_exist_in_different_companies.Committable suggestion skipped: line range outside the PR's diff.
Modules/Core/Tests/Feature/ReportBuilderFieldCanvasIntegrationTest.php-68-68 (1)
68-68:markTestIncomplete()after assertions defeats test reliability.Same issue as in the unit tests—assertions pass first, then the test is marked incomplete. This masks passing tests as incomplete in CI results. Remove
markTestIncomplete()if tests are complete.Modules/Core/resources/views/filament/admin/resources/report-template-resource/pages/design-report-template.blade.php-304-304 (1)
304-304: Invalid Tailwind class:font-[#429AE1].The
font-[]arbitrary value expects a font family, not a color. This class has no effect. If you intended to set text color, usetext-[#429AE1].Suggested fix
- class="text-sm font-[#429AE1] text-white uppercase tracking-tight overflow-hidden text-ellipsis w-full" + class="text-sm font-medium text-white uppercase tracking-tight overflow-hidden text-ellipsis w-full"Committable suggestion skipped: line range outside the PR's diff.
Modules/Core/Tests/Unit/ReportBlockWidthTest.php-28-30 (1)
28-30:markTestIncomplete()after assertions is counterproductive.All test methods execute assertions first, then mark the test as incomplete. This causes tests to report as "incomplete" even when they pass, which obscures actual test results and CI feedback. If the tests are complete (which they appear to be), remove the
markTestIncomplete()calls. If genuinely incomplete, skip the assertions.Suggested fix (apply to all test methods)
/* Assert */ $this->assertEquals('one_third', $width->value); $this->assertEquals(4, $gridWidth); - $this->markTestIncomplete('Test implementation complete but marked incomplete as per requirements'); }Modules/Core/resources/views/filament/admin/resources/report-template-resource/pages/design-report-template.blade.php-28-29 (1)
28-29: Removeconsole.logstatements before merging.Debug logging statements (lines 28, 50, 52, 129) should be removed from production code. They expose internal state and clutter the browser console.
Suggested fix
- console.log('Initializing with loaded blocks:', loadedBlocks);resources/lang/en/ip.php-981-993 (1)
981-993: Duplicate translation key:report_field_quote_item_discount.This key is defined twice (lines 981 and 993) with the same value. PHP arrays silently overwrite duplicate keys, keeping only the last occurrence. Remove one occurrence or rename if they represent different concepts.
Suggested fix
'report_field_quote_item_subtotal' => 'Quote Item Subtotal', 'report_field_quote_item_tax_name' => 'Quote Item Tax Name', 'report_field_quote_item_tax_rate' => 'Quote Item Tax Rate', 'report_field_quote_item_total' => 'Quote Item Total', - 'report_field_quote_item_discount' => 'Quote Item Discount', 'report_field_payment_date' => 'Payment Date',Modules/Core/resources/views/filament/admin/resources/report-blocks/fields-canvas.blade.php-22-31 (1)
22-31: Accessibility: Drag-and-drop lacks keyboard alternative.The current implementation relies entirely on mouse drag operations. Users who cannot use a mouse (motor impairments, keyboard-only users) cannot add fields to the canvas.
Consider adding:
- Click-to-add as an alternative to drag
- Keyboard navigation (Tab through fields, Enter to add)
- ARIA roles for screen reader support
Modules/Core/Tests/Feature/UpdateReportTemplateTest.php-105-149 (1)
105-149: Test does not verify actual snapping behavior.The test name
it_snaps_blocks_to_grid_on_updatesuggests grid snapping is being tested, but the input positions are already grid-aligned (x: 0, y: 0, width: 6, height: 4). The assertions merely verify that valid positions remain unchanged.To meaningfully test snapping, use off-grid values and assert they get corrected:
💡 Suggested test input to verify snapping
$blocksWithValidPosition = [ [ 'id' => 'block_company_header', 'type' => 'company_header', - 'position' => ['x' => 0, 'y' => 0, 'width' => 6, 'height' => 4], + 'position' => ['x' => 3, 'y' => 7, 'width' => 5, 'height' => 3], // Off-grid values 'config' => [], 'label' => 'Company Header', // ... ], ]; /* act */ $this->service->updateTemplate($template, $blocksWithValidPosition); /* assert */ // Assert values are snapped to expected grid boundaries - $this->assertEquals(0, $savedBlocks[0]['position']['x']); - $this->assertEquals(0, $savedBlocks[0]['position']['y']); + $this->assertEquals(/* expected snapped x */, $savedBlocks[0]['position']['x']); + $this->assertEquals(/* expected snapped y */, $savedBlocks[0]['position']['y']);Committable suggestion skipped: line range outside the PR's diff.
Modules/Core/resources/views/filament/admin/resources/report-blocks/fields-canvas.blade.php-18-20 (1)
18-20: Hardcoded English strings should use@lang()for consistency.Line 17 correctly uses
@lang('ip.available_fields'), but lines 19, 55, and 82 contain hardcoded English text. For i18n consistency:🌐 Suggested fix
- <p class="text-xs text-gray-500 dark:text-gray-400 mb-3"> - Drag fields to the canvas to configure block layout - </p> + <p class="text-xs text-gray-500 dark:text-gray-400 mb-3"> + @lang('ip.drag_fields_instruction') + </p>Apply similar changes to lines 55 and 82.
Modules/Core/Tests/Feature/GridSnapperTest.php-54-70 (1)
54-70: Separate Act and Assert sections per coding guidelines.The test combines the Act & Assert phases (line 66), which violates the coding guideline requirement that "All tests MUST have an 'act' section where variables are defined BEFORE assertions."
As per coding guidelines for
**/Tests/**/*.php.✅ Proposed fix to separate Act and Assert sections
$invalidPositionWidth = new GridPositionDTO(); $invalidPositionWidth->setX(0)->setY(0)->setWidth(0)->setHeight(4); - /* Act & Assert */ - $this->assertTrue($this->gridSnapper->validate($validPosition)); - $this->assertFalse($this->gridSnapper->validate($invalidPositionX)); - $this->assertFalse($this->gridSnapper->validate($invalidPositionWidth)); + /* act */ + $validResult = $this->gridSnapper->validate($validPosition); + $invalidResultX = $this->gridSnapper->validate($invalidPositionX); + $invalidResultWidth = $this->gridSnapper->validate($invalidPositionWidth); + + /* assert */ + $this->assertTrue($validResult); + $this->assertFalse($invalidResultX); + $this->assertFalse($invalidResultWidth); }Modules/Core/Tests/Unit/BlockTransformerTest.php-247-247 (1)
247-247: Rename test to follow unit test naming convention.The test name
roundtrip_conversion_preserves_datadoes not follow the coding guideline for unit tests, which requires test functions to be prefixed withit_and make grammatical sense.As per coding guidelines: Unit test functions must be prefixed with
it_and make grammatical sense.📝 Proposed fix
- public function roundtrip_conversion_preserves_data(): void + public function it_preserves_data_during_roundtrip_conversion(): voidModules/Core/Database/Seeders/ReportBlocksSeeder.php-103-105 (1)
103-105: Undefined array key access for blocks withoutconfig.Most blocks in the
$blocksarray don't have aconfigkey defined. Line 103 accesses$block['config']without checking if it exists, which will result innulland generate{"fields":[]}for all blocks except "Item Tax Details".Use null coalescing for safety
- $config = $block['config']; + $config = $block['config'] ?? []; $config['fields'] = []; // Start with no fields as requested for drag/dropModules/Core/Tests/Unit/BlockFactoryTest.php-96-104 (1)
96-104: Add required test section comments.This test is missing the required
/* arrange */,/* act */, and/* assert */comment blocks per coding guidelines.🧪 Add section comments
public function all_returned_types_are_creatable(): void { + /* arrange */ $blockTypes = BlockFactory::all(); + /* act & assert */ foreach ($blockTypes as $block) { $handler = BlockFactory::make($block['type']); $this->assertNotNull($handler); } }Modules/Core/Tests/Unit/BlockFactoryTest.php-60-69 (1)
60-69: Add separate act section per test guidelines.The test combines the exception expectations (assert setup) with the triggering action. Per coding guidelines, all tests must have a separate
/* act */section where the action is performed before assertions.🧪 Suggested restructure
public function it_throws_exception_for_invalid_type(): void { /* arrange */ // No setup needed + /* act */ + try { + BlockFactory::make('invalid_type'); + $exceptionThrown = false; + } catch (InvalidArgumentException $e) { + $exceptionThrown = true; + $exceptionMessage = $e->getMessage(); + } + /* assert */ - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessageMatches('/Unsupported block type/i'); - BlockFactory::make('invalid_type'); + $this->assertTrue($exceptionThrown); + $this->assertMatchesRegularExpression('/Unsupported block type/i', $exceptionMessage); }Alternatively, if using PHPUnit's expectException is preferred, add a comment clarifying the act:
public function it_throws_exception_for_invalid_type(): void { /* arrange */ // No setup needed /* assert */ $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessageMatches('/Unsupported block type/i'); + + /* act */ BlockFactory::make('invalid_type'); }Modules/Core/Tests/Feature/BlockCloningTest.php-60-97 (1)
60-97: Add missing/* act */section comment.The test has
/* arrange */and/* assert */sections but is missing the/* act */section comment before the template creation and modification (lines 84-92). As per coding guidelines, all tests must have inline comment blocks for all three sections.📝 Proposed fix
$template = $this->service->createTemplate( $company, 'System Template', 'invoice', $systemBlocks ); + /* act */ $template->is_system = true; $template->save(); /* assert */Modules/Core/Tests/Feature/CreateReportTemplateTest.php-151-175 (1)
151-175: Separate act and assert sections.The test combines
/* Act & Assert */on line 165, but coding guidelines require tests to have separate act and assert sections with variables defined in the act section before assertions. Even for exception tests, maintain the separation for consistency.📝 Proposed fix
- /* Act & Assert */ + /* act */ $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage("must have a 'type'"); $this->service->createTemplate( $company, 'Test Template', 'invoice', $invalidBlocks ); + + /* assert */ + // Exception assertions aboveNote: For exception tests, the
expectExceptioncalls serve as both the act trigger and assertion setup, but maintaining separate section comments improves consistency.Modules/Core/Services/ReportRenderer.php-205-229 (1)
205-229: Potential null/property access error on$invoice->number.The
$invoiceparameter is typed asmixed(via docblock), but line 215 accesses$invoice->numberdirectly. When called fromrenderToPreviewwith sample data, this could fail if$samplelacks anumberproperty.Suggested fix
- <title>Invoice {$invoice->number}</title> + <title>Invoice {$invoice->number ?? 'Preview'}</title>Modules/Core/Filament/Admin/Resources/ReportTemplates/Pages/ReportBuilder.php-427-433 (1)
427-433: Fragile block type extraction from ID.The logic
str_replace('block_', '', $block['id'])assumes block IDs always follow theblock_{type}format. If the ID format changes or a block has multiple underscores in its type, this will break.Consider storing the
typeexplicitly when creating blocks, or using a more robust parsing approach.Modules/Core/Tests/Feature/ReportBuilderBlockEditTest.php-142-170 (1)
142-170: Test expectations may not match implementation.This test expects
Log::info()calls, but theReportBuilderclass usesLog::debug()for logging. The test appears to verify logging behavior that doesn't match the actual implementation being tested.Additionally, this test manually calls
Log::info()rather than exercising the actualReportBuildercode paths that would trigger logging.Modules/Core/Services/ReportTemplateService.php-292-294 (1)
292-294: Unused$configvariable.The static analysis correctly identifies that
$configis assigned but never used. Either use it or remove the assignment.🐛 Remove unused variable
foreach ($dbBlocks as $dbBlock) { - $config = $this->getBlockConfig($dbBlock); - // Map widths to grid units for the designer using the enum method $width = $dbBlock->width->getGridWidth();
| use Filament\Actions\ActionGroup; | ||
| use Filament\Actions\DeleteAction; | ||
| use Filament\Actions\EditAction; |
There was a problem hiding this comment.
Incorrect action imports will cause runtime errors.
The imports use Filament\Actions\* which is for page-level actions. For table record actions, you must use Filament\Tables\Actions\*.
Proposed fix
-use Filament\Actions\ActionGroup;
-use Filament\Actions\DeleteAction;
-use Filament\Actions\EditAction;
+use Filament\Tables\Actions\ActionGroup;
+use Filament\Tables\Actions\DeleteAction;
+use Filament\Tables\Actions\EditAction;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use Filament\Actions\ActionGroup; | |
| use Filament\Actions\DeleteAction; | |
| use Filament\Actions\EditAction; | |
| use Filament\Tables\Actions\ActionGroup; | |
| use Filament\Tables\Actions\DeleteAction; | |
| use Filament\Tables\Actions\EditAction; |
🤖 Prompt for AI Agents
In
@Modules/Core/Filament/Admin/Resources/ReportBlocks/Tables/ReportBlocksTable.php
around lines 5 - 7, The file imports ActionGroup, DeleteAction, and EditAction
from Filament\Actions\* which are page-level actions and will error for table
records; update the use statements to import these three symbols from
Filament\Tables\Actions (i.e., replace Filament\Actions\ActionGroup,
Filament\Actions\DeleteAction, Filament\Actions\EditAction with
Filament\Tables\Actions\ActionGroup, Filament\Tables\Actions\DeleteAction,
Filament\Tables\Actions\EditAction) so the table record actions in
ReportBlocksTable.php resolve correctly.
| ->recordActions([ | ||
| ActionGroup::make([ | ||
| EditAction::make(), | ||
| DeleteAction::make('delete') | ||
| ->action(function (ReportBlock $record, array $data) { | ||
| app(ReportBlockService::class)->deleteReportBlock($record); | ||
| }), | ||
| ]), | ||
| ]); |
There was a problem hiding this comment.
Use actions() instead of recordActions() and remove unused parameter.
Filament tables use ->actions() for record-level actions, not ->recordActions(). Also, the $data parameter in the delete callback is unused and can be removed.
Proposed fix
- ->recordActions([
+ ->actions([
ActionGroup::make([
EditAction::make(),
DeleteAction::make('delete')
- ->action(function (ReportBlock $record, array $data) {
+ ->action(function (ReportBlock $record) {
app(ReportBlockService::class)->deleteReportBlock($record);
}),
]),
]);🧰 Tools
🪛 PHPMD (2.15.0)
43-43: Avoid unused parameters such as '$data'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In
@Modules/Core/Filament/Admin/Resources/ReportBlocks/Tables/ReportBlocksTable.php
around lines 39 - 47, Replace the use of ->recordActions(...) with
->actions(...) on the table definition and remove the unused $data parameter
from the DeleteAction callback; specifically change the ActionGroup block that
currently uses ReportBlocksTable::recordActions to use ->actions([...]) and
update the DeleteAction::make('delete')->action(function (ReportBlock $record,
array $data) { ... }) to function (ReportBlock $record) {
app(ReportBlockService::class)->deleteReportBlock($record); } so the closure
signature matches usage and the record-level actions API is correct.
| public function snap(GridPositionDTO $position): GridPositionDTO | ||
| { | ||
| $x = max(0, min($position->getX(), $this->gridSize - 1)); | ||
| $y = max(0, $position->getY()); | ||
| $width = max(1, min($position->getWidth(), $this->gridSize - $position->getX())); | ||
| $height = max(1, $position->getHeight()); | ||
|
|
||
| return GridPositionDTO::create($x, $y, $width, $height); | ||
| } |
There was a problem hiding this comment.
Critical bug: Width calculation uses unclamped x value.
Line 29 uses $position->getX() (the original, unclamped value) instead of the clamped $x variable from line 27. This produces incorrect width calculations when the x-coordinate is out of bounds.
Example:
- Input: x=15, width=5, gridSize=12
- Line 27: x is clamped to 11
- Line 29: width = max(1, min(5, 12 - 15)) = max(1, -3) = 1
- Expected: width = max(1, min(5, 12 - 11)) = max(1, 1) = 1
While the result happens to be correct in this example, the logic is wrong and could produce incorrect results in other scenarios.
🐛 Fix the width calculation
public function snap(GridPositionDTO $position): GridPositionDTO
{
$x = max(0, min($position->getX(), $this->gridSize - 1));
$y = max(0, $position->getY());
- $width = max(1, min($position->getWidth(), $this->gridSize - $position->getX()));
+ $width = max(1, min($position->getWidth(), $this->gridSize - $x));
$height = max(1, $position->getHeight());
return GridPositionDTO::create($x, $y, $width, $height);
}🤖 Prompt for AI Agents
In @Modules/Core/Services/GridSnapperService.php around lines 25 - 33, In
GridSnapperService::snap the width calculation uses the original unclamped
$position->getX() instead of the clamped $x; update the $width expression to use
$x (i.e. max(1, min($position->getWidth(), $this->gridSize - $x))) so the
available width is computed from the clamped x; keep the existing min/max guards
and return statement using GridPositionDTO::create($x, $y, $width, $height).
| $this->fileRepository->expects($this->once()) | ||
| ->method('load') | ||
| ->with(1, 'test-template') | ||
| ->willReturn([ | ||
| [ | ||
| 'id' => 'block_1', | ||
| 'type' => 'company_header', | ||
| 'position' => ['x' => 0, 'y' => 0, 'width' => 6, 'height' => 4], | ||
| 'config' => [], | ||
| ], | ||
| ]); |
There was a problem hiding this comment.
Method name mismatch: load vs get.
The mock expects method load, but ReportTemplateFileRepository defines get. This test will fail.
Suggested fix
$this->fileRepository->expects($this->once())
- ->method('load')
+ ->method('get')
->with(1, 'test-template')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $this->fileRepository->expects($this->once()) | |
| ->method('load') | |
| ->with(1, 'test-template') | |
| ->willReturn([ | |
| [ | |
| 'id' => 'block_1', | |
| 'type' => 'company_header', | |
| 'position' => ['x' => 0, 'y' => 0, 'width' => 6, 'height' => 4], | |
| 'config' => [], | |
| ], | |
| ]); | |
| $this->fileRepository->expects($this->once()) | |
| ->method('get') | |
| ->with(1, 'test-template') | |
| ->willReturn([ | |
| [ | |
| 'id' => 'block_1', | |
| 'type' => 'company_header', | |
| 'position' => ['x' => 0, 'y' => 0, 'width' => 6, 'height' => 4], | |
| 'config' => [], | |
| ], | |
| ]); |
🤖 Prompt for AI Agents
In @Modules/Core/Tests/Unit/ReportTemplateServiceTest.php around lines 230 -
240, The test mocks file repository using method name load but the actual
repository defines get; update the mock on $this->fileRepository in
ReportTemplateServiceTest to expect and stub the get method (e.g.,
->method('get')->with(1, 'test-template')->willReturn(...)) so it matches
ReportTemplateFileRepository::get, or alternatively change the repository
reference used by the service to a mock that exposes load; ensure the unique
symbols referenced ($this->fileRepository, ReportTemplateFileRepository, get)
are aligned between test and implementation.
| .fi-user-menu .fi-icon { | ||
| @apply text-blue-700; | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Missing closing brace causes parse error.
Biome reports a parse error: "expected } but instead the file ends". This indicates a structural issue with mismatched braces that will prevent the CSS from compiling.
🔍 Debugging steps to identify the issue
Manually trace through all opening and closing braces in the file, paying special attention to:
- The
.fi-topbarblock (lines 19-38) with nested selectors - The
.fi-sidebarblock (lines 40-73) with multiple nested selectors - Any blocks that were modified during the Tailwind v4 migration
You can also use a CSS validator or linter to identify the exact location of the mismatch:
#!/bin/bash
# Description: Validate CSS syntax using multiple tools
echo "=== Checking CSS syntax with Biome ==="
npx @biomejs/biome check resources/css/filament/company/invoiceplane-blue.css
echo -e "\n=== Alternative: Format file to reveal structure issues ==="
npx prettier --parser css resources/css/filament/company/invoiceplane-blue.css --check🧰 Tools
🪛 Biome (2.1.2)
[error] 217-217: expected } but instead the file ends
the file ends here
(parse)
🤖 Prompt for AI Agents
In @resources/css/filament/company/invoiceplane-blue.css at line 217, The file
ends with a missing closing brace causing a parse error; inspect the open CSS
blocks (start with the .fi-topbar and .fi-sidebar selectors and any recently
modified Tailwind v4 blocks) and add the missing closing "}" to properly
terminate the innermost unclosed rule (ensure every "{" has a matching "}" for
nested rules), then re-run Biome/Prettier to confirm the syntax error is
resolved.
| @@ -1,7 +1,10 @@ | |||
| @import 'tailwindcss'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine its contents
head -100 resources/css/filament/company/invoiceplane.cssRepository: InvoicePlane/InvoicePlane-v2
Length of output: 1858
🏁 Script executed:
# Search for old Tailwind v3 important modifier syntax in CSS files
rg -n '@apply\s+!' --type=cssRepository: InvoicePlane/InvoicePlane-v2
Length of output: 484
🏁 Script executed:
# Also search for any other variations of the old syntax pattern
rg -n '!\w+' resources/css/filament/company/invoiceplane.cssRepository: InvoicePlane/InvoicePlane-v2
Length of output: 106
🌐 Web query:
Tailwind v4 important modifier breaking change syntax
💡 Result:
Short answer: In Tailwind v4 the important modifier moves from the start of the utility to the end of the class name — e.g. use bg-red-500! (and hover:bg-red-500!) instead of !bg-red-500 (or hover:!bg-red-500). The old prefix form is deprecated for v4; the postfix form is the new preferred syntax. [1][2][3]
Examples:
- v3 deprecated: class="!text-sm" or class="hover:!text-sm".
- v4 preferred: class="text-sm!" or class="hover:text-sm!".
Sources: Tailwind v4 upgrade guide and related discussions. [1][2][3]
Fix Tailwind v4 breaking change: important modifier must come after utility name.
Upgrading to Tailwind v4 requires moving the important modifier (!) from before the utility to after it.
Line 61 uses the old v3 syntax which will break:
@apply !bg-primary-700; /* ❌ v3 syntax */Must be updated to:
@apply bg-primary-700!; /* ✅ v4 syntax */Fix
.fi-sidebar-item.fi-active {
@apply text-white;
- @apply !bg-primary-700;
+ @apply bg-primary-700!;
@apply rounded-lg;
}🤖 Prompt for AI Agents
In @resources/css/filament/company/invoiceplane.css at line 1, Update the
Tailwind v4 important modifier usage in invoiceplane.css by moving the "!" from
before the utility name to after it for any @apply rules (e.g., change
occurrences like "@apply !bg-primary-700;" to the v4 form "@apply
bg-primary-700!;"); search for other @apply lines using the old "!utility"
pattern and convert them similarly so all important modifiers follow the utility
name.
nielsdrost7
left a comment
nielsdrost7
left a comment
There was a problem hiding this comment.
@copilot In
@Modules/Core/Filament/Admin/Resources/ReportBlocks/Tables/ReportBlocksTable.php:
- Around line 39-47: Replace the use of ->recordActions(...) with ->actions(...)
on the table definition and remove the unused $data parameter from the
DeleteAction callback; specifically change the ActionGroup block that currently
uses ReportBlocksTable::recordActions to use ->actions([...]) and update the
DeleteAction::make('delete')->action(function (ReportBlock $record, array $data)
{ ... }) to function (ReportBlock $record) {
app(ReportBlockService::class)->deleteReportBlock($record); } so the closure
signature matches usage and the record-level actions API is correct. - Around line 5-7: The file imports ActionGroup, DeleteAction, and EditAction
from Filament\Actions* which are page-level actions and will error for table
records; update the use statements to import these three symbols from
Filament\Tables\Actions (i.e., replace Filament\Actions\ActionGroup,
Filament\Actions\DeleteAction, Filament\Actions\EditAction with
Filament\Tables\Actions\ActionGroup, Filament\Tables\Actions\DeleteAction,
Filament\Tables\Actions\EditAction) so the table record actions in
ReportBlocksTable.php resolve correctly.
In @Modules/Core/Services/GridSnapperService.php:
- Around line 25-33: In GridSnapperService::snap the width calculation uses the
original unclamped $position->getX() instead of the clamped $x; update the
$width expression to use $x (i.e. max(1, min($position->getWidth(),
$this->gridSize - $x))) so the available width is computed from the clamped x;
keep the existing min/max guards and return statement using
GridPositionDTO::create($x, $y, $width, $height).
In @Modules/Core/Tests/Unit/ReportTemplateServiceTest.php:
- Around line 230-240: The test mocks file repository using method name load but
the actual repository defines get; update the mock on $this->fileRepository in
ReportTemplateServiceTest to expect and stub the get method (e.g.,
->method('get')->with(1, 'test-template')->willReturn(...)) so it matches
ReportTemplateFileRepository::get, or alternatively change the repository
reference used by the service to a mock that exposes load; ensure the unique
symbols referenced ($this->fileRepository, ReportTemplateFileRepository, get)
are aligned between test and implementation.
In @resources/css/filament/company/invoiceplane-blue.css:
- Line 217: The file ends with a missing closing brace causing a parse error;
inspect the open CSS blocks (start with the .fi-topbar and .fi-sidebar selectors
and any recently modified Tailwind v4 blocks) and add the missing closing "}" to
properly terminate the innermost unclosed rule (ensure every "{" has a matching
"}" for nested rules), then re-run Biome/Prettier to confirm the syntax error is
resolved.
In @resources/css/filament/company/invoiceplane.css:
- Line 1: Update the Tailwind v4 important modifier usage in invoiceplane.css by
moving the "!" from before the utility name to after it for any @apply rules
(e.g., change occurrences like "@apply !bg-primary-700;" to the v4 form "@apply
bg-primary-700!;"); search for other @apply lines using the old "!utility"
pattern and convert them similarly so all important modifiers follow the utility
name.
|
@nielsdrost7 I've opened a new pull request, #351, to work on those changes. Once the pull request is ready, I'll request review from you. |
…ks to fakes in Report Builder (#351) * Initial plan * Fix ReportBlocksTable imports, actions, GridSnapperService, test mock, and CSS syntax Co-authored-by: nielsdrost7 <[email protected]> * Refactor tests to use Storage::fake() instead of mocking repository Co-authored-by: nielsdrost7 <[email protected]> * Change actions to recordActions in ReportBlocksTable --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: nielsdrost7 <[email protected]>
Summary by CodeRabbit
New Features
Improvements
Removed
✏️ Tip: You can customize this high-level summary in your review settings.