From 9cc94c2e37de537c1ba80abc6480475fe136b6aa Mon Sep 17 00:00:00 2001 From: sdi Date: Sun, 9 Aug 2026 23:50:12 +0300 Subject: [PATCH 1/4] Allow to customize invoice filename via invoice template mechanism --- .../billing/invoice_template_playground.rb | 31 +++++++++--- app/admin/billing/invoice_templates.rb | 7 ++- app/assets/javascripts/template_playground.js | 31 ++++++++++-- .../api/rest/admin/invoices_controller.rb | 2 +- app/models/billing/invoice.rb | 4 -- app/models/billing/invoice_template.rb | 12 +++-- .../rest/admin/invoice_template_resource.rb | 2 +- .../billing_invoice/generate_document.rb | 25 ++++++++-- app/services/yeti_pdf/client.rb | 47 +++++++++++++++---- db/custom_seeds/invoice_template_example.rb | 3 ++ ..._filename_template_to_invoice_templates.rb | 17 +++++++ db/structure.sql | 4 +- .../rest/admin/api/invoice_template_spec.rb | 5 +- spec/factories/billing/invoice_templates.rb | 9 ++-- .../new_invoice_template_spec.rb | 3 ++ spec/models/billing/invoice_template_spec.rb | 21 +++++++-- spec/requests/api/rest/admin/invoices_spec.rb | 6 +-- spec/services/billing_invoice/approve_spec.rb | 2 +- .../billing_invoice/generate_document_spec.rb | 27 +++++++++-- spec/services/yeti_pdf/client_spec.rb | 39 ++++++++++++++- 20 files changed, 242 insertions(+), 55 deletions(-) create mode 100644 db/migrate/20260809120000_add_filename_template_to_invoice_templates.rb diff --git a/app/admin/billing/invoice_template_playground.rb b/app/admin/billing/invoice_template_playground.rb index 8e8f57526..a8ca98c98 100644 --- a/app/admin/billing/invoice_template_playground.rb +++ b/app/admin/billing/invoice_template_playground.rb @@ -14,26 +14,33 @@ return render(plain: 'Not authorized to read this invoice', status: 403) unless authorized?(:read, invoice) data = BillingInvoice::InvoiceData.call(invoice: invoice) - pdf = YetiPdf::Client.render_pdf(template: params[:template].to_s, data: data) - send_data pdf, type: 'application/pdf', disposition: 'inline' + result = YetiPdf::Client.render_pdf( + template: params[:template].to_s, + filename_template: params[:filename_template].presence, + data: data + ) + # The name the document would be filed under, for the editor to display — + # a header, so it rides along with the inline PDF the iframe is loading. + response.set_header('X-Rendered-Filename', ERB::Util.url_encode(result.filename.to_s)) + send_data result.pdf, type: 'application/pdf', disposition: 'inline' rescue ActiveRecord::RecordNotFound render plain: 'Invoice not found', status: 404 rescue YetiPdf::Client::Error => e render plain: e.message, status: :unprocessable_entity end - # GET ?template_id= -> the saved html_template, for the "Rollback" button. + # GET ?template_id= -> the saved templates, for the "Rollback" button. page_action :template, method: :get do template = Billing::InvoiceTemplate.find(params[:template_id]) - render json: { html_template: template.html_template.to_s } + render json: { html_template: template.html_template.to_s, filename_template: template.filename_template.to_s } rescue ActiveRecord::RecordNotFound - render json: { html_template: '' }, status: 404 + render json: { html_template: '', filename_template: '' }, status: 404 end - # PATCH -> persist the edited html_template back to the template. + # PATCH -> persist the edited templates back to the template record. page_action :save, method: :patch do template = Billing::InvoiceTemplate.find(params[:template_id]) - template.update!(html_template: params[:html_template].to_s) + template.update!(html_template: params[:html_template].to_s, filename_template: params[:filename_template].to_s) render json: { ok: true } rescue ActiveRecord::RecordNotFound render json: { error: 'Template not found' }, status: 404 @@ -66,6 +73,16 @@ end end + div class: 'tp-filename-row', + style: 'display:flex; gap:8px; align-items:center; margin-bottom:8px;' do + label 'Filename:', for: 'tp-filename' + input type: 'text', id: 'tp-filename', spellcheck: 'false', style: 'flex:1;', + value: template&.filename_template.to_s + # Filled from the preview response with the name yeti-pdf actually + # rendered — the template author sees the result, not just the source. + span '', id: 'tp-filename-preview', style: 'opacity:.75; white-space:nowrap;' + end + div class: 'tp-body' do div class: 'tp-editor-pane' do textarea template&.html_template.to_s, id: 'tp-template', class: 'tp-editor', spellcheck: 'false' diff --git a/app/admin/billing/invoice_templates.rb b/app/admin/billing/invoice_templates.rb index b990fba7e..cd3da2171 100644 --- a/app/admin/billing/invoice_templates.rb +++ b/app/admin/billing/invoice_templates.rb @@ -6,7 +6,7 @@ actions :all # :index,:create, :new, :destroy, :delete, :edit, :update before_action :left_sidebar! - permit_params :name, :html_template + permit_params :name, :html_template, :filename_template acts_as_export :id, :name, :created_at @@ -39,6 +39,10 @@ def find_resource f.semantic_errors *f.object.errors.attribute_names f.inputs form_title do f.input :name + f.input :filename_template, + hint: 'pongo2 template naming the generated file, rendered against the same data as the ' \ + 'HTML template. No extension — ".pdf" is added when the document is served. ' \ + 'Example: {{ invoice.reference }}_{{ invoice.start_date|strfdate:"%Y-%m" }}' f.input :html_template, as: :text, input_html: { rows: 24 } end f.actions @@ -48,6 +52,7 @@ def find_resource attributes_table do row :id row :name + row :filename_template row :created_at end diff --git a/app/assets/javascripts/template_playground.js b/app/assets/javascripts/template_playground.js index 5afa61867..e08de934a 100644 --- a/app/assets/javascripts/template_playground.js +++ b/app/assets/javascripts/template_playground.js @@ -11,6 +11,8 @@ $(document).ready(function () { var csrf = csrfEl ? csrfEl.content : ''; var invoiceSel = document.getElementById('tp-invoice'); var editor = document.getElementById('tp-template'); + var filenameInput = document.getElementById('tp-filename'); + var filenamePreview = document.getElementById('tp-filename-preview'); var iframe = document.getElementById('tp-pdf'); var errorBox = document.getElementById('tp-error'); var body = container.querySelector('.tp-body'); @@ -84,6 +86,14 @@ $(document).ready(function () { errorBox.style.display = 'none'; } + // The name the document would be stored under, as rendered by yeti-pdf + // (URL-encoded by the preview action so it survives the header). + function showFilename(encoded) { + if (!filenamePreview) return; + var name = encoded ? decodeURIComponent(encoded) : ''; + filenamePreview.textContent = name ? '→ ' + name + '.pdf' : ''; + } + // Render the current template against the selected invoice. A spinner is // shown over the previous PDF until the response arrives. var spinner = container.querySelector('.tp-spinner'); @@ -103,13 +113,19 @@ $(document).ready(function () { 'X-CSRF-Token': csrf, 'Accept': 'application/pdf' }, - body: JSON.stringify({ invoice_id: invoiceSel.value, template: templateValue() }) + body: JSON.stringify({ + invoice_id: invoiceSel.value, + template: templateValue(), + filename_template: filenameInput ? filenameInput.value : '' + }) }).then(function (resp) { if (resp.ok) { + showFilename(resp.headers.get('X-Rendered-Filename')); return resp.blob().then(function (blob) { iframe.src = URL.createObjectURL(blob); }); } + showFilename(''); return resp.text().then(function (t) { showError('Render failed: ' + t); }); }).catch(function (e) { showError('Request failed: ' + e.message); @@ -125,6 +141,7 @@ $(document).ready(function () { } invoiceSel.addEventListener('change', render); // immediate on invoice change + if (filenameInput) { filenameInput.addEventListener('input', scheduleRender); } if (cm) { cm.on('change', scheduleRender); } else { @@ -146,7 +163,11 @@ $(document).ready(function () { hideError(); fetch(templateUrl + '?template_id=' + encodeURIComponent(templateId), { headers: { 'Accept': 'application/json' } }) .then(function (r) { return r.json(); }) - .then(function (j) { setTemplate(j.html_template || ''); render(); }) + .then(function (j) { + setTemplate(j.html_template || ''); + if (filenameInput) { filenameInput.value = j.filename_template || ''; } + render(); + }) .catch(function (e) { showError('Reload failed: ' + e.message); }); }); @@ -157,7 +178,11 @@ $(document).ready(function () { fetch(saveUrl, { method: 'PATCH', headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrf, 'Accept': 'application/json' }, - body: JSON.stringify({ template_id: templateId, html_template: templateValue() }) + body: JSON.stringify({ + template_id: templateId, + html_template: templateValue(), + filename_template: filenameInput ? filenameInput.value : '' + }) }).then(function (r) { if (r.ok) { if (saveStatus) { diff --git a/app/controllers/api/rest/admin/invoices_controller.rb b/app/controllers/api/rest/admin/invoices_controller.rb index e68f6089b..4d9cdef87 100644 --- a/app/controllers/api/rest/admin/invoices_controller.rb +++ b/app/controllers/api/rest/admin/invoices_controller.rb @@ -5,6 +5,6 @@ def pdf doc = ::Billing::InvoiceDocument.find_by(invoice_id: params[:id]) return head 404 if doc.nil? || doc.pdf_data.blank? - send_data doc.pdf_data, filename: "invoice-#{params[:id]}.pdf" + send_data doc.pdf_data, filename: "#{doc.filename}.pdf" end end diff --git a/app/models/billing/invoice.rb b/app/models/billing/invoice.rb index e41a6a99b..b12237217 100644 --- a/app/models/billing/invoice.rb +++ b/app/models/billing/invoice.rb @@ -210,10 +210,6 @@ def regenerate_document end end - def file_name - "#{id}_#{start_date}_#{end_date}" - end - delegate :contacts_for_invoices, :invoice_period, to: :account def subject diff --git a/app/models/billing/invoice_template.rb b/app/models/billing/invoice_template.rb index 81abb6131..60e8c1c29 100644 --- a/app/models/billing/invoice_template.rb +++ b/app/models/billing/invoice_template.rb @@ -5,10 +5,11 @@ # Table name: billing.invoice_templates # Database name: primary # -# id :integer(4) not null, primary key -# html_template :text -# name :string not null -# created_at :timestamptz +# id :integer(4) not null, primary key +# filename_template :string default("invoice-{{invoice.reference}}"), not null +# html_template :text +# name :string not null +# created_at :timestamptz # # Indexes # @@ -19,6 +20,9 @@ class Billing::InvoiceTemplate < ApplicationRecord self.table_name = 'billing.invoice_templates' validates :name, presence: true, uniqueness: true validates :html_template, presence: true + # NOT NULL alone would still allow '', which yeti-pdf treats as "no filename + # template" and would leave the document with no name to store. + validates :filename_template, presence: true, length: { maximum: 255 } def display_name name diff --git a/app/resources/api/rest/admin/invoice_template_resource.rb b/app/resources/api/rest/admin/invoice_template_resource.rb index 9b93c16a5..dc81d47b3 100644 --- a/app/resources/api/rest/admin/invoice_template_resource.rb +++ b/app/resources/api/rest/admin/invoice_template_resource.rb @@ -3,7 +3,7 @@ class Api::Rest::Admin::InvoiceTemplateResource < ::BaseResource model_name 'Billing::InvoiceTemplate' - attributes :name, :html_template + attributes :name, :html_template, :filename_template paginator :paged diff --git a/app/services/billing_invoice/generate_document.rb b/app/services/billing_invoice/generate_document.rb index 57fbfeffc..fd0ec9053 100644 --- a/app/services/billing_invoice/generate_document.rb +++ b/app/services/billing_invoice/generate_document.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true # Generates the invoice PDF document: builds the raw invoice data payload, sends -# it with the account's html_template to the external yeti-pdf service, and -# stores the returned PDF on the InvoiceDocument. +# it with the account's html_template and filename_template to the external +# yeti-pdf service, and stores the returned PDF and rendered name on the +# InvoiceDocument. # # Any generation failure (no template, yeti-pdf not configured, yeti-pdf error) # is recorded on invoice.pdf_error and swallowed, so a rendering problem never @@ -22,6 +23,15 @@ def initialize(invoice_id) end end + # yeti-pdf rejects a filename template that renders to something unusable, + # so reaching this means it answered without the header at all — a version + # that predates filename_template support. + class FilenameMissing < Error + def initialize(invoice_id) + super("yeti-pdf returned no filename for invoice #{invoice_id}") + end + end + parameter :invoice, required: true def call @@ -39,12 +49,17 @@ def generate_document! raise PdfApiNotConfigured, invoice.id unless YetiPdf::Client.configured? data = InvoiceData.call(invoice: invoice) - pdf_data = YetiPdf::Client.render_pdf(template: template.html_template, data: data) + result = YetiPdf::Client.render_pdf( + template: template.html_template, + filename_template: template.filename_template, + data: data + ) + raise FilenameMissing, invoice.id if result.filename.blank? Billing::InvoiceDocument.create!( invoice: invoice, - filename: invoice.file_name.to_s, - pdf_data: pdf_data + filename: result.filename, + pdf_data: result.pdf ) end diff --git a/app/services/yeti_pdf/client.rb b/app/services/yeti_pdf/client.rb index 8bd304874..55704eba1 100644 --- a/app/services/yeti_pdf/client.rb +++ b/app/services/yeti_pdf/client.rb @@ -3,8 +3,9 @@ require 'httpx' # Client for the external yeti-pdf render service: POSTs a pongo2 template plus -# a JSON data payload and returns either the rendered PDF bytes (#render_pdf) -# or the merged HTML (#render_html — a cheap pongo2 merge with no PDF step). +# a JSON data payload and returns either the rendered PDF and the name to file +# it under (#render_pdf) or the merged HTML (#render_html — a cheap pongo2 +# merge with no PDF step). module YetiPdf class Client class Error < StandardError; end @@ -12,6 +13,12 @@ class Error < StandardError; end RENDER_PATH = '/v1/render' RENDER_HTML_PATH = '/v1/render/html' DEFAULT_TIMEOUT = 30 + PDF_EXTENSION = '.pdf' + + # What #render_pdf hands back: the document and the name to file it under, + # both produced by a single render request. #filename is nil unless a + # filename_template was sent. + Result = Struct.new(:pdf, :filename) class << self def render_pdf(...) @@ -29,33 +36,55 @@ def configured? end end - # @return [String] binary PDF bytes - def render_pdf(template:, data:, options: {}) - post(RENDER_PATH, template: template, data: data, options: options) + # Renders the document and, when filename_template is given, the name to + # store it under — one request, one data payload, both templates merged by + # yeti-pdf. A filename template that renders to something unusable (empty, + # a path separator, a control character) fails the whole request there, so + # a bad name never reaches us. + # + # @return [Result] the PDF bytes and the rendered filename (or nil) + def render_pdf(template:, data:, filename_template: nil, options: {}) + response = post(RENDER_PATH, template: template, data: data, options: options, + filename_template: filename_template) + Result.new(response.body.to_s, rendered_filename(response)) end # @return [String] the merged HTML (pre-PDF); useful for debugging/preview def render_html(template:, data:, options: {}) - post(RENDER_HTML_PATH, template: template, data: data, options: options) + post(RENDER_HTML_PATH, template: template, data: data, options: options).body.to_s end private - def post(path, template:, data:, options:) + def post(path, template:, data:, options:, filename_template: nil) cfg = YetiConfig.invoice&.pdf_api raise Error, 'invoice.pdf_api.base_url is not configured' if cfg&.base_url.blank? + payload = { template: template, data: data, options: options } + payload[:filename_template] = filename_template if filename_template.present? + proxy = proxy_for(cfg) http = proxy.apply(client(cfg)) - response = proxy.run { http.post(url(cfg, path), json: { template: template, data: data, options: options }) } + response = proxy.run { http.post(url(cfg, path), json: payload) } response.raise_for_status - response.body.to_s + response rescue HTTPX::HTTPError => e raise Error, "yeti-pdf returned HTTP #{e.status}: #{safe_body(e.response)}" rescue HTTPX::Error => e raise Error, "yeti-pdf request failed: #{e.message}" end + # yeti-pdf returns the rendered name in Content-Disposition with a ".pdf" + # extension (so the response is directly saveable); we store the base name + # and re-append the extension when serving. httpx parses both the quoted + # and the RFC 2231 filename*=utf-8'' forms. + def rendered_filename(response) + name = response.body.filename + return if name.blank? + + name.delete_suffix(PDF_EXTENSION) + end + def client(cfg) t = (cfg.timeout || DEFAULT_TIMEOUT).to_i # Rendering a large invoice can take yeti-pdf minutes, so the client must diff --git a/db/custom_seeds/invoice_template_example.rb b/db/custom_seeds/invoice_template_example.rb index e26edbe7e..b31d63f5f 100644 --- a/db/custom_seeds/invoice_template_example.rb +++ b/db/custom_seeds/invoice_template_example.rb @@ -9,4 +9,7 @@ # Created once; an existing row with the same name is left untouched. Billing::InvoiceTemplate.find_or_create_by!(name: 'Example (HTML)') do |template| template.html_template = File.read(Rails.root.join('db/custom_seeds/invoice_template_example.html')) + # Rendered by yeti-pdf alongside the document itself; no extension, ".pdf" is + # appended when the document is served. + template.filename_template = 'invoice-{{invoice.reference}}-{{ invoice.end_date|strfdate:"%Y-%m" }}' end diff --git a/db/migrate/20260809120000_add_filename_template_to_invoice_templates.rb b/db/migrate/20260809120000_add_filename_template_to_invoice_templates.rb new file mode 100644 index 000000000..529594d5b --- /dev/null +++ b/db/migrate/20260809120000_add_filename_template_to_invoice_templates.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddFilenameTemplateToInvoiceTemplates < ActiveRecord::Migration[7.2] + def up + execute %q{ + ALTER TABLE billing.invoice_templates + ADD COLUMN filename_template character varying NOT NULL + DEFAULT 'invoice-{{invoice.reference}}'; + } + end + + def down + execute %q{ + ALTER TABLE billing.invoice_templates DROP COLUMN filename_template; + } + end +end diff --git a/db/structure.sql b/db/structure.sql index 684f1a31f..5d1265d8a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11336,7 +11336,8 @@ CREATE TABLE billing.invoice_templates ( id integer NOT NULL, name character varying NOT NULL, created_at timestamp with time zone, - html_template text + html_template text, + filename_template character varying DEFAULT 'invoice-{{invoice.reference}}'::character varying NOT NULL ); @@ -20794,6 +20795,7 @@ ALTER TABLE ONLY sys.sensors SET search_path TO gui, public, switch, billing, class4, runtime_stats, sys, logs, data_import; INSERT INTO "public"."schema_migrations" (version) VALUES +('20260809120000'), ('20260729120000'), ('20260719120000'), ('20260705140000'), diff --git a/spec/acceptance/rest/admin/api/invoice_template_spec.rb b/spec/acceptance/rest/admin/api/invoice_template_spec.rb index 3501db782..b910d5565 100644 --- a/spec/acceptance/rest/admin/api/invoice_template_spec.rb +++ b/spec/acceptance/rest/admin/api/invoice_template_spec.rb @@ -31,11 +31,14 @@ # html_template dasherizes to the "html-template" param, whose value # rspec_api_documentation looks up by that (dashed) name; point it back at # the underscored let so the value is actually sent (otherwise it arrives - # blank and the required validation returns 422). + # blank and the required validation returns 422). Same for + # filename_template. jsonapi_attribute(:html_template, required: true, method: :html_template) + jsonapi_attribute(:filename_template, required: true, method: :filename_template) let(:name) { 'Daily' } let(:html_template) { '

{{ invoice.reference }}

' } + let(:filename_template) { 'invoice-{{invoice.reference}}' } example_request 'create new entry' do expect(status).to eq(201) diff --git a/spec/factories/billing/invoice_templates.rb b/spec/factories/billing/invoice_templates.rb index f961acf0c..f010e354c 100644 --- a/spec/factories/billing/invoice_templates.rb +++ b/spec/factories/billing/invoice_templates.rb @@ -5,10 +5,11 @@ # Table name: billing.invoice_templates # Database name: primary # -# id :integer(4) not null, primary key -# html_template :text -# name :string not null -# created_at :timestamptz +# id :integer(4) not null, primary key +# filename_template :string default("invoice-{{invoice.reference}}"), not null +# html_template :text +# name :string not null +# created_at :timestamptz # # Indexes # diff --git a/spec/features/billing/invoice_templates/new_invoice_template_spec.rb b/spec/features/billing/invoice_templates/new_invoice_template_spec.rb index cf5a98f28..8d15c2b5b 100644 --- a/spec/features/billing/invoice_templates/new_invoice_template_spec.rb +++ b/spec/features/billing/invoice_templates/new_invoice_template_spec.rb @@ -9,9 +9,11 @@ include_context :fill_form, 'new_billing_invoice_template' do let(:html_template) { '{{ invoice.reference }}' } + let(:filename_template) { 'invoice-{{invoice.reference}}' } let(:attributes) do { name: 'new template', + filename_template: filename_template, html_template: html_template } end @@ -22,6 +24,7 @@ expect(Billing::InvoiceTemplate.last).to have_attributes( name: attributes[:name], + filename_template: filename_template, html_template: html_template ) end diff --git a/spec/models/billing/invoice_template_spec.rb b/spec/models/billing/invoice_template_spec.rb index 60bd9b6e2..bb10f2fa6 100644 --- a/spec/models/billing/invoice_template_spec.rb +++ b/spec/models/billing/invoice_template_spec.rb @@ -5,10 +5,11 @@ # Table name: billing.invoice_templates # Database name: primary # -# id :integer(4) not null, primary key -# html_template :text -# name :string not null -# created_at :timestamptz +# id :integer(4) not null, primary key +# filename_template :string default("invoice-{{invoice.reference}}"), not null +# html_template :text +# name :string not null +# created_at :timestamptz # # Indexes # @@ -31,4 +32,16 @@ expect(template).not_to be_valid expect(template.errors[:name]).to be_present end + + it 'defaults filename_template so a new template always names its documents' do + expect(described_class.new.filename_template).to eq('invoice-{{invoice.reference}}') + end + + # NOT NULL alone would let '' through, which yeti-pdf reads as "no filename + # template" — the document would come back nameless. + it 'is invalid with a blank filename_template' do + template = described_class.new(name: 'blank-name', html_template: '

x

', filename_template: '') + expect(template).not_to be_valid + expect(template.errors[:filename_template]).to be_present + end end diff --git a/spec/requests/api/rest/admin/invoices_spec.rb b/spec/requests/api/rest/admin/invoices_spec.rb index b49510d3c..92c40195b 100644 --- a/spec/requests/api/rest/admin/invoices_spec.rb +++ b/spec/requests/api/rest/admin/invoices_spec.rb @@ -886,14 +886,14 @@ let!(:contractor) { create(:customer) } let!(:account) { create(:account, contractor:) } let!(:invoice) { create(:invoice, account:) } - let!(:invoice_document) { create(:invoice_document, :filled, invoice:) } + let!(:invoice_document) { create(:invoice_document, :filled, invoice:, filename: 'INV-1_2020-01') } - it 'responds with pdf file' do + it 'responds with pdf file named after the stored document filename' do subject expect(response.status).to eq(200) expect(response.headers['Content-Transfer-Encoding']).to eq('binary') expect(response.headers['Content-Type']).to eq('application/pdf') - expect(response.headers['Content-Disposition']).to include("attachment; filename=\"invoice-#{invoice.id}.pdf\"") + expect(response.headers['Content-Disposition']).to include('attachment; filename="INV-1_2020-01.pdf"') expect(response.body).to match(invoice_document.pdf_data) end diff --git a/spec/services/billing_invoice/approve_spec.rb b/spec/services/billing_invoice/approve_spec.rb index ef5ce1261..5670157ef 100644 --- a/spec/services/billing_invoice/approve_spec.rb +++ b/spec/services/billing_invoice/approve_spec.rb @@ -11,7 +11,7 @@ let!(:account) { FactoryBot.create(:account, contractor:, send_invoices_to: [contact.id]) } let(:invoice_attrs) { { account:, contractor: } } let!(:invoice) { FactoryBot.create(:invoice, :pending, invoice_attrs) } - let(:invoice_document_attrs) { { invoice:, filename: "#{invoice.id}_#{invoice.start_date}_#{invoice.end_date}" } } + let(:invoice_document_attrs) { { invoice:, filename: "invoice-#{invoice.reference}" } } let!(:invoice_document) { FactoryBot.create(:invoice_document, :filled, invoice_document_attrs) } before { FactoryBot.create(:smtp_connection, global: true) } diff --git a/spec/services/billing_invoice/generate_document_spec.rb b/spec/services/billing_invoice/generate_document_spec.rb index ede85c1cf..c2977d898 100644 --- a/spec/services/billing_invoice/generate_document_spec.rb +++ b/spec/services/billing_invoice/generate_document_spec.rb @@ -21,28 +21,31 @@ context 'when yeti-pdf is configured and succeeds' do let(:pdf_bytes) { '%PDF-1.7 rendered' } + let(:rendered_filename) { 'INV-1_2020-01' } before do allow(YetiPdf::Client).to receive(:configured?).and_return(true) - allow(YetiPdf::Client).to receive(:render_pdf).and_return(pdf_bytes) + allow(YetiPdf::Client).to receive(:render_pdf) + .and_return(YetiPdf::Client::Result.new(pdf_bytes, rendered_filename)) end - it 'stores the pdf on a new invoice document' do + it 'stores the pdf and the name yeti-pdf rendered on a new invoice document' do expect { subject }.to change { Billing::InvoiceDocument.count }.by(1) doc = Billing::InvoiceDocument.last! expect(doc).to have_attributes( invoice: invoice, - filename: invoice.file_name.to_s, + filename: rendered_filename, pdf_data: pdf_bytes ) end - it 'sends the html_template and the nested raw data payload to the client' do + it 'sends both templates and the nested raw data payload in one request' do subject expect(YetiPdf::Client).to have_received(:render_pdf).with( template: '

{{ invoice.reference }}

', + filename_template: invoice_template.filename_template, data: hash_including(:account, :contractor, :invoice) - ) + ).once end it 'clears a previously recorded pdf_error' do @@ -52,6 +55,20 @@ end end + context 'when yeti-pdf answers without a filename' do + before do + allow(YetiPdf::Client).to receive(:configured?).and_return(true) + allow(YetiPdf::Client).to receive(:render_pdf) + .and_return(YetiPdf::Client::Result.new('%PDF-1.7', nil)) + end + + it 'records the error rather than storing a nameless document' do + subject + expect(invoice.reload.pdf_error).to match(/returned no filename/) + expect(Billing::InvoiceDocument.count).to eq(0) + end + end + context 'when yeti-pdf returns an error' do before do allow(YetiPdf::Client).to receive(:configured?).and_return(true) diff --git a/spec/services/yeti_pdf/client_spec.rb b/spec/services/yeti_pdf/client_spec.rb index df44b53de..faed48677 100644 --- a/spec/services/yeti_pdf/client_spec.rb +++ b/spec/services/yeti_pdf/client_spec.rb @@ -23,13 +23,50 @@ stub = stub_request(:post, render_url) .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }, body: '%PDF-bytes') - expect(subject).to eq('%PDF-bytes') + expect(subject).to have_attributes(pdf: '%PDF-bytes', filename: nil) expect(stub).to have_been_requested expect(WebMock).to have_requested(:post, render_url) .with(headers: { 'Content-Type' => %r{application/json} }, body: hash_including('template' => template, 'options' => {})) end + it 'omits filename_template when none is given' do + stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') + + subject + expect(WebMock).to have_requested(:post, render_url) + .with { |req| !JSON.parse(req.body).key?('filename_template') } + end + + context 'with a filename_template' do + subject do + described_class.render_pdf(template: template, data: data, filename_template: '{{ invoice.reference }}') + end + + it 'sends it and returns the rendered name without the extension' do + stub_request(:post, render_url).to_return( + status: 200, + headers: { 'Content-Type' => 'application/pdf', + 'Content-Disposition' => 'attachment; filename="INV-1.pdf"' }, + body: '%PDF-bytes' + ) + + expect(subject).to have_attributes(pdf: '%PDF-bytes', filename: 'INV-1') + expect(WebMock).to have_requested(:post, render_url) + .with(body: hash_including('filename_template' => '{{ invoice.reference }}')) + end + + it 'decodes an RFC 2231 encoded name' do + stub_request(:post, render_url).to_return( + status: 200, + headers: { 'Content-Disposition' => "attachment; filename*=utf-8''Acm%C3%A9%20%26%20Co.pdf" }, + body: '%PDF-bytes' + ) + + expect(subject.filename).to eq('Acmé & Co') + end + end + it 'raises Error with the status on a non-2xx response' do stub_request(:post, render_url).to_return(status: 422, body: '{"error":"template_error"}') From 719b58073a2fe6f51476c5eac5ee9afbc1c2ea51 Mon Sep 17 00:00:00 2001 From: sdi Date: Mon, 10 Aug 2026 01:00:43 +0300 Subject: [PATCH 2/4] fix --- .../billing/invoice_template_playground.rb | 5 +++- app/services/yeti_pdf/client.rb | 23 ++++++++++++---- spec/services/yeti_pdf/client_spec.rb | 27 ++++++++++++++++--- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/app/admin/billing/invoice_template_playground.rb b/app/admin/billing/invoice_template_playground.rb index a8ca98c98..5e59876f7 100644 --- a/app/admin/billing/invoice_template_playground.rb +++ b/app/admin/billing/invoice_template_playground.rb @@ -16,7 +16,10 @@ data = BillingInvoice::InvoiceData.call(invoice: invoice) result = YetiPdf::Client.render_pdf( template: params[:template].to_s, - filename_template: params[:filename_template].presence, + # Sent even when empty, so clearing the field surfaces yeti-pdf's + # validation error here instead of silently previewing a nameless + # document that could never be generated for real. + filename_template: params[:filename_template].to_s, data: data ) # The name the document would be filed under, for the editor to display — diff --git a/app/services/yeti_pdf/client.rb b/app/services/yeti_pdf/client.rb index 55704eba1..ba86c5dfa 100644 --- a/app/services/yeti_pdf/client.rb +++ b/app/services/yeti_pdf/client.rb @@ -36,17 +36,27 @@ def configured? end end - # Renders the document and, when filename_template is given, the name to + # Renders the document and, when a filename_template is given, the name to # store it under — one request, one data payload, both templates merged by # yeti-pdf. A filename template that renders to something unusable (empty, # a path separator, a control character) fails the whole request there, so # a bad name never reaches us. # - # @return [Result] the PDF bytes and the rendered filename (or nil) - def render_pdf(template:, data:, filename_template: nil, options: {}) + # filename_template is required so callers state which they want, and nil + # differs from '': nil asks for no name at all, while '' is a request for a + # name with a broken template and is rejected by yeti-pdf rather than + # silently ignored. + # + # @return [Result] the PDF bytes and the rendered filename (nil if none was + # asked for) + def render_pdf(template:, data:, filename_template:, options: {}) response = post(RENDER_PATH, template: template, data: data, options: options, filename_template: filename_template) - Result.new(response.body.to_s, rendered_filename(response)) + # Only a caller that asked for a name gets one back: the response header + # is read solely when we sent a template for it, so the result never + # depends on what yeti-pdf might name a document by default. + filename = rendered_filename(response) unless filename_template.nil? + Result.new(response.body.to_s, filename) end # @return [String] the merged HTML (pre-PDF); useful for debugging/preview @@ -61,7 +71,10 @@ def post(path, template:, data:, options:, filename_template: nil) raise Error, 'invoice.pdf_api.base_url is not configured' if cfg&.base_url.blank? payload = { template: template, data: data, options: options } - payload[:filename_template] = filename_template if filename_template.present? + # Sent verbatim when given, including '' — yeti-pdf tells absent (no name + # wanted) from empty (a name wanted, template broken) by the key's + # presence, so blank must not be dropped here. + payload[:filename_template] = filename_template unless filename_template.nil? proxy = proxy_for(cfg) http = proxy.apply(client(cfg)) diff --git a/spec/services/yeti_pdf/client_spec.rb b/spec/services/yeti_pdf/client_spec.rb index faed48677..0d717e5cd 100644 --- a/spec/services/yeti_pdf/client_spec.rb +++ b/spec/services/yeti_pdf/client_spec.rb @@ -17,7 +17,7 @@ end describe '.render_pdf' do - subject { described_class.render_pdf(template: template, data: data) } + subject { described_class.render_pdf(template: template, data: data, filename_template: nil) } it 'posts the template and data and returns the pdf bytes' do stub = stub_request(:post, render_url) @@ -30,7 +30,7 @@ body: hash_including('template' => template, 'options' => {})) end - it 'omits filename_template when none is given' do + it 'omits filename_template when nil, so no name is asked for' do stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') subject @@ -38,6 +38,27 @@ .with { |req| !JSON.parse(req.body).key?('filename_template') } end + # Blank must reach yeti-pdf: it distinguishes "no name wanted" (key absent) + # from "name wanted, template broken" (key present but empty) and rejects + # the latter, which is how the playground surfaces a cleared field. + it 'sends an empty filename_template rather than dropping it' do + stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') + + described_class.render_pdf(template: template, data: data, filename_template: '') + expect(WebMock).to have_requested(:post, render_url) + .with { |req| JSON.parse(req.body)['filename_template'] == '' } + end + + it 'ignores a Content-Disposition when no filename was requested' do + stub_request(:post, render_url).to_return( + status: 200, + headers: { 'Content-Disposition' => 'attachment; filename="whatever.pdf"' }, + body: '%PDF-bytes' + ) + + expect(subject.filename).to be_nil + end + context 'with a filename_template' do subject do described_class.render_pdf(template: template, data: data, filename_template: '{{ invoice.reference }}') @@ -92,7 +113,7 @@ before { allow(YetiConfig).to receive(:invoice).and_return(nil) } it 'raises a clear Error' do - expect { described_class.render_pdf(template: template, data: data) } + expect { described_class.render_pdf(template: template, data: data, filename_template: nil) } .to raise_error(YetiPdf::Client::Error, /not configured/) end end From 3feb28c9f970b409b2f5e54e658e4e3864db693e Mon Sep 17 00:00:00 2001 From: sdi Date: Mon, 10 Aug 2026 10:52:01 +0300 Subject: [PATCH 3/4] fixes --- .../billing/invoice_template_playground.rb | 11 ++- app/services/yeti_pdf/client.rb | 37 +++----- .../invoice_template_playground_spec.rb | 57 ++++++++++++ spec/services/yeti_pdf/client_spec.rb | 86 +++++++------------ 4 files changed, 110 insertions(+), 81 deletions(-) create mode 100644 spec/requests/invoice_template_playground_spec.rb diff --git a/app/admin/billing/invoice_template_playground.rb b/app/admin/billing/invoice_template_playground.rb index 5e59876f7..67a61b955 100644 --- a/app/admin/billing/invoice_template_playground.rb +++ b/app/admin/billing/invoice_template_playground.rb @@ -13,12 +13,17 @@ invoice = Billing::Invoice.find(params[:invoice_id]) return render(plain: 'Not authorized to read this invoice', status: 403) unless authorized?(:read, invoice) + # A blank filename template is invalid here (the column is NOT NULL and the + # model requires it), so it is caught locally — previewing a document that + # could never be generated for real would be misleading, and there is + # nothing to ask yeti-pdf about. + if params[:filename_template].blank? + return render(plain: 'Filename template must not be blank', status: :unprocessable_entity) + end + data = BillingInvoice::InvoiceData.call(invoice: invoice) result = YetiPdf::Client.render_pdf( template: params[:template].to_s, - # Sent even when empty, so clearing the field surfaces yeti-pdf's - # validation error here instead of silently previewing a nameless - # document that could never be generated for real. filename_template: params[:filename_template].to_s, data: data ) diff --git a/app/services/yeti_pdf/client.rb b/app/services/yeti_pdf/client.rb index ba86c5dfa..12d0cd646 100644 --- a/app/services/yeti_pdf/client.rb +++ b/app/services/yeti_pdf/client.rb @@ -36,27 +36,21 @@ def configured? end end - # Renders the document and, when a filename_template is given, the name to - # store it under — one request, one data payload, both templates merged by - # yeti-pdf. A filename template that renders to something unusable (empty, - # a path separator, a control character) fails the whole request there, so - # a bad name never reaches us. + # Renders the document and the name to store it under — one request, one + # data payload, both templates merged by yeti-pdf. A filename template that + # renders to something unusable (empty, a path separator, a control + # character) fails the whole request there, so a bad name never reaches us. # - # filename_template is required so callers state which they want, and nil - # differs from '': nil asks for no name at all, while '' is a request for a - # name with a broken template and is rejected by yeti-pdf rather than - # silently ignored. + # filename_template is required: yeti-web always names its documents, so + # every Content-Disposition on the response is a name we asked for. The + # endpoint itself allows the field to be omitted, but no caller here does. # - # @return [Result] the PDF bytes and the rendered filename (nil if none was - # asked for) + # @return [Result] the PDF bytes and the rendered filename (nil only if + # yeti-pdf answered without one) def render_pdf(template:, data:, filename_template:, options: {}) response = post(RENDER_PATH, template: template, data: data, options: options, filename_template: filename_template) - # Only a caller that asked for a name gets one back: the response header - # is read solely when we sent a template for it, so the result never - # depends on what yeti-pdf might name a document by default. - filename = rendered_filename(response) unless filename_template.nil? - Result.new(response.body.to_s, filename) + Result.new(response.body.to_s, rendered_filename(response)) end # @return [String] the merged HTML (pre-PDF); useful for debugging/preview @@ -66,16 +60,13 @@ def render_html(template:, data:, options: {}) private - def post(path, template:, data:, options:, filename_template: nil) + # Every keyword becomes a field of the JSON body, so each endpoint sends + # exactly the keys it was given — render_html has no filename_template to + # pass and therefore never sends one. + def post(path, **payload) cfg = YetiConfig.invoice&.pdf_api raise Error, 'invoice.pdf_api.base_url is not configured' if cfg&.base_url.blank? - payload = { template: template, data: data, options: options } - # Sent verbatim when given, including '' — yeti-pdf tells absent (no name - # wanted) from empty (a name wanted, template broken) by the key's - # presence, so blank must not be dropped here. - payload[:filename_template] = filename_template unless filename_template.nil? - proxy = proxy_for(cfg) http = proxy.apply(client(cfg)) response = proxy.run { http.post(url(cfg, path), json: payload) } diff --git a/spec/requests/invoice_template_playground_spec.rb b/spec/requests/invoice_template_playground_spec.rb new file mode 100644 index 000000000..824dc3fb2 --- /dev/null +++ b/spec/requests/invoice_template_playground_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +# The playground preview action (app/admin/billing/invoice_template_playground.rb). +RSpec.describe 'Invoice template playground preview', type: :request do + include_context :login_as_admin + + let!(:contractor) { create(:vendor) } + let!(:account) { create(:account, contractor: contractor) } + let!(:invoice) { create(:invoice, account: account) } + + let(:params) do + { invoice_id: invoice.id, template: '

{{ invoice.reference }}

', filename_template: filename_template } + end + + # The test env keeps allow_forgery_protection on, so the preview POST needs a + # token from a real session — take it from the page the way the browser does + # (the editor reads the same meta tag) instead of weakening protection. + def csrf_token + get template_playground_path + Nokogiri::HTML(response.body).at('meta[name="csrf-token"]')&.[]('content') + end + + subject { post template_playground_preview_path, params: params, headers: { 'X-CSRF-Token' => csrf_token } } + + before { allow(YetiPdf::Client).to receive(:render_pdf) } + + context 'with a blank filename template' do + let(:filename_template) { '' } + + # Blank is rejected locally: the column is NOT NULL and the model requires + # it, so there is nothing worth asking yeti-pdf about. + it 'responds with a validation error and does not call yeti-pdf' do + subject + expect(response.status).to eq(422) + expect(response.body).to match(/Filename template must not be blank/) + expect(YetiPdf::Client).not_to have_received(:render_pdf) + end + end + + context 'with a filename template' do + let(:filename_template) { 'invoice-{{invoice.reference}}' } + + before do + allow(YetiPdf::Client).to receive(:render_pdf) + .and_return(YetiPdf::Client::Result.new('%PDF-bytes', 'invoice-INV-1')) + end + + it 'renders the pdf and reports the rendered name' do + subject + expect(response.status).to eq(200) + expect(response.body).to eq('%PDF-bytes') + expect(response.headers['X-Rendered-Filename']).to eq('invoice-INV-1') + expect(YetiPdf::Client).to have_received(:render_pdf) + .with(hash_including(filename_template: filename_template)) + end + end +end diff --git a/spec/services/yeti_pdf/client_spec.rb b/spec/services/yeti_pdf/client_spec.rb index 0d717e5cd..f8a458c37 100644 --- a/spec/services/yeti_pdf/client_spec.rb +++ b/spec/services/yeti_pdf/client_spec.rb @@ -17,75 +17,43 @@ end describe '.render_pdf' do - subject { described_class.render_pdf(template: template, data: data, filename_template: nil) } + subject { described_class.render_pdf(template: template, data: data, filename_template: filename_template) } - it 'posts the template and data and returns the pdf bytes' do - stub = stub_request(:post, render_url) - .to_return(status: 200, headers: { 'Content-Type' => 'application/pdf' }, body: '%PDF-bytes') + let(:filename_template) { '{{ invoice.reference }}' } - expect(subject).to have_attributes(pdf: '%PDF-bytes', filename: nil) + it 'posts both templates and the data, and returns the pdf and the rendered name' do + stub = stub_request(:post, render_url).to_return( + status: 200, + headers: { 'Content-Type' => 'application/pdf', + 'Content-Disposition' => 'attachment; filename="INV-1.pdf"' }, + body: '%PDF-bytes' + ) + + expect(subject).to have_attributes(pdf: '%PDF-bytes', filename: 'INV-1') expect(stub).to have_been_requested expect(WebMock).to have_requested(:post, render_url) .with(headers: { 'Content-Type' => %r{application/json} }, - body: hash_including('template' => template, 'options' => {})) - end - - it 'omits filename_template when nil, so no name is asked for' do - stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') - - subject - expect(WebMock).to have_requested(:post, render_url) - .with { |req| !JSON.parse(req.body).key?('filename_template') } - end - - # Blank must reach yeti-pdf: it distinguishes "no name wanted" (key absent) - # from "name wanted, template broken" (key present but empty) and rejects - # the latter, which is how the playground surfaces a cleared field. - it 'sends an empty filename_template rather than dropping it' do - stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') - - described_class.render_pdf(template: template, data: data, filename_template: '') - expect(WebMock).to have_requested(:post, render_url) - .with { |req| JSON.parse(req.body)['filename_template'] == '' } + body: hash_including('template' => template, + 'filename_template' => filename_template, + 'options' => {})) end - it 'ignores a Content-Disposition when no filename was requested' do + it 'decodes an RFC 2231 encoded name' do stub_request(:post, render_url).to_return( status: 200, - headers: { 'Content-Disposition' => 'attachment; filename="whatever.pdf"' }, + headers: { 'Content-Disposition' => "attachment; filename*=utf-8''Acm%C3%A9%20%26%20Co.pdf" }, body: '%PDF-bytes' ) - expect(subject.filename).to be_nil + expect(subject.filename).to eq('Acmé & Co') end - context 'with a filename_template' do - subject do - described_class.render_pdf(template: template, data: data, filename_template: '{{ invoice.reference }}') - end - - it 'sends it and returns the rendered name without the extension' do - stub_request(:post, render_url).to_return( - status: 200, - headers: { 'Content-Type' => 'application/pdf', - 'Content-Disposition' => 'attachment; filename="INV-1.pdf"' }, - body: '%PDF-bytes' - ) - - expect(subject).to have_attributes(pdf: '%PDF-bytes', filename: 'INV-1') - expect(WebMock).to have_requested(:post, render_url) - .with(body: hash_including('filename_template' => '{{ invoice.reference }}')) - end - - it 'decodes an RFC 2231 encoded name' do - stub_request(:post, render_url).to_return( - status: 200, - headers: { 'Content-Disposition' => "attachment; filename*=utf-8''Acm%C3%A9%20%26%20Co.pdf" }, - body: '%PDF-bytes' - ) + # Only reachable against a yeti-pdf too old to know filename_template; + # BillingInvoice::GenerateDocument turns it into a recorded pdf_error. + it 'reports no filename when the response carries none' do + stub_request(:post, render_url).to_return(status: 200, body: '%PDF-bytes') - expect(subject.filename).to eq('Acmé & Co') - end + expect(subject.filename).to be_nil end it 'raises Error with the status on a non-2xx response' do @@ -107,13 +75,21 @@ expect(described_class.render_html(template: template, data: data)).to eq('

INV-1

') end + + it 'sends no filename_template — the endpoint renders no document to name' do + stub_request(:post, html_url).to_return(status: 200, body: '

INV-1

') + + described_class.render_html(template: template, data: data) + expect(WebMock).to have_requested(:post, html_url) + .with { |req| !JSON.parse(req.body).key?('filename_template') } + end end context 'when pdf_api is not configured' do before { allow(YetiConfig).to receive(:invoice).and_return(nil) } it 'raises a clear Error' do - expect { described_class.render_pdf(template: template, data: data, filename_template: nil) } + expect { described_class.render_pdf(template: template, data: data, filename_template: 'x') } .to raise_error(YetiPdf::Client::Error, /not configured/) end end From 98b38163c2bd3e04299115d00bf46722493dc76c Mon Sep 17 00:00:00 2001 From: sdi Date: Mon, 10 Aug 2026 11:12:58 +0300 Subject: [PATCH 4/4] bump json gem --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1f18e05da..0fdd35cac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -485,7 +485,7 @@ GEM railties (>= 3.2) jquery-ui-rails (8.0.0) railties (>= 3.2.16) - json (2.20.0) + json (2.21.2) json-jwt (1.17.1) activesupport (>= 4.2) aes_key_wrap