Skip to content

Conversation

@nielsdrost7
Copy link
Collaborator

@nielsdrost7 nielsdrost7 commented Oct 27, 2025

Summary by CodeRabbit

  • New Features

    • Added multi-format export capability (CSV and Excel) across all data modules.
    • Each export option includes both current and legacy versions for compatibility.
    • Exports now process asynchronously with completion notifications.
    • Export actions integrated into all list pages for easy access.
  • Documentation

    • Added comprehensive project guidelines and export system documentation.
  • Dependencies

    • Added maatwebsite/excel library for export functionality.

coderabbitai[bot]

This comment was marked as outdated.

…o feature/85-core-export-clients-invoices-expenses-payments-etc
@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 13, 2025
nielsdrost7 and others added 3 commits November 13, 2025 06:19
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@nielsdrost7 nielsdrost7 marked this pull request as draft November 13, 2025 05:21
@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 13, 2025
nielsdrost7 and others added 2 commits November 13, 2025 06:23
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 13, 2025
Copilot AI and others added 2 commits November 13, 2025 07:54
…ples and i18n support (#128)

* Initial plan

* Create Filament Exporters for all modules

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Update all List Pages to use Filament Export Actions

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Add Filament exports table migration

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Add comprehensive export refactoring documentation

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Fix exporters: add translations, handle enums/dates/accessors properly, implement DRY with BaseExporter

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Update documentation to reflect code improvements and explain test requirements

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…guidelines (#129)

* Initial plan

* Refactor Expenses, Invoices, and Quotes export tests to use Queue fakes

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Complete export test refactoring for all modules and remove exports migration

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Add export architecture documentation explaining temporary nature of export records

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Clean up unused imports in ExpensesExportImportTest

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Update README and documentation files with queue requirements for exports

- Added queue worker requirement to main README.md features and installation
- Updated .github/copilot-instructions.md with Queue System in tech stack
- Added Export System Rules section to copilot-instructions.md
- Enhanced Modules/Core/Filament/Exporters/README.md with detailed queue configuration
- Included local development and production setup instructions
- Added Supervisor configuration example for production environments

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Create comprehensive .junie/guidelines.md for AI agent configuration

Created detailed guidelines document for AI agents (like Junie) with:
- Complete project architecture overview
- SOLID principles and code quality standards
- DTO/Transformer patterns with examples
- Testing standards and export testing rules
- Database/Model conventions
- Filament resource best practices
- Export system architecture and queue configuration
- Security and multi-tenancy guidelines
- Development workflow commands
- Performance optimization strategies
- Common pitfalls to avoid
- Code review checklist
- Documentation references and learning resources

This provides maximum information for AI agents to understand the codebase structure, conventions, and performance requirements.

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
@nielsdrost7
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 13, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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: 17

♻️ Duplicate comments (14)
Modules/Clients/Exports/ContactsExport.php (1)

36-46: LGTM! Correct enum handling and null-safe implementation.

The export class correctly handles enum values by calling ->label() (line 40) and uses null-safe operators throughout. The implementation follows best practices with translated headings and proper data mapping.

Note: The previous review comment about enum handling has already been addressed—line 40 correctly uses $row->relation?->relation_type?->label() ?? ''.

Modules/Expenses/Filament/Company/Resources/Expenses/Pages/ListExpenses.php (1)

31-55: Duplicate pattern across modules.

This export action configuration is identical to the patterns in ListInvoices and ListTasks. See the refactoring suggestion in ListInvoices.php for centralizing this repeated code.

Modules/Payments/Models/Payment.php (1)

46-49: Update docblock type hint for payment_method.

The $casts array now maps payment_method to PaymentMethod::class (Line 46), but the docblock at Line 27 still declares it as string. Update the docblock to reflect the enum type.

Apply this diff to fix the docblock:

- * @property string       $payment_method
+ * @property PaymentMethod $payment_method
Modules/Projects/Filament/Company/Resources/Projects/Pages/ListProjects.php (1)

31-55: Export action group follows the established pattern.

The export action group implementation is consistent with other list pages in this PR. See the comment on ListRelations.php regarding potential extraction of this repeated pattern.

Modules/Invoices/Services/InvoiceExportService.php (1)

14-23: Guard company context and eager-load customer to prevent N+1.

The method lacks a guard for missing company context and will trigger N+1 queries when loading customers during export.

Apply this diff:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $invoices    = Invoice::query()->where('company_id', $companyId)->get();
+    $companyId = session('current_company_id');
+    if (!$companyId) {
+        abort(400, 'Missing current company context');
+    }
+    $invoices = Invoice::query()
+        ->with(['customer'])
+        ->where('company_id', $companyId)
+        ->get();
     $fileName    = 'invoices-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
     $version     = config('ip.export_version', 2);
     $exportClass = $version === 1 ? InvoicesLegacyExport::class : InvoicesExport::class;
-
-    return Excel::download(new $exportClass($invoices), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    $writer = $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX;
+    return Excel::download(new $exportClass($invoices), $fileName, $writer);
 }
Modules/Products/Services/ProductExportService.php (1)

14-23: Guard company context and avoid N+1 with eager loads.

The method lacks company context guard and will trigger N+1 queries for product relationships.

Apply this diff:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $products    = Product::query()->where('company_id', $companyId)->get();
+    $companyId = session('current_company_id');
+    if (!$companyId) {
+        abort(400, 'Missing current company context');
+    }
+    $products = Product::query()
+        ->with(['productCategory', 'productUnit'])
+        ->where('company_id', $companyId)
+        ->get();
     $fileName    = 'products-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
     $version     = config('ip.export_version', 2);
     $exportClass = $version === 1 ? ProductsLegacyExport::class : ProductsExport::class;
-
-    return Excel::download(new $exportClass($products), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    $writer = $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX;
+    return Excel::download(new $exportClass($products), $fileName, $writer);
 }
Modules/Projects/Exports/ProjectsExport.php (1)

40-40: Fix null dereference on project_status.

Accessing label() without the nullsafe operator will cause a fatal error when project_status is null.

Apply this diff:

-            $row->project_status->label() ?? '',
+            $row->project_status?->label() ?? '',
Modules/Payments/Services/PaymentExportService.php (1)

30-38: Validate session and limit memory usage.

The method lacks company context validation and loads all payments into memory, which can cause OOM errors for large datasets.

Apply this diff to add session validation:

 public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
 {
     $companyId   = session('current_company_id');
+    
+    if (!$companyId) {
+        abort(403, 'No company context available');
+    }
+    
     $payments    = Payment::query()->where('company_id', $companyId)->get();

For memory concerns, consider chunking or limiting:

-    $payments    = Payment::query()->where('company_id', $companyId)->get();
+    $payments    = Payment::query()
+        ->where('company_id', $companyId)
+        ->orderBy('paid_at', 'desc')
+        ->limit(10000)
+        ->get();
Modules/Projects/Services/TaskExportService.php (2)

14-23: Refactor to eliminate duplication and add safeguards.

This method duplicates logic from exportWithVersion and lacks session validation. Additionally, loading all tasks via ->get() can exhaust memory.

Apply this diff:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $tasks       = Task::query()->where('company_id', $companyId)->get();
-    $fileName    = 'tasks-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
-    $version     = config('ip.export_version', 2);
-    $exportClass = $version === 1 ? TasksLegacyExport::class : TasksExport::class;
-
-    return Excel::download(new $exportClass($tasks), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    return $this->exportWithVersion($format, config('ip.export_version', 2));
 }

25-33: Validate session and address memory concerns.

Missing company context validation and unbounded query can cause issues.

Apply this diff:

 public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
 {
     $companyId   = session('current_company_id');
+    
+    if (!$companyId) {
+        abort(403, 'No company context available');
+    }
+    
     $tasks       = Task::query()->where('company_id', $companyId)->get();

Consider limiting or chunking:

-    $tasks       = Task::query()->where('company_id', $companyId)->get();
+    $tasks       = Task::query()
+        ->where('company_id', $companyId)
+        ->orderBy('created_at', 'desc')
+        ->limit(10000)
+        ->get();
Modules/Expenses/Services/ExpenseExportService.php (2)

16-17: Validate session company ID before querying.

The session current_company_id is accessed without validation. If it's null or missing, the query will fail or return incorrect results.

Add validation:

-$companyId   = session('current_company_id');
+$companyId = session('current_company_id');
+if (!$companyId) {
+    abort(403, 'No company selected');
+}
 $expenses    = Expense::query()->where('company_id', $companyId)->get();

17-17: Unbounded query may cause memory exhaustion.

Loading all expenses with ->get() can exhaust memory for companies with large datasets. Consider using cursor() or processing in chunks, or rely on Filament's queued export mechanism which handles pagination automatically.

Modules/Payments/Feature/Modules/PaymentsExportImportTest.php (1)

33-36: Use payment_amount to match the Payments schema

Line 33: The Payments table and factory expose payment_amount; there is no amount column. Posting 'amount' in the column map (and in the special-character factory override on Line 106) will leave the export job without a matching field and will break record creation. Swap every occurrence of 'amount' for 'payment_amount', including the fixture override.

-                    'amount' => ['isEnabled' => true, 'label' => 'Payment Amount'],
+                    'payment_amount' => ['isEnabled' => true, 'label' => 'Payment Amount'],
@@
-        $payment = Payment::factory()->for($this->company)->create([
-            'amount' => 123.45,
+        $payment = Payment::factory()->for($this->company)->create([
+            'payment_amount' => 123.45,

Also applies to: 60-61, 85-86, 115-116, 141-142, 167-168, 106-107

Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php (1)

32-37: Mount the versioned export actions defined on ListInvoices

Line 32: ListInvoices registers exportCsvV2/exportCsvV1 and exportExcelV2/exportExcelV1; the unversioned exportCsv/exportExcel methods no longer exist after this refactor. Mounting the old names will throw ActionNotFound. Switch these calls (here and at Lines 59, 85, and 114) to the corresponding versioned actions.

-            ->callAction('exportCsv', data: [
+            ->callAction('exportCsvV2', data: [
@@
-            ->callAction('exportExcel', data: [
+            ->callAction('exportExcelV2', data: [
@@
-            ->callAction('exportExcel', data: [
+            ->callAction('exportExcelV2', data: [
@@
-            ->callAction('exportExcel', data: [
+            ->callAction('exportExcelV2', data: [

Also applies to: 59-63, 85-88, 114-118

🧹 Nitpick comments (15)
Modules/Clients/Models/Relation.php (1)

175-178: Remove commented-out code.

Commented-out code should be removed to keep the codebase clean. If this accessor is needed in the future, it can be retrieved from git history. Additionally, the commented code contains a typo (primary_ontact should be primary_contact).

Apply this diff to remove the dead code:

-    /*public function getPrimaryContactAttribute(): string
-    {
-        return mb_trim($this->primary_ontact?->first_name . ' ' . $this->primary_contact?->last_name);
-    }*/
Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php (1)

34-58: Consider extracting labels to translation keys and reducing duplication.

The export action group contains four similar actions with hardcoded English labels. For consistency with the broader i18n approach in the codebase (e.g., trans('ip.export_completed')), consider extracting these labels to translation keys.

Additionally, the four export actions follow a repetitive pattern that could be abstracted into a helper method or trait to reduce duplication across modules.

Example refactor for labels:

 ActionGroup::make([
     ExportAction::make('exportCsvV2')
-        ->label('Export as CSV (v2)')
+        ->label(trans('ip.export_csv_v2'))
         ->icon('heroicon-o-document-text')
         ->exporter(TaskExporter::class)
         ->formats([ExportFormat::Csv]),
     ExportAction::make('exportCsvV1')
-        ->label('Export as CSV (v1, Legacy)')
+        ->label(trans('ip.export_csv_v1_legacy'))
         ->icon('heroicon-o-document-text')
         ->exporter(TaskLegacyExporter::class)
         ->formats([ExportFormat::Csv]),
     // ... similar for Excel actions
 ])
-    ->label('Export')
+    ->label(trans('ip.export'))

Note: This same pattern appears across multiple modules (Invoices, Expenses, Payments, etc.), so a centralized helper would improve maintainability.

Modules/Invoices/Filament/Company/Resources/Invoices/Pages/ListInvoices.php (1)

31-55: Duplicate pattern: Consider centralizing export action configuration.

This export action group follows the same pattern as other modules (Tasks, Expenses, etc.), with the same hardcoded labels and repetitive structure. This duplication increases maintenance burden when changes are needed across all modules.

Consider creating a shared trait or helper method in a base class (e.g., Modules\Core\Filament\Traits\HasExportActions) that generates these standard export actions:

// Modules/Core/Filament/Traits/HasExportActions.php
trait HasExportActions
{
    protected function getExportActionGroup(
        string $exporterClass, 
        string $legacyExporterClass
    ): ActionGroup {
        return ActionGroup::make([
            ExportAction::make('exportCsvV2')
                ->label(trans('ip.export_csv_v2'))
                ->icon('heroicon-o-document-text')
                ->exporter($exporterClass)
                ->formats([ExportFormat::Csv]),
            // ... other actions
        ])
            ->label(trans('ip.export'))
            ->icon(Heroicon::OutlinedFolderArrowDown)
            ->button();
    }
}

Then in ListInvoices:

use Modules\Core\Filament\Traits\HasExportActions;

class ListInvoices extends ListRecords
{
    use HasExportActions;
    
    protected function getHeaderActions(): array
    {
        return [
            CreateAction::make()->...,
            $this->getExportActionGroup(
                InvoiceExporter::class, 
                InvoiceLegacyExporter::class
            ),
        ];
    }
}
Modules/Payments/Exports/PaymentsLegacyExport.php (1)

34-42: Consider formatting dates and currency values in the export.

The map() method exports raw values for paid_at (timestamp) and payment_amount (numeric). For better user experience in CSV/Excel exports, consider formatting these fields:

  • paid_at: Format as a human-readable date (e.g., $row->paid_at?->format('Y-m-d'))
  • payment_amount: Format with currency symbols or decimal places

Example enhancement:

 public function map($row): array
 {
     return [
         $row->payment_method?->label() ?? '',
         $row->payment_status?->label() ?? '',
-        $row->payment_amount,
-        $row->paid_at,
+        number_format($row->payment_amount, 2),
+        $row->paid_at?->format(config('ip.date_format', 'Y-m-d')) ?? '',
     ];
 }

Note: If there's a centralized date/currency formatting utility in the codebase, use that for consistency.

Modules/Clients/Filament/Exporters/ContactExporter.php (1)

22-24: Remove redundant formatStateUsing closure.

The formatStateUsing closure on Line 24 simply returns $record->full_name, which is already the default behavior for ExportColumn::make('full_name'). The closure is redundant.

Apply this diff to simplify:

         ExportColumn::make('full_name')
-            ->label(trans('ip.contact_name'))
-            ->formatStateUsing(fn ($state, Contact $record) => $record->full_name),
+            ->label(trans('ip.contact_name')),
Modules/Projects/Exports/TasksLegacyExport.php (1)

36-46: Consider formatting date and price fields for consistency.

Lines 41-42 export due_at and task_price without formatting. For better readability and consistency in the exported file, consider applying date and number formatting.

Apply this diff to add formatting:

     public function map($row): array
     {
         return [
             $row->task_status?->label() ?? '',
             $row->task_name,
-            $row->due_at,
-            $row->task_price,
+            $row->due_at?->format('Y-m-d') ?? '',
+            number_format($row->task_price, 2, '.', ''),
             $row->project?->project_name ?? '',
             $row->relation?->trading_name ?? $row->relation?->company_name ?? '',
         ];
     }
Modules/Products/Filament/Company/Resources/Products/Pages/ListProducts.php (1)

31-55: Use consistent icon reference style.

Lines 34, 39, 44, 49 use string literals for icons ('heroicon-o-document-text', 'heroicon-o-document'), while Line 54 uses the Heroicon::OutlinedFolderArrowDown constant. Mixing styles is inconsistent and may lead to confusion.

Apply this diff to use string literals consistently:

         ])
             ->label('Export')
-            ->icon(Heroicon::OutlinedFolderArrowDown)
+            ->icon('heroicon-o-folder-arrow-down')
             ->button(),

Or alternatively, use constants for all icon references throughout the action group for type safety.

Modules/Clients/Filament/Company/Resources/Contacts/Pages/ListContacts.php (1)

31-55: Use consistent icon reference style.

Lines 34, 39, 44, 49 use string literals for icons ('heroicon-o-document-text', 'heroicon-o-document'), while Line 54 uses the Heroicon::OutlinedFolderArrowDown constant. This is inconsistent with the other icon references in the same action group.

Apply this diff to use string literals consistently:

         ])
             ->label('Export')
-            ->icon(Heroicon::OutlinedFolderArrowDown)
+            ->icon('heroicon-o-folder-arrow-down')
             ->button(),

Or alternatively, use constants for all icon references throughout the action group for type safety.

Modules/Products/Filament/Exporters/ProductLegacyExporter.php (1)

20-21: Consider formatting the price column for consistency.

The price column is exported without formatting. For better readability and consistency in exported files, consider applying number formatting to monetary values.

Apply this diff to add price formatting:

         ExportColumn::make('price')
-            ->label(trans('ip.product_price')),
+            ->label(trans('ip.product_price'))
+            ->formatStateUsing(fn ($state) => number_format($state, 2, '.', '')),
Modules/Clients/Filament/Company/Resources/Relations/Pages/ListRelations.php (1)

32-56: Consider extracting the export action group to reduce duplication.

The export action group pattern (lines 32-56) is identical across multiple list pages (ListRelations, ListProjects, and likely others in this PR). While functional, this duplication increases maintenance burden.

Consider creating a trait or helper method to generate the export action group:

trait HasExportActions
{
    protected function makeExportActionGroup(string $exporterClass, string $legacyExporterClass): ActionGroup
    {
        return ActionGroup::make([
            ExportAction::make('exportCsvV2')
                ->label('Export as CSV (v2)')
                ->icon('heroicon-o-document-text')
                ->exporter($exporterClass)
                ->formats([ExportFormat::Csv]),
            ExportAction::make('exportCsvV1')
                ->label('Export as CSV (v1, Legacy)')
                ->icon('heroicon-o-document-text')
                ->exporter($legacyExporterClass)
                ->formats([ExportFormat::Csv]),
            ExportAction::make('exportExcelV2')
                ->label('Export as Excel (v2)')
                ->icon('heroicon-o-document')
                ->exporter($exporterClass)
                ->formats([ExportFormat::Xlsx]),
            ExportAction::make('exportExcelV1')
                ->label('Export as Excel (v1, Legacy)')
                ->icon('heroicon-o-document')
                ->exporter($legacyExporterClass)
                ->formats([ExportFormat::Xlsx]),
        ])
            ->label('Export')
            ->icon(Heroicon::OutlinedFolderArrowDown)
            ->button();
    }
}

Then use it as:

$this->makeExportActionGroup(RelationExporter::class, RelationLegacyExporter::class)
Modules/Clients/Filament/Exporters/ContactLegacyExporter.php (1)

22-24: Remove redundant formatStateUsing closure.

The closure simply returns the attribute value without transformation.

Simplify:

-            ExportColumn::make('full_name')
-                ->label(trans('ip.contact_name'))
-                ->formatStateUsing(fn ($state, Contact $record) => $record->full_name),
+            ExportColumn::make('full_name')
+                ->label(trans('ip.contact_name')),
Modules/Clients/Exports/RelationsExport.php (1)

53-54: Remove redundant null coalescing.

Using ?? null is redundant since the left-hand expression is already nullable.

Simplify:

-            $row->email ?? null,
-            $row->phone ?? null,
+            $row->email,
+            $row->phone,
Modules/Clients/Services/RelationExportService.php (1)

14-33: Eliminate duplication between export methods.

The export() and exportWithVersion() methods contain nearly identical logic, differing only in how the version is determined (config vs parameter). As previously suggested, refactor export() to delegate to exportWithVersion().

Apply this diff to remove the duplication:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $relations   = Relation::query()->where('company_id', $companyId)->get();
-    $fileName    = 'relations-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
-    $version     = config('ip.export_version', 2);
-    $exportClass = $version === 1 ? RelationsLegacyExport::class : RelationsExport::class;
-
-    return Excel::download(new $exportClass($relations), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    return $this->exportWithVersion($format, config('ip.export_version', 2));
 }
Modules/Projects/Filament/Exporters/TaskLegacyExporter.php (1)

1-39: LGTM! Implementation is correct.

The legacy task exporter is properly implemented with correct null-safe navigation and formatting. Note that this exporter appears identical to TaskExporter - if the legacy and modern exporters continue to share the same column definitions, consider extracting a shared column configuration to reduce duplication.

Modules/Expenses/Services/ExpenseExportService.php (1)

14-33: Consolidate duplicate export logic.

Both export() and exportWithVersion() share identical logic except for the version source. Consider extracting the common logic to reduce duplication.

Apply this refactor:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $expenses    = Expense::query()->where('company_id', $companyId)->get();
-    $fileName    = 'expenses-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
-    $version     = config('ip.export_version', 2);
-    $exportClass = $version === 1 ? ExpensesLegacyExport::class : ExpensesExport::class;
-
-    return Excel::download(new $exportClass($expenses), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    return $this->exportWithVersion($format, config('ip.export_version', 2));
 }

 public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
 {
     $companyId   = session('current_company_id');
     $expenses    = Expense::query()->where('company_id', $companyId)->get();
     $fileName    = 'expenses-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
     $exportClass = $version === 1 ? ExpensesLegacyExport::class : ExpensesExport::class;

     return Excel::download(new $exportClass($expenses), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1ff6b3 and 5e9cfe9.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (77)
  • .github/EXPORT-REFACTORING.md (1 hunks)
  • .github/copilot-instructions.md (3 hunks)
  • .junie/guidelines.md (1 hunks)
  • Modules/Clients/Exports/ContactsExport.php (1 hunks)
  • Modules/Clients/Exports/ContactsLegacyExport.php (1 hunks)
  • Modules/Clients/Exports/RelationsExport.php (1 hunks)
  • Modules/Clients/Exports/RelationsLegacyExport.php (1 hunks)
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php (2 hunks)
  • Modules/Clients/Feature/Modules/RelationsExportImportTest.php (1 hunks)
  • Modules/Clients/Filament/Company/Resources/Contacts/Pages/ListContacts.php (2 hunks)
  • Modules/Clients/Filament/Company/Resources/Relations/Pages/ListRelations.php (2 hunks)
  • Modules/Clients/Filament/Exporters/ContactExporter.php (1 hunks)
  • Modules/Clients/Filament/Exporters/ContactLegacyExporter.php (1 hunks)
  • Modules/Clients/Filament/Exporters/RelationExporter.php (1 hunks)
  • Modules/Clients/Filament/Exporters/RelationLegacyExporter.php (1 hunks)
  • Modules/Clients/Models/Relation.php (1 hunks)
  • Modules/Clients/Services/ContactExportService.php (1 hunks)
  • Modules/Clients/Services/RelationExportService.php (1 hunks)
  • Modules/Core/Filament/Exporters/BaseExporter.php (1 hunks)
  • Modules/Core/Filament/Exporters/README.md (1 hunks)
  • Modules/Expenses/Exports/ExpensesExport.php (1 hunks)
  • Modules/Expenses/Exports/ExpensesLegacyExport.php (1 hunks)
  • Modules/Expenses/Feature/Modules/ExpensesExportImportTest.php (1 hunks)
  • Modules/Expenses/Filament/Company/Resources/Expenses/Pages/ListExpenses.php (2 hunks)
  • Modules/Expenses/Filament/Exporters/ExpenseExporter.php (1 hunks)
  • Modules/Expenses/Filament/Exporters/ExpenseLegacyExporter.php (1 hunks)
  • Modules/Expenses/Services/ExpenseExportService.php (1 hunks)
  • Modules/Invoices/Exports/InvoicesExport.php (1 hunks)
  • Modules/Invoices/Exports/InvoicesLegacyExport.php (1 hunks)
  • Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php (2 hunks)
  • Modules/Invoices/Filament/Company/Resources/Invoices/Pages/ListInvoices.php (2 hunks)
  • Modules/Invoices/Filament/Exporters/InvoiceExporter.php (1 hunks)
  • Modules/Invoices/Filament/Exporters/InvoiceLegacyExporter.php (1 hunks)
  • Modules/Invoices/Services/InvoiceExportService.php (1 hunks)
  • Modules/Payments/Exports/PaymentsExport.php (1 hunks)
  • Modules/Payments/Exports/PaymentsLegacyExport.php (1 hunks)
  • Modules/Payments/Feature/Modules/PaymentsExportImportTest.php (2 hunks)
  • Modules/Payments/Filament/Company/Resources/Payments/Pages/ListPayments.php (2 hunks)
  • Modules/Payments/Filament/Company/Resources/Payments/Tables/PaymentsTable.php (1 hunks)
  • Modules/Payments/Filament/Exporters/PaymentExporter.php (1 hunks)
  • Modules/Payments/Filament/Exporters/PaymentLegacyExporter.php (1 hunks)
  • Modules/Payments/Models/Payment.php (2 hunks)
  • Modules/Payments/Services/PaymentExportService.php (1 hunks)
  • Modules/Products/Exports/ProductsExport.php (1 hunks)
  • Modules/Products/Exports/ProductsLegacyExport.php (1 hunks)
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php (2 hunks)
  • Modules/Products/Filament/Company/Resources/Products/Pages/ListProducts.php (2 hunks)
  • Modules/Products/Filament/Exporters/ProductExporter.php (1 hunks)
  • Modules/Products/Filament/Exporters/ProductLegacyExporter.php (1 hunks)
  • Modules/Products/Services/ProductExportService.php (1 hunks)
  • Modules/Projects/Exports/ProjectsExport.php (1 hunks)
  • Modules/Projects/Exports/ProjectsLegacyExport.php (1 hunks)
  • Modules/Projects/Exports/TasksExport.php (1 hunks)
  • Modules/Projects/Exports/TasksLegacyExport.php (1 hunks)
  • Modules/Projects/Feature/Modules/ProjectsExportImportTest.php (1 hunks)
  • Modules/Projects/Feature/Modules/TasksExportImportTest.php (1 hunks)
  • Modules/Projects/Filament/Company/Resources/Projects/Pages/ListProjects.php (2 hunks)
  • Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php (2 hunks)
  • Modules/Projects/Filament/Company/Resources/Tasks/Tables/TasksTable.php (1 hunks)
  • Modules/Projects/Filament/Exporters/ProjectExporter.php (1 hunks)
  • Modules/Projects/Filament/Exporters/ProjectLegacyExporter.php (1 hunks)
  • Modules/Projects/Filament/Exporters/TaskExporter.php (1 hunks)
  • Modules/Projects/Filament/Exporters/TaskLegacyExporter.php (1 hunks)
  • Modules/Projects/Services/ProjectExportService.php (1 hunks)
  • Modules/Projects/Services/TaskExportService.php (1 hunks)
  • Modules/Quotes/Exports/QuotesExport.php (1 hunks)
  • Modules/Quotes/Exports/QuotesLegacyExport.php (1 hunks)
  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php (2 hunks)
  • Modules/Quotes/Filament/Company/Resources/Quotes/Pages/ListQuotes.php (2 hunks)
  • Modules/Quotes/Filament/Exporters/QuoteExporter.php (1 hunks)
  • Modules/Quotes/Filament/Exporters/QuoteLegacyExporter.php (1 hunks)
  • Modules/Quotes/Services/QuoteExportService.php (1 hunks)
  • README.md (2 hunks)
  • composer.json (1 hunks)
  • config/ip.php (2 hunks)
  • phpstan-baseline.neon (1 hunks)
  • resources/lang/en/ip.php (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{app,Modules}/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{app,Modules}/**/*.php: Prefer early returns for readability
Catch Error, ErrorException, and Throwable separately

Files:

  • Modules/Payments/Exports/PaymentsLegacyExport.php
  • Modules/Clients/Services/ContactExportService.php
  • Modules/Expenses/Exports/ExpensesLegacyExport.php
  • Modules/Projects/Services/ProjectExportService.php
  • Modules/Expenses/Filament/Exporters/ExpenseLegacyExporter.php
  • Modules/Quotes/Exports/QuotesLegacyExport.php
  • Modules/Products/Filament/Exporters/ProductLegacyExporter.php
  • Modules/Products/Filament/Company/Resources/Products/Pages/ListProducts.php
  • Modules/Invoices/Services/InvoiceExportService.php
  • Modules/Projects/Filament/Exporters/TaskLegacyExporter.php
  • Modules/Invoices/Exports/InvoicesExport.php
  • Modules/Clients/Filament/Exporters/RelationExporter.php
  • Modules/Projects/Filament/Exporters/TaskExporter.php
  • Modules/Expenses/Services/ExpenseExportService.php
  • Modules/Clients/Filament/Exporters/RelationLegacyExporter.php
  • Modules/Projects/Services/TaskExportService.php
  • Modules/Projects/Exports/ProjectsExport.php
  • Modules/Payments/Filament/Exporters/PaymentExporter.php
  • Modules/Clients/Services/RelationExportService.php
  • Modules/Projects/Feature/Modules/ProjectsExportImportTest.php
  • Modules/Payments/Exports/PaymentsExport.php
  • Modules/Projects/Exports/ProjectsLegacyExport.php
  • Modules/Clients/Feature/Modules/RelationsExportImportTest.php
  • Modules/Products/Filament/Exporters/ProductExporter.php
  • Modules/Quotes/Services/QuoteExportService.php
  • Modules/Clients/Models/Relation.php
  • Modules/Projects/Feature/Modules/TasksExportImportTest.php
  • Modules/Clients/Exports/ContactsExport.php
  • Modules/Invoices/Filament/Exporters/InvoiceLegacyExporter.php
  • Modules/Clients/Filament/Company/Resources/Contacts/Pages/ListContacts.php
  • Modules/Projects/Filament/Exporters/ProjectExporter.php
  • Modules/Invoices/Filament/Exporters/InvoiceExporter.php
  • Modules/Quotes/Filament/Exporters/QuoteLegacyExporter.php
  • Modules/Quotes/Filament/Exporters/QuoteExporter.php
  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • Modules/Payments/Filament/Company/Resources/Payments/Tables/PaymentsTable.php
  • Modules/Projects/Exports/TasksLegacyExport.php
  • Modules/Clients/Exports/ContactsLegacyExport.php
  • Modules/Payments/Filament/Exporters/PaymentLegacyExporter.php
  • Modules/Clients/Filament/Exporters/ContactLegacyExporter.php
  • Modules/Products/Services/ProductExportService.php
  • Modules/Projects/Exports/TasksExport.php
  • Modules/Expenses/Exports/ExpensesExport.php
  • Modules/Projects/Filament/Company/Resources/Projects/Pages/ListProjects.php
  • Modules/Payments/Services/PaymentExportService.php
  • Modules/Core/Filament/Exporters/BaseExporter.php
  • Modules/Quotes/Filament/Company/Resources/Quotes/Pages/ListQuotes.php
  • Modules/Projects/Filament/Exporters/ProjectLegacyExporter.php
  • Modules/Payments/Filament/Company/Resources/Payments/Pages/ListPayments.php
  • Modules/Expenses/Filament/Exporters/ExpenseExporter.php
  • Modules/Clients/Filament/Exporters/ContactExporter.php
  • Modules/Clients/Exports/RelationsLegacyExport.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Tables/TasksTable.php
  • Modules/Payments/Models/Payment.php
  • Modules/Expenses/Filament/Company/Resources/Expenses/Pages/ListExpenses.php
  • Modules/Invoices/Exports/InvoicesLegacyExport.php
  • Modules/Clients/Exports/RelationsExport.php
  • Modules/Expenses/Feature/Modules/ExpensesExportImportTest.php
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php
  • Modules/Invoices/Filament/Company/Resources/Invoices/Pages/ListInvoices.php
  • Modules/Clients/Filament/Company/Resources/Relations/Pages/ListRelations.php
  • Modules/Products/Exports/ProductsExport.php
  • Modules/Payments/Feature/Modules/PaymentsExportImportTest.php
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php
  • Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php
  • Modules/Quotes/Exports/QuotesExport.php
  • Modules/Products/Exports/ProductsLegacyExport.php
Modules/**/Services/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Modules/**/Services/**/*.php: All DTO transformations must go through Transformers
Services must not build DTOs manually; use Transformers directly
All transformations must go through Transformers (service layer)

Files:

  • Modules/Clients/Services/ContactExportService.php
  • Modules/Projects/Services/ProjectExportService.php
  • Modules/Invoices/Services/InvoiceExportService.php
  • Modules/Expenses/Services/ExpenseExportService.php
  • Modules/Projects/Services/TaskExportService.php
  • Modules/Clients/Services/RelationExportService.php
  • Modules/Quotes/Services/QuoteExportService.php
  • Modules/Products/Services/ProductExportService.php
  • Modules/Payments/Services/PaymentExportService.php
{app,Modules}/**/{Http/Controllers,Services,Jobs}/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{app,Modules}/**/{Http/Controllers,Services,Jobs}/**/*.php: All API requests must go through the Advanced API Client
No direct API calls in controllers, services, or jobs
Use Laravel’s HTTP client instead of curl or Guzzle
Log API responses and errors separately for debugging
Use repository methods for upserts; do not call updateOrCreate directly

Files:

  • Modules/Clients/Services/ContactExportService.php
  • Modules/Projects/Services/ProjectExportService.php
  • Modules/Invoices/Services/InvoiceExportService.php
  • Modules/Expenses/Services/ExpenseExportService.php
  • Modules/Projects/Services/TaskExportService.php
  • Modules/Clients/Services/RelationExportService.php
  • Modules/Quotes/Services/QuoteExportService.php
  • Modules/Products/Services/ProductExportService.php
  • Modules/Payments/Services/PaymentExportService.php
{app,Modules}/**/Filament/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{app,Modules}/**/Filament/**/*.php: Filament resources must respect proper panel separation and namespaces
Add docblocks above form(), table(), and getRelations() with relationships/fields
Preserve the exact method signatures for Filament resource methods
Use the correct Action::make() syntax with fluent methods in Filament
Do not display raw created_at or updated_at in tables/infolists; use dedicated timestamp columns

Files:

  • Modules/Expenses/Filament/Exporters/ExpenseLegacyExporter.php
  • Modules/Products/Filament/Exporters/ProductLegacyExporter.php
  • Modules/Products/Filament/Company/Resources/Products/Pages/ListProducts.php
  • Modules/Projects/Filament/Exporters/TaskLegacyExporter.php
  • Modules/Clients/Filament/Exporters/RelationExporter.php
  • Modules/Projects/Filament/Exporters/TaskExporter.php
  • Modules/Clients/Filament/Exporters/RelationLegacyExporter.php
  • Modules/Payments/Filament/Exporters/PaymentExporter.php
  • Modules/Products/Filament/Exporters/ProductExporter.php
  • Modules/Invoices/Filament/Exporters/InvoiceLegacyExporter.php
  • Modules/Clients/Filament/Company/Resources/Contacts/Pages/ListContacts.php
  • Modules/Projects/Filament/Exporters/ProjectExporter.php
  • Modules/Invoices/Filament/Exporters/InvoiceExporter.php
  • Modules/Quotes/Filament/Exporters/QuoteLegacyExporter.php
  • Modules/Quotes/Filament/Exporters/QuoteExporter.php
  • Modules/Payments/Filament/Company/Resources/Payments/Tables/PaymentsTable.php
  • Modules/Payments/Filament/Exporters/PaymentLegacyExporter.php
  • Modules/Clients/Filament/Exporters/ContactLegacyExporter.php
  • Modules/Projects/Filament/Company/Resources/Projects/Pages/ListProjects.php
  • Modules/Core/Filament/Exporters/BaseExporter.php
  • Modules/Quotes/Filament/Company/Resources/Quotes/Pages/ListQuotes.php
  • Modules/Projects/Filament/Exporters/ProjectLegacyExporter.php
  • Modules/Payments/Filament/Company/Resources/Payments/Pages/ListPayments.php
  • Modules/Expenses/Filament/Exporters/ExpenseExporter.php
  • Modules/Clients/Filament/Exporters/ContactExporter.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Tables/TasksTable.php
  • Modules/Expenses/Filament/Company/Resources/Expenses/Pages/ListExpenses.php
  • Modules/Invoices/Filament/Company/Resources/Invoices/Pages/ListInvoices.php
  • Modules/Clients/Filament/Company/Resources/Relations/Pages/ListRelations.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php
{app,Modules}/**/Models/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{app,Modules}/**/Models/**/*.php: Do not define $fillable on Eloquent models
Do not enable timestamps or softDeletes on models unless explicitly specified
Use native PHP type hints and use $casts for Enum fields on models

Files:

  • Modules/Clients/Models/Relation.php
  • Modules/Payments/Models/Payment.php
🧠 Learnings (13)
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Filament/**/*.php : Filament resources must respect proper panel separation and namespaces

Applied to files:

  • composer.json
  • README.md
  • .github/copilot-instructions.md
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Filament/**/*.php : Add docblocks above form(), table(), and getRelations() with relationships/fields

Applied to files:

  • composer.json
  • Modules/Clients/Filament/Exporters/RelationExporter.php
  • Modules/Clients/Filament/Exporters/RelationLegacyExporter.php
  • Modules/Clients/Feature/Modules/RelationsExportImportTest.php
  • Modules/Projects/Filament/Exporters/ProjectLegacyExporter.php
  • .github/EXPORT-REFACTORING.md
  • .github/copilot-instructions.md
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Console/Commands/**/*Filament*.php : Resource generators must use Filament internal traits (e.g., CanReadModelSchemas)

Applied to files:

  • README.md
  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • .github/copilot-instructions.md
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to Modules/*/Tests/**/*.php : Unit test functions must be prefixed with it_

Applied to files:

  • Modules/Projects/Feature/Modules/ProjectsExportImportTest.php
  • Modules/Projects/Feature/Modules/TasksExportImportTest.php
  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • Modules/Expenses/Feature/Modules/ExpensesExportImportTest.php
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to Modules/*/Tests/**/*.php : Prefer Fakes and Fixtures over Mocks in tests

Applied to files:

  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php
  • .github/copilot-instructions.md
  • Modules/Payments/Feature/Modules/PaymentsExportImportTest.php
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to Modules/*/Tests/**/*.php : Add inline comment blocks for Arrange, Act, Assert sections in tests

Applied to files:

  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • .github/copilot-instructions.md
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Filament/**/*.php : Use the correct Action::make() syntax with fluent methods in Filament

Applied to files:

  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • Modules/Projects/Filament/Company/Resources/Projects/Pages/ListProjects.php
  • Modules/Quotes/Filament/Company/Resources/Quotes/Pages/ListQuotes.php
  • .github/EXPORT-REFACTORING.md
  • Modules/Expenses/Feature/Modules/ExpensesExportImportTest.php
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Projects/Filament/Company/Resources/Tasks/Pages/ListTasks.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Console/Commands/**/*Filament*.php : Use copyStubToApp() instead of inline string replacements in generators

Applied to files:

  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • .github/EXPORT-REFACTORING.md
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to Modules/*/Tests/**/*.php : Put reusable logic (fixtures/setup) in abstract test cases, not inline

Applied to files:

  • Modules/Quotes/Feature/Modules/QuotesExportImportTest.php
  • Modules/Clients/Feature/Modules/ClientsExportImportTest.php
  • .github/copilot-instructions.md
  • Modules/Payments/Feature/Modules/PaymentsExportImportTest.php
  • Modules/Products/Feature/Modules/ProductsExportImportTest.php
  • Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Filament/**/*.php : Preserve the exact method signatures for Filament resource methods

Applied to files:

  • Modules/Projects/Filament/Exporters/ProjectLegacyExporter.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Console/Commands/**/*Filament*.php : Separate form and table generators by field type

Applied to files:

  • .github/EXPORT-REFACTORING.md
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to {app,Modules}/**/Models/**/*.php : Use native PHP type hints and use $casts for Enum fields on models

Applied to files:

  • Modules/Payments/Models/Payment.php
📚 Learning: 2025-10-26T10:24:20.950Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-26T10:24:20.950Z
Learning: Applies to Modules/*/Tests/**/*.php : Do not use test annotations in unit tests

Applied to files:

  • .github/copilot-instructions.md
🪛 markdownlint-cli2 (0.18.1)
Modules/Core/Filament/Exporters/README.md

50-50: Bare URL used

(MD034, no-bare-urls)


56-56: Bare URL used

(MD034, no-bare-urls)


58-58: Bare URL used

(MD034, no-bare-urls)


60-60: Bare URL used

(MD034, no-bare-urls)


62-62: Bare URL used

(MD034, no-bare-urls)

🪛 PHPMD (2.15.0)
Modules/Expenses/Filament/Exporters/ExpenseLegacyExporter.php

18-18: Avoid unused parameters such as '$state'. (undefined)

(UnusedFormalParameter)

Modules/Projects/Feature/Modules/ProjectsExportImportTest.php

27-27: Avoid unused local variables such as '$projects'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$projects'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$project'. (undefined)

(UnusedLocalVariable)


134-134: Avoid unused local variables such as '$projects'. (undefined)

(UnusedLocalVariable)


160-160: Avoid unused local variables such as '$projects'. (undefined)

(UnusedLocalVariable)

Modules/Clients/Feature/Modules/RelationsExportImportTest.php

27-27: Avoid unused local variables such as '$relations'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$relations'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$relation'. (undefined)

(UnusedLocalVariable)


133-133: Avoid unused local variables such as '$relations'. (undefined)

(UnusedLocalVariable)


159-159: Avoid unused local variables such as '$relations'. (undefined)

(UnusedLocalVariable)

Modules/Projects/Feature/Modules/TasksExportImportTest.php

27-27: Avoid unused local variables such as '$tasks'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$tasks'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$task'. (undefined)

(UnusedLocalVariable)


134-134: Avoid unused local variables such as '$tasks'. (undefined)

(UnusedLocalVariable)


160-160: Avoid unused local variables such as '$tasks'. (undefined)

(UnusedLocalVariable)

Modules/Quotes/Feature/Modules/QuotesExportImportTest.php

27-27: Avoid unused local variables such as '$quotes'. (undefined)

(UnusedLocalVariable)


54-54: Avoid unused local variables such as '$quotes'. (undefined)

(UnusedLocalVariable)


106-106: Avoid unused local variables such as '$quote'. (undefined)

(UnusedLocalVariable)

Modules/Expenses/Feature/Modules/ExpensesExportImportTest.php

27-27: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)


55-55: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)


109-109: Avoid unused local variables such as '$expense'. (undefined)

(UnusedLocalVariable)


139-139: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)


167-167: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)


194-194: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)


221-221: Avoid unused local variables such as '$expenses'. (undefined)

(UnusedLocalVariable)

Modules/Clients/Feature/Modules/ClientsExportImportTest.php

27-27: Avoid unused local variables such as '$clients'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$clients'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$client'. (undefined)

(UnusedLocalVariable)


133-133: Avoid unused local variables such as '$clients'. (undefined)

(UnusedLocalVariable)


159-159: Avoid unused local variables such as '$clients'. (undefined)

(UnusedLocalVariable)

Modules/Payments/Feature/Modules/PaymentsExportImportTest.php

27-27: Avoid unused local variables such as '$payments'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$payments'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$payment'. (undefined)

(UnusedLocalVariable)


134-134: Avoid unused local variables such as '$payments'. (undefined)

(UnusedLocalVariable)


160-160: Avoid unused local variables such as '$payments'. (undefined)

(UnusedLocalVariable)

Modules/Products/Feature/Modules/ProductsExportImportTest.php

27-27: Avoid unused local variables such as '$products'. (undefined)

(UnusedLocalVariable)


53-53: Avoid unused local variables such as '$products'. (undefined)

(UnusedLocalVariable)


105-105: Avoid unused local variables such as '$product'. (undefined)

(UnusedLocalVariable)


134-134: Avoid unused local variables such as '$products'. (undefined)

(UnusedLocalVariable)


160-160: Avoid unused local variables such as '$products'. (undefined)

(UnusedLocalVariable)

Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php

27-27: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)


54-54: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)


106-106: Avoid unused local variables such as '$invoice'. (undefined)

(UnusedLocalVariable)


136-136: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)


162-162: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)


188-188: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)


214-214: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)

Comment on lines +25 to +27
Queue::fake();
Storage::fake('local');
$clients = Client::factory()->for($this->company)->count(3)->create();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fake the bus before calling Bus::assertChained()

Line 25: These tests call Bus::assertChained() but never execute Bus::fake(), so the facade stays bound to the real dispatcher and the assertion method is unavailable. Please insert Bus::fake() (once per test or centrally in setUp()) ahead of the action, and apply the same adjustment throughout this class.

-        Queue::fake();
-        Storage::fake('local');
+        Queue::fake();
+        Storage::fake('local');
+        Bus::fake();
🧰 Tools
🪛 PHPMD (2.15.0)

27-27: Avoid unused local variables such as '$clients'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents
In Modules/Clients/Feature/Modules/ClientsExportImportTest.php around lines 25
to 27, the tests call Bus::assertChained() but never call Bus::fake(), leaving
the Bus facade bound to the real dispatcher and making the assertion
unavailable; add Bus::fake() before the action that triggers the chained jobs
(either at the start of each test or once in the test class setUp() so it runs
before actions), and update all tests in this class that use
Bus::assertChained() to ensure Bus::fake() is executed beforehand.

Comment on lines +16 to +21
ExportColumn::make('relation_id')
->label(trans('ip.relation_id'))
->formatStateUsing(fn ($state, Contact $record) => $record->relation?->trading_name ?? $record->relation?->company_name ?? ''),
ExportColumn::make('type')
->label(trans('ip.type'))
->formatStateUsing(fn ($state, Contact $record) => $record->relation?->relation_type?->label() ?? ''),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify eager-loading of relation to prevent N+1.

Multiple columns access $record->relation, which will cause N+1 queries if not eager-loaded where this exporter is used.

Run this script to verify the relation is eager-loaded in export contexts:


🏁 Script executed:

#!/bin/bash
# Check if Filament export actions eager-load the relation

# Find where ContactLegacyExporter is used
rg -nP --type=php -C3 'ContactLegacyExporter'

# Look for ->with or ->load usage in export contexts
ast-grep --pattern $'ExportAction::make($_)
  $$$
  ->exporter(ContactLegacyExporter::class)
  $$$'

Length of output: 3301



---

🏁 Script executed:

```shell
#!/bin/bash
# Check the ListContacts page table configuration
rg -nP --type=php -A30 'function table\(\)|protected function table' Modules/Clients/Filament/Company/Resources/Contacts/Pages/ListContacts.php

# Check BaseExporter implementation
fd -e php -x grep -l 'BaseExporter' {} \; | head -5

# Look for BaseExporter definition
rg -nP --type=php -A20 'class BaseExporter' Modules/Core/Filament/Exporters/

Length of output: 2112



---

🏁 Script executed:

```shell
#!/bin/bash
# Check full ContactLegacyExporter implementation
cat Modules/Clients/Filament/Exporters/ContactLegacyExporter.php

# Check for modifyQuery or similar methods in BaseExporter
rg -nP --type=php 'modifyQuery|getTableQuery|query\(' Modules/Core/Filament/Exporters/BaseExporter.php

# Check if any with/load in exporters
rg -nP --type=php 'with\(|load\(' Modules/Clients/Filament/Exporters/ Modules/Core/Filament/Exporters/

Length of output: 1430



---

🏁 Script executed:

```shell
#!/bin/bash
# Check if Contact model has any eager loading configs or scopes
rg -nP --type=php 'protected \$with|with\s*=' Modules/Clients/Models/Contact.php

# Check if there's any query modification in Filament resource for contacts
rg -nP --type=php -B2 -A10 'modifyQueryUsing|getEloquentQuery' Modules/Clients/Filament/Company/Resources/Contacts/

# Look for Exporter parent class usage or hooks
ast-grep --pattern 'class BaseExporter extends $_' Modules/Core/Filament/Exporters/BaseExporter.php

Length of output: 0



---

🌐 Web query:

```web
Filament PHP ExportAction modifyQuery eager loading relations

💡 Result:

Yes — you can eager-load relations for Filament's ExportAction by modifying the query. Two approaches:

  1. On the action instance (per-use), call modifyQueryUsing() and add with():
    Example:
    ExportAction::make()
    ->exporter(ProductExporter::class)
    ->modifyQueryUsing(fn (Illuminate\Database\Eloquent\Builder $query, array $options = []) => $query->with(['relation', 'otherRelation']));

    This closure can also accept the $options array from the export options form. [1][2]

  2. In the exporter class (global for that exporter) override modifyQuery():
    Example:
    public static function modifyQuery(Builder $query): Builder
    {
    return $query->with([
    'purchasable' => fn (MorphTo $morphTo) => $morphTo->morphWith([
    ProductPurchase::class => ['product'],
    ServicePurchase::class => ['service'],
    ]),
    ]);
    }
    This is useful for polymorphic morphWith usage. [1]

Caveat: some exporter serialization libraries can error when the query contains eager-loaded relations. If you hit that, a common workaround is to clear eager loads in modifyQuery(): $query->setEagerLoads([]); then explicitly eager-load needed relations on the final query or in the exporter row-building. [6][7]

Sources:

  • Filament ExportAction docs (modifyQueryUsing / modifyQuery examples). [1]
  • Filament docs (modifyQuery example with morphWith). [2]
  • Community issue showing serialize problems and setEagerLoads([]) workaround. [7]

Numbered sources:
[1] Filament Export action — modifyQueryUsing docs.
[2] Filament Export action — modifyQuery example with morphWith.
[7] Community thread: export fails when query has with relation / workaround.



---

🏁 Script executed:

```shell
#!/bin/bash
# Final check: does BaseExporter have modifyQuery method?
rg -nP --type=php 'public static function modifyQuery|public function modifyQuery' Modules/Core/Filament/Exporters/BaseExporter.php

# Final check: does ContactLegacyExporter have modifyQuery method?
rg -nP --type=php 'public static function modifyQuery|public function modifyQuery' Modules/Clients/Filament/Exporters/ContactLegacyExporter.php

# Check if any exporter in the codebase has modifyQuery for reference
rg -nP --type=php 'public static function modifyQuery' Modules/

Length of output: 0


Add eager-loading of the relation to prevent N+1 queries.

The exporter accesses $record->relation and $record->relation->relation_type in lines 18–22 without eager-loading. Add a modifyQuery() method to ContactLegacyExporter:

public static function modifyQuery(Builder $query): Builder
{
    return $query->with(['relation.relation_type']);
}

Alternatively, add ->modifyQueryUsing(fn (Builder $query) => $query->with(['relation.relation_type'])) to each ExportAction in ListContacts.php (lines 37–41 and 47–51).

🤖 Prompt for AI Agents
In Modules/Clients/Filament/Exporters/ContactLegacyExporter.php around lines 16
to 21 the exporter accesses $record->relation and
$record->relation->relation_type without eager-loading causing N+1 queries; fix
by adding a modifyQuery() method on ContactLegacyExporter that returns the
Builder with the relation and relation_type relationships eager-loaded (use
with(['relation.relation_type'])), or alternatively add ->modifyQueryUsing(fn
(Builder $query) => $query->with(['relation.relation_type'])) to the
ExportAction definitions in ListContacts.php at lines 37–41 and 47–51 so the
relation and its relation_type are loaded in one query.

Comment on lines +14 to +33
public function export(string $format = 'xlsx'): BinaryFileResponse
{
$companyId = session('current_company_id');
$contacts = Contact::query()->where('company_id', $companyId)->get();
$fileName = 'contacts-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
$version = config('ip.export_version', 2);
$exportClass = $version === 1 ? ContactsLegacyExport::class : ContactsExport::class;

return Excel::download(new $exportClass($contacts), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
}

public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
{
$companyId = session('current_company_id');
$contacts = Contact::query()->where('company_id', $companyId)->get();
$fileName = 'contacts-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
$exportClass = $version === 1 ? ContactsLegacyExport::class : ContactsExport::class;

return Excel::download(new $exportClass($contacts), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Eliminate code duplication between export methods.

The export() and exportWithVersion() methods contain nearly identical logic (lines 16-22 vs 27-32). The only difference is how the version is determined (from config vs parameter).

Refactor to eliminate duplication:

 public function export(string $format = 'xlsx'): BinaryFileResponse
 {
-    $companyId   = session('current_company_id');
-    $contacts    = Contact::query()->where('company_id', $companyId)->get();
-    $fileName    = 'contacts-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
-    $version     = config('ip.export_version', 2);
-    $exportClass = $version === 1 ? ContactsLegacyExport::class : ContactsExport::class;
-
-    return Excel::download(new $exportClass($contacts), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
+    return $this->exportWithVersion($format, config('ip.export_version', 2));
 }

 public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
 {
     $companyId   = session('current_company_id');
     $contacts    = Contact::query()->where('company_id', $companyId)->get();
     $fileName    = 'contacts-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
     $exportClass = $version === 1 ? ContactsLegacyExport::class : ContactsExport::class;

     return Excel::download(new $exportClass($contacts), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
 }
🤖 Prompt for AI Agents
In Modules/Clients/Services/ContactExportService.php lines 14-33, both export()
and exportWithVersion() duplicate the same logic; modify export() to delegate to
exportWithVersion() by passing the configured version
(config('ip.export_version', 2)) and remove the duplicated body, keeping
exportWithVersion() as the single implementation that builds $companyId,
$contacts, $fileName, selects $exportClass and returns Excel::download(...);
ensure method signatures and return types remain BinaryFileResponse and preserve
the csv/xlsx selection logic.

Comment on lines +25 to 27
Queue::fake();
Storage::fake('local');
$invoices = Invoice::factory()->for($this->company)->count(3)->create();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fake the bus before using Bus::assertChained()

Line 25: The dispatcher isn’t faked, so calling Bus::assertChained() will crash against the real bus implementation. Please insert Bus::fake() (once per test or in shared setup) before triggering the action, and replicate this fix throughout the class.

-        Queue::fake();
-        Storage::fake('local');
+        Queue::fake();
+        Storage::fake('local');
+        Bus::fake();
🧰 Tools
🪛 PHPMD (2.15.0)

27-27: Avoid unused local variables such as '$invoices'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents
In Modules/Invoices/Feature/Modules/InvoicesExportImportTest.php around lines 25
to 27, the test uses Bus::assertChained() but never fakes the bus; insert a
Bus::fake() call before any action that dispatches jobs (either at the start of
this test, or better in the test class setUp so it applies to all tests), and
ensure every test in this file that asserts chained bus dispatches calls
Bus::fake() (or relies on the shared setUp) before triggering the behavior being
asserted.

Comment on lines +21 to +23
ExportColumn::make('customer_name')
->label(trans('ip.customer_name'))
->formatStateUsing(fn ($state, Invoice $record) => $record->customer?->trading_name ?? $record->customer?->company_name ?? ''),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify eager loading for customer relationship.

Line 23 accesses $record->customer?->trading_name and $record->customer?->company_name within the export closure. Ensure the customer relationship is eager loaded to prevent N+1 queries during bulk exports.


🏁 Script executed:

#!/bin/bash
# Check if BaseExporter or export mechanism handles relationship eager loading
rg -nP -C5 'with\(|load\(' Modules/Core/Filament/Exporters/BaseExporter.php

Length of output: 0


I need to gather more information about the export mechanism and relationship loading. Let me investigate the exporter structure and query handling.


🏁 Script executed:

#!/bin/bash
# Check if InvoiceExporter exists and its structure
fd -t f "InvoiceExporter.php" --exec cat {}

Length of output: 1346


🏁 Script executed:

#!/bin/bash
# Search for BaseExporter in different locations
fd -t f "BaseExporter.php"

Length of output: 48


🏁 Script executed:

#!/bin/bash
# Look for where invoice exports are executed/queried
rg -nP 'InvoiceExporter|->export|getQuery|->query' Modules/Invoices --type php -C3

Length of output: 4337


🏁 Script executed:

#!/bin/bash
# Search for eager loading patterns with 'customer' relationship
rg -nP 'with\(.*customer|load\(.*customer' Modules/Invoices --type php -C2

Length of output: 0


I need to check the BaseExporter class and Invoice model to understand how queries are built for exports.


🏁 Script executed:

#!/bin/bash
# Check BaseExporter implementation
cat Modules/Core/Filament/Exporters/BaseExporter.php

Length of output: 930


🏁 Script executed:

#!/bin/bash
# Check Invoice model for customer relationship
rg -nP 'public function customer|protected.*customer' Modules/Invoices/Models/Invoice.php -A2

Length of output: 124


🏁 Script executed:

#!/bin/bash
# Search for modifyQuery, query, or builder in BaseExporter
rg -nP 'modifyQuery|function.*query|getQuery|Exporter' Modules/Core/Filament/Exporters/ --type php -C3

Length of output: 830


Let me check if there are other exporters in the codebase that might show how relationships are handled, and search for any query customization patterns.


🏁 Script executed:

#!/bin/bash
# Find all exporters to see if any override query/relationship loading
fd -t f "Exporter.php" --exec grep -l "modifyQuery\|with(\|load(" {} \;

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check InvoiceLegacyExporter to see if it handles relationships differently
rg -nP 'class InvoiceLegacyExporter|with\(|customer' Modules/Invoices/Filament/Exporters/ --type php -B2 -A10

Length of output: 4319


🏁 Script executed:

#!/bin/bash
# Search for Filament Export usage patterns with query building in ListInvoices
cat Modules/Invoices/Filament/Company/Resources/Invoices/Pages/ListInvoices.php

Length of output: 2312


Let me check Filament's documentation to understand how the Exporter class handles relationship loading.


---

🌐 Web query:

```web
Filament export columns relationship eager loading N+1 queries

💡 Result:

Short answer — use ExportColumn dot-notation plus explicitly eager-load relationships on the export query (Exporter::modifyQuery or ExportAction::modifyQueryUsing) so the exporter doesn't trigger N+1s.

Why: ExportColumn supports relationship access via dot notation (e.g. ExportColumn::make('author.name')), but the export job uses the export query independently of your table rendering, so you must ensure relationships are eagerly loaded for that query to avoid N+1 queries. Filament provides two ways to modify the export query: override modifyQuery() on the Exporter class or call modifyQueryUsing() on the ExportAction. [1][2]

Examples:

  • In an Exporter:
    public static function modifyQuery(Builder $query): Builder
    {
    return $query->with(['author', 'author.profile', 'tags']);
    }
    // then ExportColumn::make('author.name')

  • When adding the ExportAction to a table:
    ExportAction::make()
    ->exporter(ProductExporter::class)
    ->modifyQueryUsing(fn (Builder $query) => $query->with(['author', 'tags']))

Notes:

  • You can also use relationship aggregation helpers (counts(), avg(), etc.) provided by ExportColumn to avoid loading full relations when you only need aggregates. [2]
  • If you rely on a model's $with, confirm the exporter query respects it — explicitly adding with(...) in modifyQuery is more reliable. [1][3]

Sources:

  • Filament ExportAction / Exporter docs (modifyQuery / modifyQueryUsing, ExportColumn relationship dot-notation & aggregates). [1][2]
  • Filament guidance on eager-loading and avoiding N+1s. [3]

References:
[1] Filament — Export action / Exporter documentation.
[2] Filament — ExportColumn relationship & aggregates docs.
[3] Filament / performance docs — eager loading guidance.


Add eager loading for customer relationship in exporters to prevent N+1 queries.

The InvoiceExporter and InvoiceLegacyExporter access $record->customer within formatStateUsing closures without eager loading. Per Filament's export query behavior, relationships are not automatically loaded during export jobs, causing N+1 queries during bulk exports.

Add modifyQuery() to both exporters:

public static function modifyQuery(Builder $query): Builder
{
    return $query->with('customer');
}

Alternatively, configure eager loading on the ExportAction in ListInvoices using modifyQueryUsing().

🤖 Prompt for AI Agents
In Modules/Invoices/Filament/Exporters/InvoiceExporter.php around lines 21-23,
the formatStateUsing closure accesses $record->customer causing N+1 queries
during exports; add a public static modifyQuery(Builder $query): Builder method
that returns $query->with('customer') to eager-load the customer relationship;
apply the same change to
Modules/Invoices/Filament/Exporters/InvoiceLegacyExporter.php (add identical
modifyQuery) so both exporters eager-load customer before exporting.

Comment on lines +25 to +33
public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
{
$companyId = session('current_company_id');
$quotes = Quote::query()->where('company_id', $companyId)->get();
$fileName = 'quotes-' . now()->format('Y-m-d_H-i-s') . '.' . ($format === 'csv' ? 'csv' : 'xlsx');
$exportClass = $version === 1 ? QuotesLegacyExport::class : QuotesExport::class;

return Excel::download(new $exportClass($quotes), $fileName, $format === 'csv' ? ExcelAlias::CSV : ExcelAlias::XLSX);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate session and address memory concerns.

The method lacks company context validation and may exhaust memory with large datasets.

Apply this diff:

 public function exportWithVersion(string $format = 'xlsx', int $version = 2): BinaryFileResponse
 {
     $companyId   = session('current_company_id');
+    
+    if (!$companyId) {
+        abort(403, 'No company context available');
+    }
+    
     $quotes      = Quote::query()->where('company_id', $companyId)->get();

For the memory concern, consider chunking for large datasets:

-    $quotes      = Quote::query()->where('company_id', $companyId)->get();
+    $quotes      = Quote::query()
+        ->where('company_id', $companyId)
+        ->orderBy('created_at', 'desc')
+        ->limit(10000)
+        ->get();

Or investigate if the export library supports streaming/cursor-based iteration.

🤖 Prompt for AI Agents
In Modules/Quotes/Services/QuoteExportService.php around lines 25 to 33,
validate that session('current_company_id') returns a non-null companyId and
handle the missing/invalid case (throw a meaningful exception or return an
appropriate error response) before querying; to prevent memory exhaustion avoid
calling ->get() for all quotes — instead use cursor()/chunkById() or switch to
an export implementation that accepts a query/stream (e.g., a FromQuery export
or generator) so rows are streamed/chunked to the Excel/CSV writer rather than
loaded into memory, and ensure the export class is constructed/used in a
streaming-friendly way while preserving the format selection logic.

Comment on lines 9 to +19
-
message: '#^Method Modules\\Expenses\\Filament\\Company\\Widgets\\RecentExpensesWidget\:\:getTableQuery\(\) should return Illuminate\\Database\\Eloquent\\Builder\|Illuminate\\Database\\Eloquent\\Relations\\Relation\|null but returns Illuminate\\Database\\Query\\Builder\.$#'
identifier: return.type
message: '#^Property Modules\\Core\\Tests\\Unit\\DateFieldAutoPopulationTest\:\:\$company \(Modules\\Core\\Models\\Company\) does not accept Illuminate\\Database\\Eloquent\\Collection\<int, Illuminate\\Database\\Eloquent\\Model\>\|Illuminate\\Database\\Eloquent\\Model\.$#'
identifier: assign.propertyType
count: 1
path: Modules/Expenses/Filament/Company/Widgets/RecentExpensesWidget.php
path: Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php

-
message: '#^Cannot access property \$rate on null\.$#'
identifier: property.nonObject
message: '#^Property Modules\\Core\\Tests\\Unit\\DateFieldAutoPopulationTest\:\:\$user \(Modules\\Core\\Models\\User\) does not accept Illuminate\\Database\\Eloquent\\Collection\<int, Illuminate\\Database\\Eloquent\\Model\>\|Illuminate\\Database\\Eloquent\\Model\.$#'
identifier: assign.propertyType
count: 1
path: Modules/Invoices/Database/Factories/InvoiceItemFactory.php
path: Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Consider fixing the type issues instead of baselining them.

These new baseline entries suppress type mismatches in DateFieldAutoPopulationTest.php where $company and $user properties receive generic Collection|Model types instead of the expected specific types (Company and User).

This typically occurs when factory methods lack proper return type declarations. Rather than adding to the baseline, consider:

  1. Adding explicit type casting in the test setup: $this->company = Company::factory()->create(); should be annotated or cast
  2. Improving factory return types if they're generic
  3. Using PHPStan @var annotations in the test class

Fixing these issues improves type safety and prevents potential bugs in test code.

Run this script to examine the test file and identify the exact factory usage patterns:


🏁 Script executed:

#!/bin/bash
# Description: Check the DateFieldAutoPopulationTest.php file to see how $company and $user are assigned

cat Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php | grep -A 5 -B 5 "this->company\|this->user"

Length of output: 7577



🏁 Script executed:

#!/bin/bash
# Check if the test class has property declarations with types for $user and $company
rg -A 2 -B 2 'class DateFieldAutoPopulationTest' Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php | head -40
rg -n 'protected.*\$company\|protected.*\$user\|private.*\$company\|private.*\$user' Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php

Length of output: 137


🏁 Script executed:

#!/bin/bash
# Check the factory definitions to see their return types
fd -e "php" -path "*User*Factory*" | head -5
fd -e "php" -path "*Company*Factory*" | head -5

Length of output: 337



🏁 Script executed:

#!/bin/bash
# Search for property declarations in the test class (without type hints)
rg -n '^\s*(protected|private|public)\s+\$' Modules/Core/Tests/Unit/DateFieldAutoPopulationTest.php

# Check the parent class
rg -A 10 'class AbstractCompanyPanelTestCase' --type php

# Find factory files
fd 'Factory.php' | grep -E '(User|Company)Factory'

Length of output: 946



🏁 Script executed:

#!/bin/bash
# Check the factory return types
echo "=== UserFactory ==="
rg -A 5 'class UserFactory|public function create' Modules/Core/Database/Factories/UserFactory.php | head -30

echo ""
echo "=== CompanyFactory ==="
rg -A 5 'class CompanyFactory|public function create' Modules/Core/Database/Factories/CompanyFactory.php | head -30

Length of output: 287


Add return type hints to factories or cast in test setup instead of baselining.

The type mismatch occurs because UserFactory and CompanyFactory don't declare return types on their create() methods. PHPStan infers Collection|Model instead of the specific model types, conflicting with the properly typed properties protected User $user; and protected Company $company; in AbstractCompanyPanelTestCase.

Fix this by:

  • Adding return type hints to factory create() methods: public function create(): User { ... }
  • Or type-casting in setUp(): $this->user = (User)User::factory()->create();

This ensures type safety and prevents masking potential issues in test code. Baselining avoids fixing the root cause.

@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 13, 2025
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 13, 2025
…utomatic versioning, and GitHub releases (#134)

* Initial plan

* Add production release workflow with Crowdin integration

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Optimize release workflow based on code review feedback

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Address final code review feedback and update documentation

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Add explicit permissions to workflow for security (CodeQL fix)

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Enhance workflow with user feedback: PHP 8.4, vendor cleaner, checksums, release notes

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Address code review feedback: optimize vendor cleaner install, improve mPDF cleanup, limit release notes

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Update Crowdin config to use full language names instead of two-letter codes

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

* Add automatic versioning, tagging, and GitHub release creation

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@InvoicePlane InvoicePlane deleted a comment from coderabbitai bot Nov 21, 2025
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