Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 9, 2026

Addresses feedback from #349 review: incorrect Filament action namespace usage, grid snapping calculation bug, test mock method mismatch, and Tailwind v4 syntax migration. Additionally refactors tests to use Storage fakes instead of mocks per project guidelines.

Changes

ReportBlocksTable.php

  • Import actions from Filament\Tables\Actions\* instead of Filament\Actions\* (page-level vs table-level)
  • Replace ->recordActions() with ->actions() for correct table API
  • Remove unused array $data parameter from DeleteAction closure

GridSnapperService.php

  • Use clamped $x in width calculation instead of unclamped $position->getX()
  • Prevents width overflow when position is snapped to grid boundary
// Before: used original X, could exceed grid
$width = max(1, min($position->getWidth(), $this->gridSize - $position->getX()));

// After: uses clamped X
$width = max(1, min($position->getWidth(), $this->gridSize - $x));

ReportTemplateServiceTest.php

  • Refactored from mocks to Storage::fake('report_templates') following project guideline "Prefer Fakes and Fixtures over Mocks"
  • Use real ReportTemplateFileRepository instance with faked storage layer
  • it_persists_blocks() now validates actual file creation and content using Storage assertions
  • it_loads_blocks() now creates test fixtures using Storage::put() instead of mocking return values
  • Provides better integration test coverage by testing real repository logic

invoiceplane.css

  • Migrate Tailwind v4 important modifier: !bg-primary-700bg-primary-700!

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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

Copilot AI changed the title [WIP] Update action corrections and test updates for report builder Fix Filament action imports, grid calculations, and test mocks in Report Builder Jan 9, 2026
Copilot AI requested a review from nielsdrost7 January 9, 2026 03:22
Copy link
Collaborator

@nielsdrost7 nielsdrost7 left a comment

Choose a reason for hiding this comment

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

@copilot I prefer faking over mocking.
Any changes required?

Copy link
Contributor Author

Copilot AI commented Jan 9, 2026

@copilot I prefer faking over mocking.
Any changes required?

No changes required - the code now uses Storage::fake('report_templates') instead of mocking the repository, following the project guideline "Prefer Fakes and Fixtures over Mocks".

Changes in commit eaa747f:

  • Replaced $this->createMock(ReportTemplateFileRepository::class) with new ReportTemplateFileRepository()
  • Added Storage::fake('report_templates') in setUp()
  • it_persists_blocks() now uses Storage::disk('report_templates')->assertExists() and validates actual file content
  • it_loads_blocks() now uses Storage::disk('report_templates')->put() to create test data instead of mocking get()

Both tests now interact with the real repository using Laravel's Storage fake, providing better integration test coverage.

Copilot AI changed the title Fix Filament action imports, grid calculations, and test mocks in Report Builder Fix Filament action imports, grid calculations, and refactor test mocks to fakes in Report Builder Jan 9, 2026
Copilot AI requested a review from nielsdrost7 January 9, 2026 03:47
@nielsdrost7 nielsdrost7 marked this pull request as ready for review January 9, 2026 04:05
@nielsdrost7 nielsdrost7 merged commit 4bf32e2 into feature/98-report-builder-only Jan 9, 2026
1 check passed
@nielsdrost7 nielsdrost7 deleted the copilot/sub-pr-349 branch January 9, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants