From ff8e5d28e8de6a44f09d378d7bbd52148769d3c9 Mon Sep 17 00:00:00 2001 From: Naomi Gilbert Date: Fri, 31 Jul 2026 20:02:52 -0400 Subject: [PATCH] fix(reports): persist layout view/pagination settings and de-duplicate CSV group column Report save silently dropped part of the layout. Laravel's validate() only returns keys that carry a rule, and store/update declared rules for layout.fields and layout.group_order only, so view_id, show_headers_only and per_page never reached the database. The consequences reported by manual QA: the selected view never persisted, and a saved report's PDF fell back to the default table because its layout carried no view_id. Declare the three keys explicitly. The CSV export also prepended the group-by field and then looped over every selected column. Since the grouped field is normally displayed too, its header and value appeared twice on every row. Emit it once. Both are covered by regression tests. --- .../app/Http/Controllers/ReportController.php | 16 +++- apps/api/tests/Feature/ReportFeatureTest.php | 73 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/apps/api/app/Http/Controllers/ReportController.php b/apps/api/app/Http/Controllers/ReportController.php index 81e46a2..9326d7d 100644 --- a/apps/api/app/Http/Controllers/ReportController.php +++ b/apps/api/app/Http/Controllers/ReportController.php @@ -55,6 +55,11 @@ public function store(Request $request): JsonResponse 'layout.fields.*.order' => 'sometimes|integer', 'layout.group_order' => 'nullable|array', 'layout.group_order.*' => 'string', + // Declared explicitly: validate() only returns keys that carry a rule, so + // anything omitted here is dropped before the report is persisted. + 'layout.view_id' => 'nullable|uuid|exists:views,id', + 'layout.show_headers_only' => 'nullable|boolean', + 'layout.per_page' => 'nullable|integer|min:1|max:100', ]); $report = Report::create($validated); @@ -84,6 +89,11 @@ public function update(Request $request, Report $report): JsonResponse 'layout.fields.*.order' => 'sometimes|integer', 'layout.group_order' => 'nullable|array', 'layout.group_order.*' => 'string', + // Declared explicitly: validate() only returns keys that carry a rule, so + // anything omitted here is dropped before the report is persisted. + 'layout.view_id' => 'nullable|uuid|exists:views,id', + 'layout.show_headers_only' => 'nullable|boolean', + 'layout.per_page' => 'nullable|integer|min:1|max:100', ]); $report->update($validated); @@ -243,11 +253,13 @@ private function generateCsvResponse(array $columns, array $groups, ?string $gro $csvHeaders = []; $showHeadersOnly = ($layout['show_headers_only'] ?? false) === true; + // The group field is commonly also a selected column; emit it once. + $groupIsSelectedColumn = $groupBy !== null && in_array($groupBy, $columns, true); if ($showHeadersOnly) { $csvHeaders[] = $groupBy ?? 'Groupe'; $csvHeaders[] = 'Nombre de fiches'; } else { - if ($groupBy) { + if ($groupBy && ! $groupIsSelectedColumn) { $csvHeaders[] = $groupBy; } foreach ($columns as $col) { @@ -264,7 +276,7 @@ private function generateCsvResponse(array $columns, array $groups, ?string $gro } else { foreach ($group['records'] as $rec) { $row = []; - if ($groupBy) { + if ($groupBy && ! $groupIsSelectedColumn) { $row[] = $groupKey; } foreach ($columns as $col) { diff --git a/apps/api/tests/Feature/ReportFeatureTest.php b/apps/api/tests/Feature/ReportFeatureTest.php index 9edd274..17ad79b 100644 --- a/apps/api/tests/Feature/ReportFeatureTest.php +++ b/apps/api/tests/Feature/ReportFeatureTest.php @@ -6,6 +6,7 @@ use App\Models\Report; use App\Models\Table; use App\Models\User; +use App\Models\View; use App\Models\Workspace; use App\Models\WorkspaceMember; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -359,3 +360,75 @@ function createAuthenticatedUser() expect($previewCsvResponse->headers->get('Content-Type'))->toContain('text/csv'); expect(str_contains($previewCsvResponse->streamedContent(), 'title'))->toBeTrue(); }); + +test('layout view_id, show_headers_only and per_page survive save and reload', function () { + $setup = createAuthenticatedUser(); + $user = $setup['user']; + $table = $setup['table']; + $view = View::factory()->create(['table_id' => $table->id]); + + $payload = [ + 'table_id' => $table->id, + 'name' => 'Rapport avec vue', + 'query' => ['select' => ['Nom'], 'group_by' => 'Ville'], + 'layout' => [ + 'fields' => [['name' => 'Nom', 'visible' => true, 'order' => 1]], + 'view_id' => $view->id, + 'show_headers_only' => true, + 'per_page' => 25, + ], + ]; + + $created = $this->actingAs($user)->postJson('/api/v1/reports', $payload); + $created->assertStatus(201); + + $report = Report::find($created->json('id')); + expect($report->layout['view_id'])->toBe($view->id); + expect($report->layout['show_headers_only'])->toBeTrue(); + expect($report->layout['per_page'])->toBe(25); + + // The same keys must also survive an update. + $otherView = View::factory()->create(['table_id' => $table->id]); + $payload['layout']['view_id'] = $otherView->id; + $payload['layout']['per_page'] = 50; + + $this->actingAs($user) + ->putJson("/api/v1/reports/{$report->id}", $payload) + ->assertStatus(200); + + $report->refresh(); + expect($report->layout['view_id'])->toBe($otherView->id); + expect($report->layout['per_page'])->toBe(50); +}); + +test('csv export emits the group field once when it is also a selected column', function () { + $setup = createAuthenticatedUser(); + $user = $setup['user']; + $table = $setup['table']; + + Field::factory()->create(['table_id' => $table->id, 'name' => 'Ville', 'type' => 'text']); + Field::factory()->create(['table_id' => $table->id, 'name' => 'Nom', 'type' => 'text']); + + Record::create([ + 'table_id' => $table->id, + 'data' => ['Ville' => 'Québec', 'Nom' => 'Tremblay'], + 'version' => 1, + ]); + + $response = $this->actingAs($user)->postJson('/api/v1/reports/preview/csv', [ + 'table_id' => $table->id, + 'query' => ['select' => ['Ville', 'Nom'], 'group_by' => 'Ville'], + 'layout' => ['fields' => [ + ['name' => 'Ville', 'visible' => true, 'order' => 1], + ['name' => 'Nom', 'visible' => true, 'order' => 2], + ]], + ]); + + $response->assertStatus(200); + $csv = $response->streamedContent(); + $headerLine = str_getcsv(explode("\n", trim($csv))[0]); + $headerLine[0] = preg_replace('/^\x{FEFF}/u', '', $headerLine[0]); + + expect($headerLine)->toBe(['Ville', 'Nom']); + expect(array_count_values($headerLine)['Ville'])->toBe(1); +});