diff --git a/.env.example b/.env.example index c0b7e662..6c943abc 100644 --- a/.env.example +++ b/.env.example @@ -35,6 +35,11 @@ DOMPDF_ENABLE_REMOTE=false # PDF_MARGIN_RIGHT=0 # PDF_MARGIN_BOTTOM=0 # PDF_MARGIN_LEFT=0 +# +# The report templates get their own margin on all four sides: unlike the +# document templates they carry no inset of their own, and reports can run to +# several pages, where only a page margin repeats. +# PDF_REPORT_MARGIN=1.2cm # Gotenberg (optional alternative PDF driver; the default is dompdf). # PDF_DRIVER=gotenberg diff --git a/app/Console/Commands/CreateTemplateCommand.php b/app/Console/Commands/CreateTemplateCommand.php index 84d4d3be..b1b1b4d3 100644 --- a/app/Console/Commands/CreateTemplateCommand.php +++ b/app/Console/Commands/CreateTemplateCommand.php @@ -104,15 +104,15 @@ class CreateTemplateCommand extends Command $source = Storage::disk('views')->get("/app/pdf/{$templateType}/{$sourceName}.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), + // Point this template at its own copies of the partials its type ships + // before the blanket namespace rewrite below catches them. 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 = PdfTemplateUtils::rewriteViewReferences( $source, + PdfTemplateUtils::partialViewMap($templateType, $templateName), ); $source = Str::replace( @@ -137,15 +137,11 @@ class CreateTemplateCommand extends Command ); } - $partial = "/app/pdf/{$templateType}/partials/table.blade.php"; - - if (Storage::disk('views')->exists($partial)) { - PdfTemplateUtils::toCustomTemplateFile( - Storage::disk('views')->get($partial), - $templateType, - sprintf('partials/%s/table.blade.php', $templateName), - ); - } + // Every partial of the type, not just the table: reports extend a shared + // layout which in turn includes a shared stylesheet, and a clone that + // was rewritten to the custom namespace without them pointed at views + // that did not exist and failed to render. + PdfTemplateUtils::copyTemplatePartials($templateType, $templateName); // Repeating page header/footer, if the source template has one. Named // with the {template}_header / {template}_footer suffix the Gotenberg diff --git a/app/Facades/Pdf.php b/app/Facades/Pdf.php index 46091f76..1f284454 100644 --- a/app/Facades/Pdf.php +++ b/app/Facades/Pdf.php @@ -5,7 +5,7 @@ namespace App\Facades; use Illuminate\Support\Facades\Facade; /** - * @method static \App\Support\Pdf\ResponseStream loadView(string $template, array $metadata = []) + * @method static \App\Support\Pdf\ResponseStream loadView(string $template, array $metadata = [], ?\App\Support\Pdf\PdfPageSetup $page = null) */ class Pdf extends Facade { diff --git a/app/Http/Controllers/Company/Report/CustomerSalesReportController.php b/app/Http/Controllers/Company/Report/CustomerSalesReportController.php index a3e15ece..b67e8222 100644 --- a/app/Http/Controllers/Company/Report/CustomerSalesReportController.php +++ b/app/Http/Controllers/Company/Report/CustomerSalesReportController.php @@ -8,6 +8,7 @@ use App\Models\Company; use App\Models\CompanySetting; use App\Models\Currency; use App\Models\Customer; +use App\Support\Pdf\PdfPageSetup; use App\Support\Pdf\PdfTemplateUtils; use Carbon\Carbon; use Illuminate\Http\JsonResponse; @@ -68,27 +69,11 @@ class CustomerSalesReportController extends Controller $to_date = Carbon::createFromFormat('Y-m-d', $request->to_date)->translatedFormat($dateFormat); $currency = Currency::findOrFail(CompanySetting::getSetting('currency', $company->id)); - $colors = [ - 'primary_text_color', - 'heading_text_color', - 'section_heading_text_color', - 'border_color', - 'body_text_color', - 'footer_text_color', - 'footer_total_color', - 'footer_bg_color', - 'date_text_color', - ]; - - $colorSettings = CompanySetting::whereIn('option', $colors) - ->whereCompany($company->id) - ->get(); - view()->share([ 'customers' => $customers, 'totalAmount' => $totalAmount, - 'colorSettings' => $colorSettings, 'company' => $company, + 'logo' => $company->logo_path, 'from_date' => $from_date, 'to_date' => $to_date, 'currency' => $currency, @@ -99,7 +84,7 @@ class CustomerSalesReportController extends Controller // template picker it has no concept of. $templatePath = PdfTemplateUtils::resolveView('reports', 'sales-customers'); - $pdf = Pdf::loadView($templatePath); + $pdf = Pdf::loadView($templatePath, [], PdfPageSetup::forReports()); if ($request->has('preview')) { return view($templatePath); diff --git a/app/Http/Controllers/Company/Report/ExpensesReportController.php b/app/Http/Controllers/Company/Report/ExpensesReportController.php index eabca0cd..6b07367a 100644 --- a/app/Http/Controllers/Company/Report/ExpensesReportController.php +++ b/app/Http/Controllers/Company/Report/ExpensesReportController.php @@ -8,6 +8,7 @@ use App\Models\Company; use App\Models\CompanySetting; use App\Models\Currency; use App\Models\Expense; +use App\Support\Pdf\PdfPageSetup; use App\Support\Pdf\PdfTemplateUtils; use Carbon\Carbon; use Illuminate\Contracts\View\View; @@ -68,26 +69,11 @@ class ExpensesReportController extends Controller $to_date = Carbon::createFromFormat('Y-m-d', $request->to_date)->translatedFormat($dateFormat); $currency = Currency::findOrFail(CompanySetting::getSetting('currency', $company->id)); - $colors = [ - 'primary_text_color', - 'heading_text_color', - 'section_heading_text_color', - 'border_color', - 'body_text_color', - 'footer_text_color', - 'footer_total_color', - 'footer_bg_color', - 'date_text_color', - ]; - $colorSettings = CompanySetting::whereIn('option', $colors) - ->whereCompany($company->id) - ->get(); - view()->share([ 'expenseGroups' => $expenseGroups, - 'colorSettings' => $colorSettings, 'totalExpense' => $totalAmount, 'company' => $company, + 'logo' => $company->logo_path, 'from_date' => $from_date, 'to_date' => $to_date, 'currency' => $currency, @@ -97,7 +83,7 @@ class ExpensesReportController extends Controller // template picker it has no concept of. $templatePath = PdfTemplateUtils::resolveView('reports', 'expenses'); - $pdf = Pdf::loadView($templatePath); + $pdf = Pdf::loadView($templatePath, [], PdfPageSetup::forReports()); if ($request->has('preview')) { return view($templatePath); diff --git a/app/Http/Controllers/Company/Report/ItemSalesReportController.php b/app/Http/Controllers/Company/Report/ItemSalesReportController.php index 33f1063c..d01f941c 100644 --- a/app/Http/Controllers/Company/Report/ItemSalesReportController.php +++ b/app/Http/Controllers/Company/Report/ItemSalesReportController.php @@ -8,6 +8,7 @@ use App\Models\Company; use App\Models\CompanySetting; use App\Models\Currency; use App\Models\InvoiceItem; +use App\Support\Pdf\PdfPageSetup; use App\Support\Pdf\PdfTemplateUtils; use Carbon\Carbon; use Illuminate\Http\JsonResponse; @@ -55,26 +56,11 @@ class ItemSalesReportController extends Controller $to_date = Carbon::createFromFormat('Y-m-d', $request->to_date)->translatedFormat($dateFormat); $currency = Currency::findOrFail(CompanySetting::getSetting('currency', $company->id)); - $colors = [ - 'primary_text_color', - 'heading_text_color', - 'section_heading_text_color', - 'border_color', - 'body_text_color', - 'footer_text_color', - 'footer_total_color', - 'footer_bg_color', - 'date_text_color', - ]; - $colorSettings = CompanySetting::whereIn('option', $colors) - ->whereCompany($company->id) - ->get(); - view()->share([ 'items' => $items, - 'colorSettings' => $colorSettings, 'totalAmount' => $totalAmount, 'company' => $company, + 'logo' => $company->logo_path, 'from_date' => $from_date, 'to_date' => $to_date, 'currency' => $currency, @@ -84,7 +70,7 @@ class ItemSalesReportController extends Controller // template picker it has no concept of. $templatePath = PdfTemplateUtils::resolveView('reports', 'sales-items'); - $pdf = Pdf::loadView($templatePath); + $pdf = Pdf::loadView($templatePath, [], PdfPageSetup::forReports()); if ($request->has('preview')) { return view($templatePath); diff --git a/app/Http/Controllers/Company/Report/ProfitLossReportController.php b/app/Http/Controllers/Company/Report/ProfitLossReportController.php index 3302a334..d13cff00 100644 --- a/app/Http/Controllers/Company/Report/ProfitLossReportController.php +++ b/app/Http/Controllers/Company/Report/ProfitLossReportController.php @@ -9,6 +9,7 @@ use App\Models\CompanySetting; use App\Models\Currency; use App\Models\Expense; use App\Models\Payment; +use App\Support\Pdf\PdfPageSetup; use App\Support\Pdf\PdfTemplateUtils; use Carbon\Carbon; use Illuminate\Http\JsonResponse; @@ -61,28 +62,12 @@ class ProfitLossReportController extends Controller $to_date = Carbon::createFromFormat('Y-m-d', $request->to_date)->translatedFormat($dateFormat); $currency = Currency::findOrFail(CompanySetting::getSetting('currency', $company->id)); - $colors = [ - 'primary_text_color', - 'heading_text_color', - 'section_heading_text_color', - 'border_color', - 'body_text_color', - 'footer_text_color', - 'footer_total_color', - 'footer_bg_color', - 'date_text_color', - ]; - $colorSettings = CompanySetting::whereIn('option', $colors) - ->whereCompany($company->id) - ->get(); - view()->share([ - 'company' => $company, 'income' => $paymentsAmount, 'expenseCategories' => $expenseCategories, 'totalExpense' => $totalAmount, - 'colorSettings' => $colorSettings, 'company' => $company, + 'logo' => $company->logo_path, 'from_date' => $from_date, 'to_date' => $to_date, 'currency' => $currency, @@ -92,7 +77,7 @@ class ProfitLossReportController extends Controller // template picker it has no concept of. $templatePath = PdfTemplateUtils::resolveView('reports', 'profit-loss'); - $pdf = Pdf::loadView($templatePath); + $pdf = Pdf::loadView($templatePath, [], PdfPageSetup::forReports()); if ($request->has('preview')) { return view($templatePath); diff --git a/app/Http/Controllers/Company/Report/TaxSummaryReportController.php b/app/Http/Controllers/Company/Report/TaxSummaryReportController.php index 551605cd..0b565710 100644 --- a/app/Http/Controllers/Company/Report/TaxSummaryReportController.php +++ b/app/Http/Controllers/Company/Report/TaxSummaryReportController.php @@ -8,6 +8,7 @@ use App\Models\Company; use App\Models\CompanySetting; use App\Models\Currency; use App\Models\Tax; +use App\Support\Pdf\PdfPageSetup; use App\Support\Pdf\PdfTemplateUtils; use Carbon\Carbon; use Illuminate\Http\JsonResponse; @@ -62,30 +63,14 @@ class TaxSummaryReportController extends Controller $to_date = Carbon::createFromFormat('Y-m-d', $request->to_date)->translatedFormat($dateFormat); $currency = Currency::findOrFail(CompanySetting::getSetting('currency', $company->id)); - $colors = [ - 'primary_text_color', - 'heading_text_color', - 'section_heading_text_color', - 'border_color', - 'body_text_color', - 'footer_text_color', - 'footer_total_color', - 'footer_bg_color', - 'date_text_color', - ]; - - $colorSettings = CompanySetting::whereIn('option', $colors) - ->whereCompany($company->id) - ->get(); - view()->share([ 'taxTypes' => $taxTypes, 'totalTaxAmount' => $totalAmount, 'expenseTaxTypes' => $expenseTaxTypes, 'totalExpenseTaxAmount' => $totalExpenseTaxAmount, 'netTaxAmount' => $netTaxAmount, - 'colorSettings' => $colorSettings, 'company' => $company, + 'logo' => $company->logo_path, 'from_date' => $from_date, 'to_date' => $to_date, 'currency' => $currency, @@ -96,7 +81,7 @@ class TaxSummaryReportController extends Controller // template picker it has no concept of. $templatePath = PdfTemplateUtils::resolveView('reports', 'tax-summary'); - $pdf = Pdf::loadView($templatePath); + $pdf = Pdf::loadView($templatePath, [], PdfPageSetup::forReports()); if ($request->has('preview')) { return view($templatePath); diff --git a/app/Support/Pdf/DompdfDriver.php b/app/Support/Pdf/DompdfDriver.php index 25f8deef..3c58ca99 100644 --- a/app/Support/Pdf/DompdfDriver.php +++ b/app/Support/Pdf/DompdfDriver.php @@ -15,9 +15,9 @@ use Illuminate\Support\Facades\App; */ class DompdfDriver implements PdfDriver { - public function loadView(string $template, array $metadata = []): ResponseStream + public function loadView(string $template, array $metadata = [], ?PdfPageSetup $page = null): ResponseStream { - $page = PdfPageSetup::fromConfig(); + $page ??= PdfPageSetup::fromConfig(); $html = $this->withPageMargins(view($template)->render(), $page); $html = $this->withDocumentTitle($html, $metadata['Title'] ?? null); diff --git a/app/Support/Pdf/GotenbergPdfDriver.php b/app/Support/Pdf/GotenbergPdfDriver.php index 8872845c..53f5edd1 100644 --- a/app/Support/Pdf/GotenbergPdfDriver.php +++ b/app/Support/Pdf/GotenbergPdfDriver.php @@ -12,9 +12,9 @@ use Psr\Http\Message\RequestInterface; class GotenbergPdfDriver implements PdfDriver { - public function loadView(string $template, array $metadata = []): ResponseStream + public function loadView(string $template, array $metadata = [], ?PdfPageSetup $page = null): ResponseStream { - return new GotenbergPdfResponse(Gotenberg::send($this->buildRequest($template, $metadata))); + return new GotenbergPdfResponse(Gotenberg::send($this->buildRequest($template, $metadata, $page))); } /** @@ -24,9 +24,9 @@ class GotenbergPdfDriver implements PdfDriver * below this line used to be inlined into loadView(), which meant the only * way to check that an option was set was to run a Gotenberg service. */ - public function buildRequest(string $template, array $metadata = []): RequestInterface + public function buildRequest(string $template, array $metadata = [], ?PdfPageSetup $page = null): RequestInterface { - $page = PdfPageSetup::fromConfig(); + $page ??= PdfPageSetup::fromConfig(); [$width, $height] = $page->gotenbergPaper(); [$marginTop, $marginBottom, $marginLeft, $marginRight] = $page->gotenbergMargins(); diff --git a/app/Support/Pdf/PdfDriver.php b/app/Support/Pdf/PdfDriver.php index e999b353..3d5230cb 100644 --- a/app/Support/Pdf/PdfDriver.php +++ b/app/Support/Pdf/PdfDriver.php @@ -9,6 +9,10 @@ interface PdfDriver * the file: Title, Author, Subject, * Keywords, Creator. Both drivers * accept the same key names. + * @param PdfPageSetup|null $page Page geometry to render at. Defaults to + * the configured one; the reports pass + * PdfPageSetup::forReports() because they + * carry no inset of their own. */ - public function loadView(string $template, array $metadata = []): ResponseStream; + public function loadView(string $template, array $metadata = [], ?PdfPageSetup $page = null): ResponseStream; } diff --git a/app/Support/Pdf/PdfPageSetup.php b/app/Support/Pdf/PdfPageSetup.php index 571983a1..1cc15ec4 100644 --- a/app/Support/Pdf/PdfPageSetup.php +++ b/app/Support/Pdf/PdfPageSetup.php @@ -13,6 +13,11 @@ namespace App\Support\Pdf; * nothing: the stock templates carry their own insets, and invoice2/estimate2 * are built around a header band that only reaches the paper edge at margin 0. * + * The reports are the exception, which is what forReports() is for. Their + * templates never carried an inset of their own and were drawn against dompdf's + * built-in 1.2cm, so they need a real page margin put back, from their own + * config key rather than the document one. + * * Dimensions are stored as CSS lengths because that is the only representation * both drivers take without loss. Gotenberg has no notion of named sizes, only * dimensions; dompdf accepts either a name from its own 66-entry table or a @@ -54,6 +59,45 @@ final class PdfPageSetup ); } + /** + * The configured page, but with the report margin on all four sides. + * + * Reports are the one family of templates that carries no inset of its own: + * they only set `.sub-container { padding: 0px 20px }` and relied on dompdf's + * built-in 1.2cm page margin, which stopped applying once DompdfDriver began + * injecting an @page rule from config. The document margins default to zero + * and must stay that way (invoice2 and estimate2 bleed a header band to the + * paper edge), so the reports get their own margin instead of inheriting + * those. Paper size and orientation still come from config: only the margins + * are overridden. + * + * A page margin rather than body padding because reports run to several + * pages, and padding only insets the first one. + */ + public static function forReports(): self + { + return self::fromConfig()->withUniformMargin(self::length('pdf.page.report_margin', '1.2cm')); + } + + /** + * A copy of this page with the same paper and orientation, and the given + * margin on all four sides. + */ + public function withUniformMargin(string $margin): self + { + $margin = self::assertLength($margin, "Invalid PDF page margin: \"{$margin}\"."); + + return new self( + width: $this->width, + height: $this->height, + orientation: $this->orientation, + marginTop: $margin, + marginRight: $margin, + marginBottom: $margin, + marginLeft: $margin, + ); + } + public function isLandscape(): bool { return $this->orientation === 'landscape'; @@ -137,9 +181,22 @@ final class PdfPageSetup $value = trim($value); + return self::assertLength($value, "Invalid PDF page length for {$key}: \"{$value}\"."); + } + + /** + * A bare 0 needs no unit; anything else is a number and one of the units + * both drivers understand. Shared by length() and withUniformMargin() so + * there is one definition of what a valid length is, with the caller + * supplying the part of the message that says where the bad value came from. + */ + private static function assertLength(string $value, string $context): string + { + $value = trim($value); + if (! preg_match('/^(0|\d+(\.\d+)?(pt|px|pc|mm|cm|in))$/', $value)) { throw new \InvalidArgumentException( - "Invalid PDF page length for {$key}: \"{$value}\". Expected 0, or a number and a unit, e.g. \"210mm\"." + $context.' Expected 0, or a number and a unit, e.g. "210mm".' ); } diff --git a/app/Support/Pdf/PdfService.php b/app/Support/Pdf/PdfService.php index fc4baf57..da36f275 100644 --- a/app/Support/Pdf/PdfService.php +++ b/app/Support/Pdf/PdfService.php @@ -4,10 +4,10 @@ namespace App\Support\Pdf; class PdfService { - public static function loadView(string $template, array $metadata = []): ResponseStream + public static function loadView(string $template, array $metadata = [], ?PdfPageSetup $page = null): ResponseStream { $driver = config('pdf.driver'); - return PdfDriverFactory::create($driver)->loadView($template, $metadata); + return PdfDriverFactory::create($driver)->loadView($template, $metadata, $page); } } diff --git a/app/Support/Pdf/PdfTemplateUtils.php b/app/Support/Pdf/PdfTemplateUtils.php index 26a4d011..dda0182d 100644 --- a/app/Support/Pdf/PdfTemplateUtils.php +++ b/app/Support/Pdf/PdfTemplateUtils.php @@ -209,4 +209,101 @@ class PdfTemplateUtils $contents ); } + + /** + * Where a custom template's own copies of its type's partials live, keyed by + * the built-in view name each one replaces. + * + * A per-template copy is what keeps two custom templates of the same type + * independent: they used to share one partials/table.blade.php, written once + * and then reused, so editing the table for one silently changed it for all + * of them. + * + * @return array + */ + public static function partialViewMap(string $templateType, string $templateName): array + { + $map = []; + + foreach (self::stockPartials($templateType) as $partial) { + $view = str_replace('/', '.', $partial); + + $map[sprintf('app.pdf.%s.partials.%s', $templateType, $view)] = sprintf( + 'pdf_templates::%s.partials.%s.%s', + $templateType, + $templateName, + $view, + ); + } + + return $map; + } + + /** + * Repoint the view names in some Blade markup. + * + * Keyed by view name rather than by directive on purpose: a partial is named + * by @include, by @extends and by several others, and a clone has to follow + * every one of them. + * + * @param array $map + */ + public static function rewriteViewReferences(string $markup, array $map): string + { + return Str::replace(array_keys($map), array_values($map), $markup); + } + + /** + * Give a custom template its own copy of every partial its type ships. + * + * References between partials are rewritten in the copies too, so a copied + * layout includes the copied stylesheet rather than the built-in one. + * Nesting of any depth is covered, because every copy is rewritten with the + * same map. Views outside the type's own partials directory, notably the + * cross-type app.pdf.partials.*, are deliberately left pointing at the + * built-ins: they are shared chrome, not part of the design being cloned. + */ + public static function copyTemplatePartials(string $templateType, string $templateName): void + { + $map = self::partialViewMap($templateType, $templateName); + + foreach (self::stockPartials($templateType) as $partial) { + $contents = Storage::disk('views')->get( + sprintf('/app/pdf/%s/partials/%s.blade.php', $templateType, $partial) + ); + + self::toCustomTemplateFile( + self::rewriteViewReferences($contents, $map), + $templateType, + sprintf('partials/%s/%s.blade.php', $templateName, $partial), + ); + } + } + + /** + * The partials a template type ships, as paths relative to its partials + * directory and without the .blade.php suffix. + * + * @return array + */ + private static function stockPartials(string $templateType): array + { + $directory = sprintf('app/pdf/%s/partials', $templateType); + + $partials = []; + + foreach (Storage::disk('views')->allFiles($directory) as $file) { + if (! Str::endsWith($file, '.blade.php')) { + continue; + } + + $partials[] = Str::before(Str::after($file, $directory.'/'), '.blade.php'); + } + + // Longest name first, so a partial whose name is a prefix of another + // one cannot rewrite the leading segment of the longer name. + usort($partials, fn (string $a, string $b) => strlen($b) <=> strlen($a)); + + return $partials; + } } diff --git a/app/Support/helpers.php b/app/Support/helpers.php index 51c5b121..77faec36 100644 --- a/app/Support/helpers.php +++ b/app/Support/helpers.php @@ -123,7 +123,17 @@ function getCustomFieldValueKey(string $type) } /** - * @return formated_money + * Format an amount given in cents as currency markup for PDF templates. + * + * The magnitude is formatted first and a single minus sign is prefixed to the + * whole assembled string, so a negative amount reads "-$24,738.00" rather than + * "$-24,738.00" and the symbol stays glued to the digits in both symbol + * positions. The symbol is wrapped in a DejaVu Sans span so it renders even + * when the active font has no glyph for it. + * + * @param int|float|string|null $money Amount in cents. + * @param Currency|null $currency Defaults to the company currency setting. + * @return string */ function format_money_pdf($money, $currency = null) { @@ -134,20 +144,24 @@ function format_money_pdf($money, $currency = null) } $format_money = number_format( - $money, + abs($money), $currency->precision, $currency->decimal_separator, $currency->thousand_separator ); - $currency_with_symbol = ''; - if ($currency->swap_currency_symbol) { - $currency_with_symbol = $format_money.''.$currency->symbol.''; - } else { - $currency_with_symbol = ''.$currency->symbol.''.$format_money; - } + $symbol = ''.$currency->symbol.''; - return $currency_with_symbol; + $currency_with_symbol = $currency->swap_currency_symbol + ? $format_money.$symbol + : $symbol.$format_money; + + // The sign is decided on the formatted digits, not on the raw input, so an + // amount that rounds away at the currency's precision (a stray cent on a + // zero-precision currency) renders as zero instead of "-0". + $is_negative = $money < 0 && preg_match('/[1-9]/', $format_money) === 1; + + return $is_negative ? '-'.$currency_with_symbol : $currency_with_symbol; } /** diff --git a/config/pdf.php b/config/pdf.php index 53d2b9d7..33baa170 100644 --- a/config/pdf.php +++ b/config/pdf.php @@ -46,6 +46,21 @@ return [ 'margin_bottom' => env('PDF_MARGIN_BOTTOM', '0'), 'margin_left' => env('PDF_MARGIN_LEFT', '0'), + /* + * The page margin used by the report templates only, applied on all four + * sides. Reports need their own knob because the document margins above + * default to zero for reasons that do not apply to them: invoice2 and + * estimate2 bleed a coloured header band to the paper edge, and the stock + * document templates carry their own 30px/50px insets. The report + * templates carry none at all, they were drawn against dompdf's built-in + * 1.2cm default, so at margin zero their content sits flush against the + * paper. Reports are also the most likely to run to several pages, and a + * page margin is the only inset that repeats on every one of them (body + * padding insets the first page only). Keeping it separate means changing + * the document margins does not silently reflow the reports. + */ + 'report_margin' => env('PDF_REPORT_MARGIN', '1.2cm'), + /* * Repeat "page / total" at the foot of every page. Gotenberg only: * Chromium repeats a footer template and substitutes the counts, and diff --git a/lang/en.json b/lang/en.json index 13bda1d7..54dd6deb 100644 --- a/lang/en.json +++ b/lang/en.json @@ -1907,16 +1907,16 @@ "pdf_payment_number": "Payment Number", "pdf_payment_mode": "Payment Mode", "pdf_payment_amount_received_label": "Amount Received", - "pdf_expense_report_label": "EXPENSES REPORT", - "pdf_total_expenses_label": "TOTAL EXPENSE", + "pdf_expense_report_label": "Expenses report", + "pdf_total_expenses_label": "Total expenses", "pdf_profit_loss_label": "PROFIT & LOSS REPORT", "pdf_sales_customers_label": "Sales Customer Report", "pdf_sales_items_label": "Sales Item Report", "pdf_tax_summery_label": "Tax Summary Report", - "pdf_income_label": "INCOME", - "pdf_net_profit_label": "NET PROFIT", + "pdf_income_label": "Income", + "pdf_net_profit_label": "Net profit", "pdf_customer_sales_report": "Sales Report: By Customer", - "pdf_total_sales_label": "TOTAL SALES", + "pdf_total_sales_label": "Total sales", "pdf_item_sales_label": "Sales Report: By Item", "pdf_tax_report_label": "TAX REPORT", "pdf_total_tax_label": "TOTAL TAX", @@ -1927,7 +1927,16 @@ "pdf_tax_refundable_label": "Tax Refundable", "pdf_tax_balance_label": "Tax Balance", "pdf_expenses_label": "Expenses", + "pdf_report_category_label": "Category", + "pdf_report_date_label": "Date", + "pdf_report_document_label": "Document", + "pdf_report_item_label": "Item", + "pdf_report_tax_type_label": "Tax type", + "pdf_report_no_records": "No records in this period", "pdf_expense_group_total_label": "Group total:", + "pdf_expense_date_label": "Date", + "pdf_expense_note_label": "Note", + "pdf_expense_amount_label": "Amount", "pdf_bill_to": "Bill to,", "pdf_ship_to": "Ship to,", "pdf_received_from": "Received from:", diff --git a/resources/views/app/pdf/reports/expenses.blade.php b/resources/views/app/pdf/reports/expenses.blade.php index cd5ae356..f9911fa9 100644 --- a/resources/views/app/pdf/reports/expenses.blade.php +++ b/resources/views/app/pdf/reports/expenses.blade.php @@ -1,249 +1,56 @@ - - +@extends('app.pdf.reports.partials.layout') - - @lang('pdf_expense_report_label') -@include("app.pdf.partials.fonts") +@section('report-title', __('pdf_expense_report_label')) +@section('footer-label', __('pdf_total_expenses_label')) - - - - - -
- - - - - - - - -
-

{{ $company->name }}

- 		-

{{ $from_date }} - {{ $to_date }}

-
-

@lang('pdf_expense_report_label')

-
-

@lang('pdf_expenses_label')

- @foreach ($expenseGroups as $group) -

{{ $group['name'] }}

- - - - - - - @foreach ($group['expenses'] as $expense) - - - - - - @endforeach -
@lang('expenses.date')@lang('expenses.note')@lang('expenses.amount')
{{ $expense->formatted_expense_date }}{{ $expense->notes ? $expense->notes : '-' }}{!! format_money_pdf($expense->base_amount, $currency) !!}
-
-

@lang('pdf_expense_group_total_label')   {!! format_money_pdf($group['total'], $currency) !!}

-
- @endforeach -
- - - - - -
-

@lang('pdf_total_expenses_label')

- 		-

{!! format_money_pdf($totalExpense, $currency) !!}

-
- - - +@section('report-body') + @forelse ($expenseGroups as $group) +
+

{{ $group['name'] }}

+ + + + + + + + + + @foreach ($group['expenses'] as $expense) + + + + + + @endforeach + + + + + +
@lang('pdf_expense_date_label')@lang('pdf_expense_note_label')@lang('pdf_expense_amount_label')
{{ $expense->formatted_expense_date }}{{ $expense->notes ?: '-' }}{!! format_money_pdf($expense->base_amount, $currency) !!}
@lang('pdf_total'){!! format_money_pdf($group['total'], $currency) !!}
+
+ @empty +
+

@lang('pdf_expenses_label')

+ + + + + + + + + + + + + +
@lang('pdf_expense_date_label')@lang('pdf_expense_note_label')@lang('pdf_expense_amount_label')
@lang('pdf_report_no_records')
+
+ @endforelse +@endsection diff --git a/resources/views/app/pdf/reports/partials/layout.blade.php b/resources/views/app/pdf/reports/partials/layout.blade.php new file mode 100644 index 00000000..05a25c1b --- /dev/null +++ b/resources/views/app/pdf/reports/partials/layout.blade.php @@ -0,0 +1,62 @@ +{{-- + The chrome shared by every report PDF: page skeleton, fonts, stylesheet, + branded header with the date range, and the grand-total band at the foot. + + A report extends this and fills four sections. Three of them are single + values and read best in the inline form: + + @extends('app.pdf.reports.partials.layout') + @section('report-title', __('pdf_profit_loss_label')) + @section('footer-label', __('pdf_net_profit_label')) + @section('footer-value', format_money_pdf($net, $currency)) + @section('report-body') ... @endsection + + Everything the header needs (company, from_date, to_date) is shared by the + report controller, so no report passes it down by hand. +--}} + + + + + @yield('report-title') + @include('app.pdf.partials.fonts') + @include('app.pdf.reports.partials.styles') + + + + + + + + +
+ @if (! empty($logo)) + + @else +

{{ $company->name }}

+ @endif + 		+

{{ $from_date }} - {{ $to_date }}

+
+ +

@yield('report-title')

+ + @yield('report-body') + +
+
+ + + + + +
+

@yield('footer-label')

+ 		+

@yield('footer-value')

+
+
+
+ + + diff --git a/resources/views/app/pdf/reports/partials/styles.blade.php b/resources/views/app/pdf/reports/partials/styles.blade.php new file mode 100644 index 00000000..35038cd2 --- /dev/null +++ b/resources/views/app/pdf/reports/partials/styles.blade.php @@ -0,0 +1,182 @@ +{{-- + The stylesheet every report PDF renders with. + + One content edge: PdfPageSetup::forReports() puts a 1.2cm margin on the + @page box, and that is the only outer inset in the whole document. Nothing + below adds a second one, so the header, the section headings, the table + columns, the total rules and the footer band all start on the same left + edge and end on the same right edge. + + Vertical rhythm is two steps: 24px between sections, 8px inside one. +--}} + diff --git a/resources/views/app/pdf/reports/profit-loss.blade.php b/resources/views/app/pdf/reports/profit-loss.blade.php index 6e732081..e63d8774 100644 --- a/resources/views/app/pdf/reports/profit-loss.blade.php +++ b/resources/views/app/pdf/reports/profit-loss.blade.php @@ -1,236 +1,47 @@ - - +@extends('app.pdf.reports.partials.layout') - - @lang('pdf_profit_loss_label') -@include("app.pdf.partials.fonts") +@section('report-title', __('pdf_profit_loss_label')) +@section('footer-label', __('pdf_net_profit_label')) - - - - - -
- +@section('report-body') +
+
- - - - - + +
-

{{ $company->name }}

- 		-

{{ $from_date }} - {{ $to_date }}

-
-

@lang('pdf_profit_loss_label')

- 		@lang('pdf_income_label'){!! format_money_pdf($income, $currency) !!}
- - - - - - -
-

@lang("pdf_income_label")

- 		-

{!! format_money_pdf($income, $currency) !!}

-
-

@lang('pdf_expenses_label')

-
- - @foreach ($expenseCategories as $expenseCategory) - - - - - @endforeach - -
-

- {{ $expenseCategory->category->name }} -

- 		-

- {!! format_money_pdf($expenseCategory->total_amount, $currency) !!} -

-
-
- - - - -
-

{!! format_money_pdf($totalExpense, $currency) !!}

-
- - - - - -
-

@lang("pdf_net_profit_label")

- 		-

{!! format_money_pdf($income - $totalExpense, $currency) !!}

-
- - - +
+

@lang('pdf_expenses_label')

+ + + + + + + + + @forelse ($expenseCategories as $expenseCategory) + + + + + @empty + + + + @endforelse + + + + + +
@lang('pdf_report_category_label')@lang('pdf_amount_label')
{{ $expenseCategory->category->name }}{!! format_money_pdf($expenseCategory->total_amount, $currency) !!}
@lang('pdf_report_no_records')
@lang('pdf_total_expenses_label'){!! format_money_pdf($totalExpense, $currency) !!}
+
+@endsection diff --git a/resources/views/app/pdf/reports/sales-customers.blade.php b/resources/views/app/pdf/reports/sales-customers.blade.php index 65be045d..3cd4bc85 100644 --- a/resources/views/app/pdf/reports/sales-customers.blade.php +++ b/resources/views/app/pdf/reports/sales-customers.blade.php @@ -1,211 +1,76 @@ - - +@extends('app.pdf.reports.partials.layout') - - @lang('pdf_sales_customers_label') -@include("app.pdf.partials.fonts") +@section('report-title', __('pdf_customer_sales_report')) +@section('footer-label', __('pdf_total_sales_label')) - - - - - -
- - - - - - - - -
-

{{ $company->name }}

- 		-

{{ $from_date }} - {{ $to_date }}

-
-

@lang('pdf_customer_sales_report')

-
- - @foreach ($customers as $customer) -

{{ $customer->name }}

-
- - @foreach ($customer->invoices as $invoice) - - - - - @endforeach + @forelse ($customersWithSales as $customer) +
+

{{ $customer->name }}

+
-

- {{ $invoice->formattedInvoiceDate }} ({{ $invoice->invoice_number }}) -

- 		-

- {!! format_money_pdf($invoice->base_total, $currency) !!} -

-
+ + + + + + + + + @foreach ($customer->invoices as $invoice) + + + {{-- A credit note is an invoice row of another type, and it stays in + the totals because a reversal netting the sale out is correct. + The tag is only so the line is not read as a sale. It reuses the + document's own label rather than a report-local key: every + pdf_*credit* key has to exist in the shipped locales, and that + one already does. --}} + + + + @endforeach + + + + +
@lang('pdf_report_date_label')@lang('pdf_report_document_label')@lang('pdf_amount_label')
{{ $invoice->formattedInvoiceDate }} + {{ $invoice->invoice_number }} + @if ($invoice->isCreditNote()) + @lang('pdf_credit_note_label') + @endif + {!! format_money_pdf($invoice->base_total, $currency) !!}
@lang('pdf_total'){!! format_money_pdf($customer->totalAmount, $currency) !!}
- - - - -
-

- {!! format_money_pdf($customer->totalAmount, $currency) !!} -

-
- @endforeach -
- - - - - - - -
-

@lang('pdf_total_sales_label')

- 		-

- {!! format_money_pdf($totalAmount, $currency) !!} -

-
- - - + @empty +
+ + + + + + + + + + + + + +
@lang('pdf_report_date_label')@lang('pdf_report_document_label')@lang('pdf_amount_label')
@lang('pdf_report_no_records')
+
+ @endforelse +@endsection diff --git a/resources/views/app/pdf/reports/sales-items.blade.php b/resources/views/app/pdf/reports/sales-items.blade.php index 16a13ec3..5345c734 100644 --- a/resources/views/app/pdf/reports/sales-items.blade.php +++ b/resources/views/app/pdf/reports/sales-items.blade.php @@ -1,210 +1,37 @@ - - +@extends('app.pdf.reports.partials.layout') - - @lang('pdf_sales_items_label') -@include("app.pdf.partials.fonts") +@section('report-title', __('pdf_item_sales_label')) +@section('footer-label', __('pdf_total_sales_label')) - - - - - -
- - - - - - - - -
-

{{ $company->name }}

- 		-

{{ $from_date }} - {{ $to_date }}

-
-

@lang('pdf_item_sales_label')

-
- -

@lang('pdf_items_label')

- @foreach ($items as $item) -
- +@section('report-body') + {{-- One table for every item, not one table per item: separate tables size + their columns independently, which is why the amounts used to wander. --}} +
+
+ - - + + + -
-

- {{ $item->name }} -

- 		-

- {!! format_money_pdf($item->total_amount, $currency) !!} -

- 		@lang('pdf_report_item_label')@lang('pdf_quantity_label')@lang('pdf_amount_label')
-
- @endforeach - - - - - + + + @forelse ($items as $item) + + + + + + @empty + + + + @endforelse +
-

- {!! format_money_pdf($totalAmount, $currency) !!} -

-
{{ $item->name }}{{ $item->total_quantity }}{!! format_money_pdf($item->total_amount, $currency) !!}
@lang('pdf_report_no_records')
- - - - - - - -
-

@lang('pdf_total_sales_label')

- 		-

- {!! format_money_pdf($totalAmount, $currency) !!} -

-
- - - +@endsection diff --git a/resources/views/app/pdf/reports/tax-summary.blade.php b/resources/views/app/pdf/reports/tax-summary.blade.php index 1c328190..e69f3e9c 100644 --- a/resources/views/app/pdf/reports/tax-summary.blade.php +++ b/resources/views/app/pdf/reports/tax-summary.blade.php @@ -1,249 +1,77 @@ - - +@extends('app.pdf.reports.partials.layout') - - @lang('pdf_tax_summery_label') -@include("app.pdf.partials.fonts") +@section('report-title', __('pdf_tax_report_label')) - - - - - -
- - - - - - - - -
-

- {{ $company->name }} -

- 		-

- {{ $from_date }} - {{ $to_date }} -

-
-

@lang('pdf_tax_report_label')

-
-

@lang('pdf_output_tax_label')

-
- - @foreach ($taxTypes as $tax) +@section('report-body') +
+

@lang('pdf_output_tax_label')

+
+ - - + + - @endforeach - -
-

- {{ $tax->taxType->name }} -

- 		-

- {!! format_money_pdf($tax->total_tax_amount, $currency) !!} -

- 		@lang('pdf_report_tax_type_label')@lang('pdf_amount_label')
-
- - - - - -
-

- {!! format_money_pdf($totalTaxAmount, $currency) !!} -

-
- -

@lang('pdf_input_tax_label')

-
- - @foreach ($expenseTaxTypes as $tax) - - - + + + @forelse ($taxTypes as $tax) + + + + + @empty + + + + @endforelse + + + - @endforeach - -
-

- {{ $tax->taxType->name }} -

- 		-

- {!! format_money_pdf($tax->total_tax_amount, $currency) !!} -

-
{{ $tax->taxType->name }}{!! format_money_pdf($tax->total_tax_amount, $currency) !!}
@lang('pdf_report_no_records')
@lang('pdf_total'){!! format_money_pdf($totalTaxAmount, $currency) !!}
-
- - - - - +
-

- {!! format_money_pdf($totalExpenseTaxAmount, $currency) !!} -

-
- - - - - -
-

- @if ($netTaxAmount > 0) - @lang('pdf_tax_payable_label') - @elseif ($netTaxAmount < 0) - @lang('pdf_tax_refundable_label') - @else - @lang('pdf_tax_balance_label') - @endif -

- 		-

- {!! format_money_pdf(abs($netTaxAmount), $currency) !!} -

-
- - - +
+

@lang('pdf_input_tax_label')

+ + + + + + + + + @forelse ($expenseTaxTypes as $tax) + + + + + @empty + + + + @endforelse + + + + + +
@lang('pdf_report_tax_type_label')@lang('pdf_amount_label')
{{ $tax->taxType->name }}{!! format_money_pdf($tax->total_tax_amount, $currency) !!}
@lang('pdf_report_no_records')
@lang('pdf_total'){!! format_money_pdf($totalExpenseTaxAmount, $currency) !!}
+
+@endsection diff --git a/tests/Feature/Pdf/CustomTemplatePartialsTest.php b/tests/Feature/Pdf/CustomTemplatePartialsTest.php new file mode 100644 index 00000000..ebaf4ed0 --- /dev/null +++ b/tests/Feature/Pdf/CustomTemplatePartialsTest.php @@ -0,0 +1,183 @@ + '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, ['*']); + + Storage::fake('pdf_templates'); + Storage::fake('public'); + + // The disk and the view namespace are registered separately, and only the + // disk is faked. Point the namespace at the same place so the cloned files + // are the ones Blade resolves. + View::addNamespace('pdf_templates', Storage::disk('pdf_templates')->path('')); + View::getFinder()->flush(); + + config(['pdf.driver' => 'dompdf']); + + $this->clone = function (string $type, string $name) { + Artisan::call('make:template', ['name' => $name, '--type' => $type]); + + View::getFinder()->flush(); + }; +}); + +function clonedTemplateFile(string $type, string $file): string +{ + return Storage::disk('pdf_templates')->path("{$type}/{$file}"); +} + +/** + * Read off disk rather than hardcoded, so a partial added to the reports design + * later is covered the moment it lands. + * + * @return array + */ +function stockPartialNames(string $type): array +{ + $files = glob(dirname(__DIR__, 3)."/resources/views/app/pdf/{$type}/partials/*.blade.php"); + + return array_values(array_map( + fn ($path) => basename($path, '.blade.php'), + $files ?: [] + )); +} + +test('a cloned report gets its own copy of every partial its type ships', function () { + ($this->clone)('reports', 'profit-loss'); + + $partials = stockPartialNames('reports'); + + expect($partials)->not->toBeEmpty(); + + foreach ($partials as $partial) { + expect(File::exists(clonedTemplateFile('reports', "partials/profit-loss/{$partial}.blade.php"))) + ->toBeTrue("partial {$partial} was not copied"); + } +}); + +test('the cloned report points at its own copies rather than at the built-ins', function () { + ($this->clone)('reports', 'profit-loss'); + + $markup = File::get(clonedTemplateFile('reports', 'profit-loss.blade.php')); + + expect($markup)->toContain('pdf_templates::reports.partials.profit-loss.layout') + ->and($markup)->not->toContain('app.pdf.reports'); +}); + +/** + * The layout includes the stylesheet, so the copies have to be rewritten too. + * Without that the copied layout still pulls in the built-in stylesheet and the + * "own copy" property is only skin deep: editing the copy changes nothing. + */ +test('a copied partial points at the copies of the partials it includes', function () { + ($this->clone)('reports', 'profit-loss'); + + $layout = File::get(clonedTemplateFile('reports', 'partials/profit-loss/layout.blade.php')); + + expect($layout)->toContain('pdf_templates::reports.partials.profit-loss.styles') + ->and($layout)->not->toContain('app.pdf.reports.partials.styles') + // app.pdf.partials.* is chrome shared by every type, not part of the + // design being cloned, so it keeps pointing at the built-in. + ->and($layout)->toContain('app.pdf.partials.fonts'); +}); + +test('a second clone of a different report gets separate copies', function () { + ($this->clone)('reports', 'profit-loss'); + ($this->clone)('reports', 'expenses'); + + foreach (['profit-loss', 'expenses'] as $report) { + expect(File::exists(clonedTemplateFile('reports', "partials/{$report}/layout.blade.php")))->toBeTrue(); + } + + $layout = File::get(clonedTemplateFile('reports', 'partials/expenses/layout.blade.php')); + + expect($layout)->toContain('pdf_templates::reports.partials.expenses.styles') + ->and($layout)->not->toContain('partials.profit-loss.'); +}); + +/** + * Keyed by template name, valued by route: the two sales reports are named + * sales-customers / sales-items but are served under /reports/sales/. + */ +dataset('cloned reports', [ + 'expenses' => ['expenses', 'expenses'], + 'profit-loss' => ['profit-loss', 'profit-loss'], + 'sales-customers' => ['sales-customers', 'sales/customers'], + 'sales-items' => ['sales-items', 'sales/items'], + 'tax-summary' => ['tax-summary', 'tax-summary'], +]); + +test('a cloned report renders a pdf', function (string $report, string $route) { + ($this->clone)('reports', $report); + + $response = get("/reports/{$route}/{$this->company->unique_hash}?from_date=2020-01-01&to_date=2030-12-31"); + + $response->assertOk(); + expect($response->getContent())->toStartWith('%PDF-'); +})->with('cloned reports'); + +/** + * A marker in the copy, because a clone that silently resolved back to the + * built-in layout would still emit a valid PDF and pass the assertion above. + */ +test('the cloned report renders through its own copied partials', function () { + ($this->clone)('reports', 'profit-loss'); + + $styles = clonedTemplateFile('reports', 'partials/profit-loss/styles.blade.php'); + File::put($styles, File::get($styles).''); + View::getFinder()->flush(); + + get("/reports/profit-loss/{$this->company->unique_hash}?from_date=2020-01-01&to_date=2030-12-31&preview=true") + ->assertOk() + ->assertSee('RENDERED BY THE CLONED STYLES', false); +}); + +/** + * The generalisation has to be a superset of the hardcoded table copy it + * replaced: invoice and estimate designs include partials/table.blade.php at a + * path custom templates in the wild already point at. + */ +test('a cloned invoice template still renders through its own copied table partial', function () { + ($this->clone)('invoice', 'branded'); + + expect(File::exists(clonedTemplateFile('invoice', 'partials/branded/table.blade.php')))->toBeTrue(); + + $table = clonedTemplateFile('invoice', 'partials/branded/table.blade.php'); + File::put($table, File::get($table).''); + View::getFinder()->flush(); + + $invoice = Invoice::factory()->hasItems(1)->create([ + 'company_id' => $this->company->id, + 'template_name' => 'branded', + ]); + + get("/invoices/pdf/{$invoice->unique_hash}?preview=true") + ->assertOk() + ->assertSee('RENDERED BY THE CLONED TABLE', false); +}); diff --git a/tests/Feature/Pdf/ReportContentTest.php b/tests/Feature/Pdf/ReportContentTest.php new file mode 100644 index 00000000..972f2cfa --- /dev/null +++ b/tests/Feature/Pdf/ReportContentTest.php @@ -0,0 +1,170 @@ + 'DatabaseSeeder', '--force' => true]); + Artisan::call('db:seed', ['--class' => 'DemoSeeder', '--force' => true]); + + $user = User::findOrFail(1); + $this->company = $user->companies()->firstOrFail(); + + $this->withHeaders(['company' => $this->company->id]); + Sanctum::actingAs($user, ['*']); +}); + +function reportPreview(string $report, string $companyHash, string $from, string $to): TestResponse +{ + return get("/reports/{$report}/{$companyHash}?from_date={$from}&to_date={$to}&preview=true"); +} + +function reportCustomer(int $companyId, string $name): Customer +{ + return Customer::factory()->create([ + 'company_id' => $companyId, + 'name' => $name, + ]); +} + +function reportCustomerInvoice(Customer $customer, string $date, array $attributes = []): Invoice +{ + return Invoice::factory()->create(array_merge([ + 'company_id' => $customer->company_id, + 'customer_id' => $customer->id, + 'invoice_date' => $date, + 'base_total' => 100000, + ], $attributes)); +} + +/** + * The drifted templates looped over an empty collection and then printed a total + * underneath it, so a period with nothing in it read as a table that had lost + * its rows rather than as a period with no records. + */ +test('a report with nothing in the period says so rather than showing a bare total', function (string $report) { + reportPreview($report, $this->company->unique_hash, '2019-01-01', '2019-01-31') + ->assertOk() + ->assertSee(__('pdf_report_no_records')); +})->with([ + 'sales/customers', + 'sales/items', + 'expenses', + 'tax-summary', + 'profit-loss', +]); + +/** + * The customer list and the invoices relation are narrowed to the period in two + * separate places in the controller, and only a customer with documents is worth + * a heading and a total. Asserted end to end so it holds however the two are + * wired. + */ +test('a customer with no documents in the period is left out of the customer sales report', function () { + $inRange = reportCustomer($this->company->id, 'In Range Trading'); + reportCustomerInvoice($inRange, '2026-01-15'); + + $outOfRange = reportCustomer($this->company->id, 'Out Of Range Trading'); + reportCustomerInvoice($outOfRange, '2025-01-15'); + + reportCustomer($this->company->id, 'Never Invoiced Trading'); + + reportPreview('sales/customers', $this->company->unique_hash, '2026-01-01', '2026-01-31') + ->assertOk() + ->assertSee('In Range Trading') + ->assertDontSee('Out Of Range Trading') + ->assertDontSee('Never Invoiced Trading'); +}); + +/** + * A credit note is an invoice row of another type, so the report listed it with + * a CN- number and a negative amount as though it were a sale. It stays in the + * totals, because a reversal netting the sale out is what the dashboard does + * too, but the line is tagged so it is not read as one. + */ +test('a credit note is tagged in the customer sales report and still nets out the period', function () { + $customer = reportCustomer($this->company->id, 'Reversal Holdings'); + reportCustomerInvoice($customer, '2026-01-10', ['base_total' => 500000]); + $creditNote = reportCustomerInvoice($customer, '2026-01-20', [ + 'type' => Invoice::TYPE_CREDIT_NOTE, + 'invoice_number' => 'CN-000042', + 'base_total' => -200000, + ]); + + reportPreview('sales/customers', $this->company->unique_hash, '2026-01-01', '2026-01-31') + ->assertOk() + ->assertSee($creditNote->invoice_number) + ->assertSee(__('pdf_credit_note_label')) + ->assertViewHas('totalAmount', 300000); +}); + +/** + * An ordinary sale carries no tag, so the one on the credit note means + * something. + */ +test('an ordinary sale carries no credit note tag', function () { + $customer = reportCustomer($this->company->id, 'Straightforward Supplies'); + reportCustomerInvoice($customer, '2026-01-10'); + + reportPreview('sales/customers', $this->company->unique_hash, '2026-01-01', '2026-01-31') + ->assertOk() + ->assertDontSee(__('pdf_credit_note_label')); +}); + +/** + * The item sales report used to emit a table per item, and separate tables size + * their columns on their own content, so no two rows lined up. + */ +test('the item sales report puts every item in one table', function () { + $customer = reportCustomer($this->company->id, 'Single Table Ltd'); + $invoice = reportCustomerInvoice($customer, '2026-01-15'); + + foreach (['Alpha Widget', 'Beta Widget', 'Gamma Widget'] as $name) { + InvoiceItem::factory()->create([ + 'company_id' => $this->company->id, + 'invoice_id' => $invoice->id, + 'name' => $name, + ]); + } + + $response = reportPreview('sales/items', $this->company->unique_hash, '2026-01-01', '2026-01-31'); + + $response->assertOk() + ->assertSee('Alpha Widget') + ->assertSee('Beta Widget') + ->assertSee('Gamma Widget') + // The quantity column is only there because itemAttributes() already + // sums it, so no report query had to grow to carry it. + ->assertSee(__('pdf_quantity_label')); + + expect(substr_count($response->getContent(), 'class="report-table"'))->toBe(1); +}); + +/** + * Every report gets the same branded header as the documents do. Four of the + * five never shared the logo path, so they fell back to printing the company + * name where the logo belongs. + */ +test('every report shares the company logo with its header', function (string $report) { + reportPreview($report, $this->company->unique_hash, '2026-01-01', '2026-01-31') + ->assertOk() + ->assertViewHas('logo'); +})->with([ + 'sales/customers', + 'sales/items', + 'expenses', + 'tax-summary', + 'profit-loss', +]); diff --git a/tests/Feature/Pdf/ReportPdfDownloadTest.php b/tests/Feature/Pdf/ReportPdfDownloadTest.php index f35e5b1a..009558a3 100644 --- a/tests/Feature/Pdf/ReportPdfDownloadTest.php +++ b/tests/Feature/Pdf/ReportPdfDownloadTest.php @@ -1,6 +1,10 @@ headers->get('content-disposition'))->toContain('attachment'); expect($response->getContent())->toStartWith('%PDF-'); })->with('reports'); + +/** + * The report templates carry no inset of their own and were drawn against + * dompdf's built-in 1.2cm margin, which stopped applying once the driver began + * injecting an @page rule from config (where documents deliberately default to + * 0). Every report route therefore has to ask for the report page explicitly, + * and a page margin rather than body padding: reports run to several pages, and + * only a page margin repeats on each one. + */ +test('every report route renders at the report page setup', function (string $report) { + config([ + 'pdf.page.margin_top' => '0', + 'pdf.page.margin_right' => '0', + 'pdf.page.margin_bottom' => '0', + 'pdf.page.margin_left' => '0', + 'pdf.page.report_margin' => '1.2cm', + ]); + + $stream = Mockery::mock(ResponseStream::class); + $stream->shouldReceive('stream')->andReturn(new Response('%PDF-')); + + Pdf::shouldReceive('loadView') + ->once() + // Compared on the margins rather than by identity: the setup is built + // per render, so no two calls ever share an instance. + ->withArgs(function (string $template, array $metadata, ?PdfPageSetup $page) { + return $page instanceof PdfPageSetup + && $page->marginCss() === PdfPageSetup::forReports()->marginCss() + && $page->marginCss() === '1.2cm 1.2cm 1.2cm 1.2cm'; + }) + ->andReturn($stream); + + get(reportUrl($report, $this->company->unique_hash))->assertOk(); +})->with('reports'); diff --git a/tests/Unit/FormatMoneyPdfTest.php b/tests/Unit/FormatMoneyPdfTest.php new file mode 100644 index 00000000..18ffae2f --- /dev/null +++ b/tests/Unit/FormatMoneyPdfTest.php @@ -0,0 +1,118 @@ + 'US Dollar', + 'code' => 'USD', + 'symbol' => '$', + 'precision' => 2, + 'thousand_separator' => ',', + 'decimal_separator' => '.', + 'swap_currency_symbol' => false, + ], $attributes)); +} + +function pdfSymbol(string $symbol = '$'): string +{ + return ''.$symbol.''; +} + +test('a positive amount keeps the symbol against the digits', function () { + expect(format_money_pdf(2473800, pdfCurrency())) + ->toBe(pdfSymbol().'24,738.00'); +}); + +test('a negative amount leads with the sign, ahead of the symbol', function () { + // The defect: this used to render "$-24,738.00". + expect(format_money_pdf(-2473800, pdfCurrency())) + ->toBe('-'.pdfSymbol().'24,738.00'); +}); + +test('a positive amount keeps the trailing symbol against the digits when swapped', function () { + expect(format_money_pdf(2473800, pdfCurrency(['swap_currency_symbol' => true]))) + ->toBe('24,738.00'.pdfSymbol()); +}); + +test('a negative amount leads with the sign when the symbol trails', function () { + expect(format_money_pdf(-2473800, pdfCurrency(['swap_currency_symbol' => true]))) + ->toBe('-24,738.00'.pdfSymbol()); +}); + +test('zero carries no sign', function (bool $swap) { + $currency = pdfCurrency(['swap_currency_symbol' => $swap]); + + $expected = $swap ? '0.00'.pdfSymbol() : pdfSymbol().'0.00'; + + expect(format_money_pdf(0, $currency))->toBe($expected) + ->and(format_money_pdf(-0, $currency))->toBe($expected) + ->and(format_money_pdf(-0.0, $currency))->toBe($expected); +})->with([true, false]); + +test('an amount that rounds away at the currency precision carries no sign', function (bool $swap) { + // Decided on the formatted digits, not on the raw value: a tenth of a cent + // is negative but prints as zero, and "-$0.00" is not a number anyone owes. + $currency = pdfCurrency(['swap_currency_symbol' => $swap]); + + $expected = $swap ? '0.00'.pdfSymbol() : pdfSymbol().'0.00'; + + expect(format_money_pdf(-0.4, $currency))->toBe($expected); +})->with([true, false]); + +test('a zero-precision currency drops the decimals and still signs correctly', function () { + $yen = pdfCurrency([ + 'name' => 'Japanese Yen', + 'code' => 'JPY', + 'symbol' => '¥', + 'precision' => 0, + ]); + + expect(format_money_pdf(2473800, $yen))->toBe(pdfSymbol('¥').'24,738') + ->and(format_money_pdf(-2473800, $yen))->toBe('-'.pdfSymbol('¥').'24,738') + // A single cent is below this currency's precision, so it is not a + // negative amount once formatted. + ->and(format_money_pdf(-1, $yen))->toBe(pdfSymbol('¥').'0'); +}); + +test('a comma decimal separator is unaffected by the sign', function () { + $euro = pdfCurrency([ + 'name' => 'Euro', + 'code' => 'EUR', + 'symbol' => '€', + 'thousand_separator' => '.', + 'decimal_separator' => ',', + 'swap_currency_symbol' => true, + ]); + + expect(format_money_pdf(2473800, $euro))->toBe('24.738,00'.pdfSymbol('€')) + ->and(format_money_pdf(-2473800, $euro))->toBe('-24.738,00'.pdfSymbol('€')) + ->and(format_money_pdf(-40, $euro))->toBe('-0,40'.pdfSymbol('€')) + ->and(format_money_pdf(0, $euro))->toBe('0,00'.pdfSymbol('€')); +}); + +test('the same input types the templates pass are still accepted', function () { + $currency = pdfCurrency(); + + expect(format_money_pdf(null, $currency))->toBe(pdfSymbol().'0.00') + ->and(format_money_pdf('-2473800', $currency))->toBe('-'.pdfSymbol().'24,738.00') + ->and(format_money_pdf(-2473800.0, $currency))->toBe('-'.pdfSymbol().'24,738.00') + ->and(format_money_pdf(-2473850.5, $currency))->toBe('-'.pdfSymbol().'24,738.51'); +}); + +test('the symbol keeps its DejaVu Sans span so it renders in any font', function () { + expect(format_money_pdf(-2473800, pdfCurrency())) + ->toContain('$') + // The sign sits outside the span, in the document font. + ->toStartWith('- '210mm', + 'pdf.page.paper_height' => '297mm', + 'pdf.page.orientation' => 'portrait', + 'pdf.page.margin_top' => '0', + 'pdf.page.margin_right' => '0', + 'pdf.page.margin_bottom' => '0', + 'pdf.page.margin_left' => '0', + 'pdf.page.report_margin' => '1.2cm', + ]); +}); + +test('reports render with the configured report margin on all four sides', function () { + config(['pdf.page.report_margin' => '2cm']); + + $page = PdfPageSetup::forReports(); + + expect($page->marginTop)->toBe('2cm') + ->and($page->marginRight)->toBe('2cm') + ->and($page->marginBottom)->toBe('2cm') + ->and($page->marginLeft)->toBe('2cm') + ->and($page->marginCss())->toBe('2cm 2cm 2cm 2cm'); +}); + +test('reports keep the configured paper size and orientation', function () { + config([ + 'pdf.page.paper_width' => '8.5in', + 'pdf.page.paper_height' => '14in', + 'pdf.page.orientation' => 'landscape', + ]); + + $page = PdfPageSetup::forReports(); + + expect($page->width)->toBe('8.5in') + ->and($page->height)->toBe('14in') + ->and($page->isLandscape())->toBeTrue(); +}); + +/** + * The whole point of the separate key: an operator who sets document margins to + * suit their own invoice template must not silently reflow every report too. + */ +test('the document margins do not leak into the report page', function () { + config([ + 'pdf.page.margin_top' => '40mm', + 'pdf.page.margin_right' => '30mm', + 'pdf.page.margin_bottom' => '20mm', + 'pdf.page.margin_left' => '10mm', + ]); + + expect(PdfPageSetup::forReports()->marginCss())->toBe('1.2cm 1.2cm 1.2cm 1.2cm') + ->and(PdfPageSetup::fromConfig()->marginCss())->toBe('40mm 30mm 20mm 10mm'); +}); + +test('a blank or unset report margin falls back to dompdf\'s old default', function ($value) { + config(['pdf.page.report_margin' => $value]); + + expect(PdfPageSetup::forReports()->marginTop)->toBe('1.2cm'); +})->with([ + 'unset' => [null], + 'blank' => [''], + 'whitespace' => [' '], +]); + +/** + * An env typo should fail loudly rather than render at some other size, and the + * message has to name the key so the fix is obvious. + */ +test('a malformed report margin is rejected', function () { + config(['pdf.page.report_margin' => '12']); + + expect(fn () => PdfPageSetup::forReports()) + ->toThrow(InvalidArgumentException::class, 'pdf.page.report_margin'); +}); + +test('withUniformMargin returns a new page and leaves the original alone', function () { + $original = PdfPageSetup::fromConfig(); + $inset = $original->withUniformMargin('15mm'); + + expect($inset)->not->toBe($original) + ->and($inset->marginCss())->toBe('15mm 15mm 15mm 15mm') + ->and($original->marginCss())->toBe('0 0 0 0') + ->and($inset->width)->toBe($original->width) + ->and($inset->height)->toBe($original->height) + ->and($inset->orientation)->toBe($original->orientation); +}); + +test('withUniformMargin rejects a value that is not a length', function () { + expect(fn () => PdfPageSetup::fromConfig()->withUniformMargin('wide')) + ->toThrow(InvalidArgumentException::class, 'Invalid PDF page margin'); +}); + +test('a bare zero is still an acceptable uniform margin', function () { + expect(PdfPageSetup::fromConfig()->withUniformMargin('0')->marginCss())->toBe('0 0 0 0'); +}); + +/** + * dompdf has no margin API, so the injected @page rule is the only thing that + * carries the margin. It has to be built from the page it was handed, not read + * back out of config, or passing one would be a no-op. + */ +test('dompdf injects the page setup it is given rather than the configured one', function () { + $method = new ReflectionMethod(DompdfDriver::class, 'withPageMargins'); + + $html = $method->invoke( + new DompdfDriver, + '', + PdfPageSetup::fromConfig()->withUniformMargin('1.2cm') + ); + + expect($html)->toContain('@page { margin: 1.2cm 1.2cm 1.2cm 1.2cm; }'); +}); + +/** + * And that loadView() actually threads the page through to that injection + * rather than reading config again, which the reflection test above cannot see. + * The wrapper is captured instead of asserting on the rendered bytes: dompdf + * stamps a CreationDate, so two renders differ whatever the margins are. + */ +test('dompdf renders the report page setup it is handed', function () { + $driver = new class extends DompdfDriver + { + public string $html = ''; + + protected function wrapper(): PDF + { + $pdf = Mockery::mock(PDF::class); + $pdf->shouldReceive('setPaper')->andReturnSelf(); + $pdf->shouldReceive('loadHTML')->andReturnUsing(function (string $html) use ($pdf) { + $this->html = $html; + + return $pdf; + }); + + return $pdf; + } + }; + + $driver->loadView('app.pdf.partials.fonts', [], PdfPageSetup::forReports()); + + expect($driver->html)->toContain('@page { margin: 1.2cm 1.2cm 1.2cm 1.2cm; }'); +}); + +test('gotenberg sends the margins of the page setup it is given', function () { + config(['pdf.connections.gotenberg.host' => 'http://gotenberg.example.com:3000']); + + $body = (string) (new GotenbergPdfDriver) + ->buildRequest('app.pdf.partials.fonts', [], PdfPageSetup::forReports()) + ->getBody(); + + expect($body)->toContain('marginTop') + ->toContain('1.2cm') + ->toContain('210mm'); +}); diff --git a/tests/Unit/PdfReportTranslationKeysTest.php b/tests/Unit/PdfReportTranslationKeysTest.php new file mode 100644 index 00000000..3e99fb19 --- /dev/null +++ b/tests/Unit/PdfReportTranslationKeysTest.php @@ -0,0 +1,54 @@ + + */ +function reportTemplateTranslationKeys(string $template): array +{ + preg_match_all( + '/(?:@lang|__|trans)\(\s*[\'"]([^\'"]+)[\'"]/', + file_get_contents(resource_path("views/app/pdf/reports/{$template}.blade.php")), + $matches + ); + + return array_values(array_unique($matches[1])); +} + +test('every translation key a report template uses exists in english', function (string $template) { + $english = json_decode(file_get_contents(lang_path('en.json')), true); + $keys = reportTemplateTranslationKeys($template); + + $missing = array_values(array_filter( + $keys, + // array_key_exists rather than toHaveKey: en.json has nested sections and + // dot notation would resolve a dotted key into one of them. + fn (string $key) => ! array_key_exists($key, $english) + )); + + expect($keys)->not->toBeEmpty() + ->and($missing)->toBe([]); +})->with([ + 'expenses', + 'profit-loss', + 'sales-customers', + 'sales-items', + 'tax-summary', +]); + +test('the expenses report headings are translated rather than printing their keys', function () { + $english = json_decode(file_get_contents(lang_path('en.json')), true); + + expect(reportTemplateTranslationKeys('expenses')) + ->toContain('pdf_expense_date_label') + ->toContain('pdf_expense_note_label') + ->toContain('pdf_expense_amount_label') + ->and($english['pdf_expense_date_label'])->toBe('Date') + ->and($english['pdf_expense_note_label'])->toBe('Note') + ->and($english['pdf_expense_amount_label'])->toBe('Amount'); +}); diff --git a/tests/Unit/PdfStockTemplatePageSetupTest.php b/tests/Unit/PdfStockTemplatePageSetupTest.php index 907621ba..290bfc2a 100644 --- a/tests/Unit/PdfStockTemplatePageSetupTest.php +++ b/tests/Unit/PdfStockTemplatePageSetupTest.php @@ -1,5 +1,7 @@ toContain('margin: 0px;') @@ -39,6 +58,57 @@ test('stock templates reset body margins without overriding page margins', funct 'reports/tax-summary', ]); +/** + * The five reports began as copies of one 2018 stylesheet and drifted apart. + * A migrated report contributes content only: chrome and rules come from the + * shared partials, so the copies cannot start diverging again. + */ +test('migrated reports carry no stylesheet of their own', function (string $template) { + $source = file_get_contents(resource_path("views/app/pdf/{$template}.blade.php")); + + expect($source)->toContain("@extends('app.pdf.reports.partials.layout')") + ->not->toContain('with([ + 'reports/expenses', + 'reports/profit-loss', + 'reports/sales-customers', + 'reports/sales-items', + 'reports/tax-summary', +]); + +/** + * What the drifted copies were full of: a property set twice in one rule, and + * `padding-top: 10px; padding-right: 30px; padding: 0px;`, where the shorthand + * silently cancels both longhands. + */ +test('the shared report stylesheet has no duplicate or self-cancelling declarations', function () { + $css = file_get_contents(resource_path('views/app/pdf/reports/partials/styles.blade.php')); + $css = preg_replace('#/\*.*?\*/#s', '', Str::between($css, '')); + + preg_match_all('/\{(?[^}]*)\}/', $css, $matches); + + foreach ($matches['declarations'] as $declarations) { + $properties = []; + + foreach (explode(';', $declarations) as $declaration) { + if (str_contains($declaration, ':')) { + $properties[] = trim(Str::before($declaration, ':')); + } + } + + expect($properties)->toEqual(array_unique($properties)); + + foreach (['padding', 'margin'] as $shorthand) { + $longhands = array_filter( + $properties, + fn (string $property) => str_starts_with($property, $shorthand.'-') + ); + + expect(in_array($shorthand, $properties, true) && $longhands !== [])->toBeFalse(); + } + } +}); + test('stock invoice and estimate headers never move above the printable content box', function (string $template) { $source = file_get_contents(resource_path("views/app/pdf/{$template}.blade.php")); $headerRules = stockTemplateCssRules($source, '.header-container');