diff --git a/app/Console/Commands/CreateTemplateCommand.php b/app/Console/Commands/CreateTemplateCommand.php index a47c60db..81c25dea 100644 --- a/app/Console/Commands/CreateTemplateCommand.php +++ b/app/Console/Commands/CreateTemplateCommand.php @@ -10,6 +10,14 @@ use Illuminate\Support\Str; class CreateTemplateCommand extends Command { + /** + * Document types that can be cloned. The --type option is checked against + * this rather than only the interactive prompt: passing an unsupported one + * used to skip the prompt and die on an uncaught FileNotFoundException + * further down, with a stack trace instead of a message. + */ + private const TYPES = ['invoice', 'estimate']; + /** * The name and signature of the console command. * @@ -22,17 +30,7 @@ class CreateTemplateCommand extends Command * * @var string */ - protected $description = 'Create estimate or invoice pdf template. '; - - /** - * Create a new command instance. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - } + protected $description = 'Create estimate or invoice pdf template.'; /** * Execute the console command. @@ -43,7 +41,23 @@ class CreateTemplateCommand extends Command $templateType = $this->option('type'); if (! $templateType) { - $templateType = $this->choice('Create a template for?', ['invoice', 'estimate']); + $templateType = $this->choice('Create a template for?', self::TYPES); + } + + if (! in_array($templateType, self::TYPES, true)) { + $this->error(sprintf( + 'Unsupported template type "%s". Supported types: %s.', + $templateType, + implode(', ', self::TYPES) + )); + + return self::INVALID; + } + + if (! preg_match('/^[A-Za-z0-9._-]+$/', $templateName)) { + $this->error('Template name may only contain letters, numbers, dots, dashes and underscores.'); + + return self::INVALID; } if (PdfTemplateUtils::customTemplateFileExists($templateType, sprintf('%s.blade.php', $templateName))) { @@ -52,15 +66,26 @@ class CreateTemplateCommand extends Command return self::INVALID; } - if (! PdfTemplateUtils::toCustomTemplateMarkupFile( - Str::replace( - sprintf('app.pdf.%s', $templateType), - sprintf('pdf_templates::%s', $templateType), - Storage::disk('views')->get("/app/pdf/{$templateType}/{$templateType}1.blade.php"), - ), - $templateType, - $templateName - )) { + $source = Storage::disk('views')->get("/app/pdf/{$templateType}/{$templateType}1.blade.php"); + + // Point this template at its own copy of the shared partial before the + // blanket namespace rewrite below catches it. Previously every custom + // template of a type included the same partials/table.blade.php, which + // was written once and then reused, so editing the table for one custom + // template silently changed it for all of them. + $source = Str::replace( + sprintf('app.pdf.%s.partials.table', $templateType), + sprintf('pdf_templates::%s.partials.%s.table', $templateType, $templateName), + $source, + ); + + $source = Str::replace( + sprintf('app.pdf.%s', $templateType), + sprintf('pdf_templates::%s', $templateType), + $source, + ); + + if (! PdfTemplateUtils::toCustomTemplateMarkupFile($source, $templateType, $templateName)) { $this->error(sprintf('Unable to create %s template.', ucfirst($templateType))); return self::FAILURE; @@ -72,12 +97,25 @@ class CreateTemplateCommand extends Command $templateName, ); - if (! PdfTemplateUtils::customTemplateFileExists($templateType, 'partials/table.blade.php')) { - PdfTemplateUtils::toCustomTemplateFile( - Storage::disk('views')->get("/app/pdf/{$templateType}/partials/table.blade.php"), - $templateType, - 'partials/table.blade.php' - ); + PdfTemplateUtils::toCustomTemplateFile( + Storage::disk('views')->get("/app/pdf/{$templateType}/partials/table.blade.php"), + $templateType, + sprintf('partials/%s/table.blade.php', $templateName), + ); + + // Repeating page header/footer, if the source template has one. Named + // with the {template}_header / {template}_footer suffix the Gotenberg + // driver looks for. + foreach (['_header', '_footer'] as $suffix) { + $companion = "/app/pdf/{$templateType}/{$templateType}1{$suffix}.blade.php"; + + if (Storage::disk('views')->exists($companion)) { + PdfTemplateUtils::toCustomTemplateFile( + Storage::disk('views')->get($companion), + $templateType, + sprintf('%s%s.blade.php', $templateName, $suffix), + ); + } } $this->info( diff --git a/app/Http/Requests/EstimatesRequest.php b/app/Http/Requests/EstimatesRequest.php index 870e134c..333802bb 100644 --- a/app/Http/Requests/EstimatesRequest.php +++ b/app/Http/Requests/EstimatesRequest.php @@ -5,6 +5,7 @@ namespace App\Http\Requests; use App\Models\CompanySetting; use App\Models\Customer; use App\Models\Estimate; +use App\Rules\PdfTemplateExists; use App\Support\DocumentTotals; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Validation\Rule; @@ -64,6 +65,7 @@ class EstimatesRequest extends FormRequest ], 'template_name' => [ 'required', + new PdfTemplateExists('estimate'), ], 'items' => [ 'required', diff --git a/app/Http/Requests/InvoicesRequest.php b/app/Http/Requests/InvoicesRequest.php index 561db0f3..d1a73524 100644 --- a/app/Http/Requests/InvoicesRequest.php +++ b/app/Http/Requests/InvoicesRequest.php @@ -5,6 +5,7 @@ namespace App\Http\Requests; use App\Models\CompanySetting; use App\Models\Customer; use App\Models\Invoice; +use App\Rules\PdfTemplateExists; use App\Support\DocumentTotals; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Validation\Rule; @@ -63,6 +64,7 @@ class InvoicesRequest extends FormRequest ], 'template_name' => [ 'required', + new PdfTemplateExists('invoice'), ], 'items' => [ 'required', diff --git a/app/Models/Estimate.php b/app/Models/Estimate.php index e2c4864c..a815c945 100644 --- a/app/Models/Estimate.php +++ b/app/Models/Estimate.php @@ -296,11 +296,9 @@ class Estimate extends Model implements HasMedia { $templateName = Str::replace('estimate', 'invoice', $this->template_name); - $name = []; - - foreach (PdfTemplateUtils::getFormattedTemplates('invoice') as $template) { - $name[] = $template['name']; - } + // Empty image format: only the names are wanted here, and the default + // builds a base64 preview for every template to answer that. + $name = array_column(PdfTemplateUtils::getFormattedTemplates('invoice', ''), 'name'); if (in_array($templateName, $name) == false) { $templateName = 'invoice1'; diff --git a/app/Models/Invoice.php b/app/Models/Invoice.php index 3b9b0262..d4481951 100644 --- a/app/Models/Invoice.php +++ b/app/Models/Invoice.php @@ -303,10 +303,9 @@ class Invoice extends Model implements HasMedia { $templateName = Str::replace('invoice', 'estimate', $this->template_name); - $names = []; - foreach (PdfTemplateUtils::getFormattedTemplates('estimate') as $template) { - $names[] = $template['name']; - } + // Empty image format: only the names are wanted here, and the default + // builds a base64 preview for every template to answer that. + $names = array_column(PdfTemplateUtils::getFormattedTemplates('estimate', ''), 'name'); if (! in_array($templateName, $names)) { $templateName = 'estimate1'; diff --git a/app/Rules/PdfTemplateExists.php b/app/Rules/PdfTemplateExists.php new file mode 100644 index 00000000..97398251 --- /dev/null +++ b/app/Rules/PdfTemplateExists.php @@ -0,0 +1,35 @@ +templateType, ''), 'name'); + + if (! in_array($value, $names, true)) { + $fail("The selected :attribute is not an available {$this->templateType} template."); + } + } +} diff --git a/app/Support/Pdf/PdfTemplateUtils.php b/app/Support/Pdf/PdfTemplateUtils.php index c452ca6a..6a6c4776 100644 --- a/app/Support/Pdf/PdfTemplateUtils.php +++ b/app/Support/Pdf/PdfTemplateUtils.php @@ -64,15 +64,23 @@ class PdfTemplateUtils ); }); - return array_map(function ($file) use ($templateType, $imageFormat) { + $formatted = []; + + foreach ($files as $file) { $templateName = Str::before(basename($file['path']), '.blade.php'); if ($file['custom']) { $imagePath = self::getCustomTemplateFilePath($templateType, sprintf('%s.png', $templateName)); - $isCustomTemplate = true; + + // A custom template needs a same-named .png. Without one the + // picker used to render — a blank tile with no hint + // that anything was missing. Fall back to the preview of the + // template make:template clones from. + if (! File::exists($imagePath)) { + $imagePath = resource_path("static/img/PDF/{$templateType}1.png"); + } } else { $imagePath = resource_path('static/img/PDF/'.$templateName.'.png'); - $isCustomTemplate = false; } if (empty($imageFormat)) { @@ -83,12 +91,19 @@ class PdfTemplateUtils $imageValue = File::exists($imagePath) ? ImageUtils::toBase64Src($imagePath) : ''; } - return [ + // Keyed by name so a custom template that shares a built-in's name + // appears once rather than twice. Custom entries come last and so + // win, which matches what findFormattedTemplate() already resolved + // to — the picker just used to show both tiles with no way to tell + // which one you were clicking. + $formatted[$templateName] = [ 'name' => $templateName, 'path' => $imageValue, - 'custom' => $isCustomTemplate, + 'custom' => $file['custom'], ]; - }, $files); + } + + return array_values($formatted); } /** diff --git a/tests/Feature/Pdf/CustomTemplateCommandTest.php b/tests/Feature/Pdf/CustomTemplateCommandTest.php new file mode 100644 index 00000000..8541684c --- /dev/null +++ b/tests/Feature/Pdf/CustomTemplateCommandTest.php @@ -0,0 +1,92 @@ +path("{$type}/{$file}"); +} + +test('it clones a template that renders through the custom namespace', function () { + Artisan::call('make:template', ['name' => 'branded', '--type' => 'invoice']); + + expect(File::exists(customTemplatePath('invoice', 'branded.blade.php')))->toBeTrue(); + + $markup = File::get(customTemplatePath('invoice', 'branded.blade.php')); + + expect($markup)->not->toContain('app.pdf.invoice') + ->and($markup)->toContain('pdf_templates::invoice'); +}); + +/** + * Every custom template of a type used to include the same + * partials/table.blade.php, written once and then reused, so editing the table + * for one silently changed it for all of them. + */ +test('each template gets its own copy of the shared partial', function () { + Artisan::call('make:template', ['name' => 'first', '--type' => 'invoice']); + Artisan::call('make:template', ['name' => 'second', '--type' => 'invoice']); + + expect(File::exists(customTemplatePath('invoice', 'partials/first/table.blade.php')))->toBeTrue() + ->and(File::exists(customTemplatePath('invoice', 'partials/second/table.blade.php')))->toBeTrue(); + + expect(File::get(customTemplatePath('invoice', 'first.blade.php'))) + ->toContain('pdf_templates::invoice.partials.first.table'); +}); + +test('a preview image is written so the picker has something to show', function () { + Artisan::call('make:template', ['name' => 'branded', '--type' => 'invoice']); + + expect(File::exists(customTemplatePath('invoice', 'branded.png')))->toBeTrue(); +}); + +test('the new template shows up in the picker', function () { + Artisan::call('make:template', ['name' => 'branded', '--type' => 'invoice']); + + $names = array_column(PdfTemplateUtils::getFormattedTemplates('invoice', ''), 'name'); + + expect($names)->toContain('branded'); +}); + +/** + * --type was never checked against the supported list. An unsupported value + * skipped the interactive prompt and then died on an uncaught + * FileNotFoundException looking for e.g. payment1.blade.php. + */ +test('an unsupported type is refused with a message rather than a stack trace', function () { + $exit = Artisan::call('make:template', ['name' => 'receipt', '--type' => 'payment']); + + expect($exit)->toBe(Command::INVALID) + ->and(Artisan::output())->toContain('Unsupported template type'); +}); + +test('a name that would escape the templates directory is refused', function (string $name) { + $exit = Artisan::call('make:template', ['name' => $name, '--type' => 'invoice']); + + expect($exit)->toBe(Command::INVALID); +})->with([ + '../escaped', + 'nested/path', +]); + +test('an existing name is not overwritten', function () { + Artisan::call('make:template', ['name' => 'branded', '--type' => 'invoice']); + File::put(customTemplatePath('invoice', 'branded.blade.php'), 'EDITED BY USER'); + + $exit = Artisan::call('make:template', ['name' => 'branded', '--type' => 'invoice']); + + expect($exit)->toBe(Command::INVALID) + ->and(File::get(customTemplatePath('invoice', 'branded.blade.php')))->toBe('EDITED BY USER'); +}); diff --git a/tests/Feature/Pdf/PdfTemplateListingTest.php b/tests/Feature/Pdf/PdfTemplateListingTest.php index 303077ed..a8cc9cf7 100644 --- a/tests/Feature/Pdf/PdfTemplateListingTest.php +++ b/tests/Feature/Pdf/PdfTemplateListingTest.php @@ -51,3 +51,46 @@ test('a template whose name only contains the word is still listed', function () expect($names)->toContain('header_led_design'); }); + +/** + * A custom template sharing a built-in's name used to appear twice with the same + * label. findFormattedTemplate() array_reverses and takes the first match, so + * the custom one silently won -- the picker gave no indication which tile you + * were clicking. + */ +test('a custom template shadowing a built-in appears once, as the custom one', function () { + writeCustomTemplate($this->customDir, 'invoice1'); + + $templates = PdfTemplateUtils::getFormattedTemplates('invoice', ''); + $matching = array_values(array_filter($templates, fn ($t) => $t['name'] === 'invoice1')); + + expect($matching)->toHaveCount(1) + ->and($matching[0]['custom'])->toBeTrue(); +}); + +/** + * A custom template needs a same-named .png. Without one the picker rendered + * : a blank tile, no error, no hint anything was missing. + */ +test('a custom template with no preview falls back rather than rendering blank', function () { + writeCustomTemplate($this->customDir, 'no_preview'); + + $templates = PdfTemplateUtils::getFormattedTemplates('invoice'); + $entry = collect($templates)->firstWhere('name', 'no_preview'); + + expect($entry['path'])->toStartWith('data:image/png;base64,'); +}); + +test('a custom template with its own preview uses it', function () { + writeCustomTemplate($this->customDir, 'with_preview'); + File::put( + "{$this->customDir}/with_preview.png", + base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==') + ); + + $stock = collect(PdfTemplateUtils::getFormattedTemplates('invoice'))->firstWhere('name', 'invoice1'); + $custom = collect(PdfTemplateUtils::getFormattedTemplates('invoice'))->firstWhere('name', 'with_preview'); + + expect($custom['path'])->toStartWith('data:image/png;base64,') + ->and($custom['path'])->not->toBe($stock['path']); +}); diff --git a/tests/Feature/Pdf/PdfTemplateValidationTest.php b/tests/Feature/Pdf/PdfTemplateValidationTest.php new file mode 100644 index 00000000..95dcdd74 --- /dev/null +++ b/tests/Feature/Pdf/PdfTemplateValidationTest.php @@ -0,0 +1,65 @@ + 'DatabaseSeeder', '--force' => true]); + Artisan::call('db:seed', ['--class' => 'DemoSeeder', '--force' => true]); + + $user = User::find(1); + $this->company = $user->companies()->first(); + $this->withHeaders(['company' => $this->company->id]); + Sanctum::actingAs($user, ['*']); +}); + +test('the rule accepts a template the picker offers', function () { + $validator = Validator::make( + ['template_name' => 'invoice1'], + ['template_name' => [new PdfTemplateExists('invoice')]] + ); + + expect($validator->fails())->toBeFalse(); +}); + +test('the rule rejects a template that does not exist', function () { + $validator = Validator::make( + ['template_name' => 'no-such-template'], + ['template_name' => [new PdfTemplateExists('invoice')]] + ); + + expect($validator->fails())->toBeTrue(); +}); + +/** + * The types are separate directories, so an estimate template is not a valid + * invoice template even though both exist. + */ +test('the rule is scoped to the document type', function () { + $validator = Validator::make( + ['template_name' => 'estimate1'], + ['template_name' => [new PdfTemplateExists('invoice')]] + ); + + expect($validator->fails())->toBeTrue(); +}); + +test('creating an invoice with an unknown template fails validation, not at render time', function () { + postJson('/api/v1/invoices', [ + 'invoice_date' => '2026-01-01', + 'due_date' => '2026-01-31', + 'customer_id' => 1, + 'template_name' => 'definitely-not-a-template', + 'items' => [], + ])->assertStatus(422)->assertJsonValidationErrors('template_name'); +});