[IP-145]: tabular reports — five company-panel report pages - #609
Closed
nielsdrost7 wants to merge 0 commit into
Closed
[IP-145]: tabular reports — five company-panel report pages#609nielsdrost7 wants to merge 0 commit into
nielsdrost7 wants to merge 0 commit into
Conversation
This was referenced Jul 13, 2026
nielsdrost7
changed the title
This was referenced Jul 18, 2026
5 tasks
Contributor
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughIntroduces five company-scoped Filament reports with shared date/client filters, tabular views, summaries, CSV export, navigation registration, translations, and feature tests covering filtering, aggregation, scoping, rendering, and export. ChangesCompany tabular reports
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CompanyUser
participant FilamentReportPage
participant EloquentQuery
participant CsvDownload
CompanyUser->>FilamentReportPage: Select date and client filters
FilamentReportPage->>EloquentQuery: Execute filtered report query
EloquentQuery-->>FilamentReportPage: Return report rows
CompanyUser->>FilamentReportPage: Trigger CSV export
FilamentReportPage->>CsvDownload: Stream headers and rows
CsvDownload-->>CompanyUser: Download CSV
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Modules/Core/Filament/Company/Pages/Reports/BaseTabularReportPage.php (1)
111-118: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider using
pluckinstead ofmapfor performance.When generating key-value options from the database, using
pluckdirectly on the query builder is more efficient than fetching full models and mapping them in memory.♻️ Proposed fix
public function getClientOptions(): array { return Relation::query() ->orderBy('company_name') - ->get(['id', 'company_name']) - ->map(fn (Relation $relation): array => ['id' => $relation->id, 'name' => $relation->company_name]) + ->pluck('company_name', 'id') + ->map(fn (string $name, int $id): array => ['id' => $id, 'name' => $name]) + ->values() ->all(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Filament/Company/Pages/Reports/BaseTabularReportPage.php` around lines 111 - 118, Update getClientOptions to use the query builder’s pluck operation instead of fetching Relation models with get and transforming them via map. Preserve the company_name ordering and return the same id-to-name option structure expected by callers.Modules/Core/Filament/Company/Pages/Reports/InvoicedByClientReport.php (1)
33-41: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid fetching all records into memory for the summary.
Calling
->get()on the full query loads all matching records into memory, which can lead to memory exhaustion and slow performance for companies with many clients. It is more efficient to calculate the summary values directly at the database level.♻️ Proposed fix
public function summaryLine(): string { - $rows = $this->reportQuery()->get(); + $query = $this->reportQuery(); + + $totalQuery = \Modules\Invoices\Models\Invoice::query() + ->whereBetween('invoiced_at', $this->dateRange()) + ->when($this->clientId, fn ($q) => $q->where('customer_id', $this->clientId)); return trans('ip.report_summary_invoiced_by_client', [ - 'clients' => $rows->count(), - 'total' => $this->money($rows->sum('invoiced_total')), + 'clients' => $query->count(), + 'total' => $this->money($totalQuery->sum('invoice_total')), ]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Filament/Company/Pages/Reports/InvoicedByClientReport.php` around lines 33 - 41, Update summaryLine() to compute the client count and invoiced_total sum through database-level aggregate queries instead of calling get() and materializing all rows. Preserve the existing translation keys and money formatting, using the reportQuery() builder while ensuring the count and sum operate on the full matching dataset.Modules/Core/Filament/Company/Pages/Reports/InvoicesPerClientReport.php (1)
33-41: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid fetching all records into memory for the summary.
Loading all rows into memory via
->get()just to compute the total invoice count is inefficient and can cause memory issues for large datasets. Compute the totals at the database level instead.♻️ Proposed fix
public function summaryLine(): string { - $rows = $this->reportQuery()->get(); + $query = $this->reportQuery(); + + $invoicesQuery = \Modules\Invoices\Models\Invoice::query() + ->whereBetween('invoiced_at', $this->dateRange()) + ->when($this->clientId, fn ($q) => $q->where('customer_id', $this->clientId)); return trans('ip.report_summary_invoices_per_client', [ - 'clients' => $rows->count(), - 'invoices' => (int) $rows->sum('invoices_count'), + 'clients' => $query->count(), + 'invoices' => $invoicesQuery->count(), ]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Filament/Company/Pages/Reports/InvoicesPerClientReport.php` around lines 33 - 41, Update summaryLine() to avoid reportQuery()->get() and calculate both the client count and total invoices_count in the database using aggregate queries. Preserve the existing translation key and summary values, ensuring clients reflects the row count and invoices remains an integer total.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Modules/Core/Filament/Company/Pages/Reports/BaseTabularReportPage.php`:
- Around line 111-118: Update getClientOptions to use the query builder’s pluck
operation instead of fetching Relation models with get and transforming them via
map. Preserve the company_name ordering and return the same id-to-name option
structure expected by callers.
In `@Modules/Core/Filament/Company/Pages/Reports/InvoicedByClientReport.php`:
- Around line 33-41: Update summaryLine() to compute the client count and
invoiced_total sum through database-level aggregate queries instead of calling
get() and materializing all rows. Preserve the existing translation keys and
money formatting, using the reportQuery() builder while ensuring the count and
sum operate on the full matching dataset.
In `@Modules/Core/Filament/Company/Pages/Reports/InvoicesPerClientReport.php`:
- Around line 33-41: Update summaryLine() to avoid reportQuery()->get() and
calculate both the client count and total invoices_count in the database using
aggregate queries. Preserve the existing translation key and summary values,
ensuring clients reflects the row count and invoices remains an integer total.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3eaf4be9-cd6a-429e-bf3b-2814ea042776
📒 Files selected for processing (10)
Modules/Core/Filament/Company/Pages/Reports/BaseTabularReportPage.phpModules/Core/Filament/Company/Pages/Reports/InvoicedByClientReport.phpModules/Core/Filament/Company/Pages/Reports/InvoicesPerClientReport.phpModules/Core/Filament/Company/Pages/Reports/InvoicingHistoryReport.phpModules/Core/Filament/Company/Pages/Reports/PaymentHistoryReport.phpModules/Core/Filament/Company/Pages/Reports/SalesByDateReport.phpModules/Core/Providers/CompanyPanelProvider.phpModules/Core/Tests/Feature/CompanyTabularReportsTest.phpModules/Core/resources/views/filament/company/pages/reports/tabular-report.blade.phpresources/lang/en/ip.php
4 tasks
nielsdrost7
force-pushed
the
feature/145-tabular-reports
branch
from
July 24, 2026 11:09
a1b9fc4 to
4eac422
Compare
nielsdrost7
force-pushed
the
feature/145-tabular-reports
branch
3 times, most recently
from
August 15, 2026 15:20
90bbcab to
20a0c07
Compare
nielsdrost7
force-pushed
the
feature/145-tabular-reports
branch
from
August 15, 2026 16:17
c2dfde0 to
6ec06b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Built on the test-infra PR #607 — merge that first. Until it merges, this diff also shows #607's single commit.
Implements Track B of epic #506 — the tabular reports module per naui95's spec in #145. Independent of the Report Builder (no shared code path).
What's in here
Five read-only Filament pages in the company panel under a Reports navigation group, each with a date-range filter (defaults to the current month), an optional client filter, a totals summary line under the table, and a CSV export of the filtered rows:
withCount/withSumAll queries are tenant-scoped by the
BelongsToCompanyglobal scope. Shared behavior lives inBaseTabularReportPage(filters, CSV streaming, access control: client_admin + elevated roles).Also ships the
AbstractCompanyPanelTestCase::testLivewire()fix (broken Livewire import +withSessionchained on the wrong object) — identical fix is on the report-builder branch (underdogg-forks#2); whichever merges second resolves cleanly.Testing
Full suite green: 314 tests, 1,033 assertions, 0 failures (develop baseline: 307). The three scenarios drafted in the issue body (in-range vs out-of-range payments, per-client sum shows 1500, cross-company invisibility) plus client filtering, paid-only sales-by-date, page smoke for all five, and the CSV download assertion.
Follow-up
The issue's "later enhancement" (print/PDF export of these reports through builder templates) stays out of scope, as specified in #145.
Summary by CodeRabbit
New Features
Bug Fixes