Add configurable invoice email templates - #2078
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a system-wide, configurable Liquid-based invoice email template (HTML + plain text) and wires invoice approval delivery through it, while reusing the existing invoice data contract and avoiding expensive “detail” payload queries for emails.
Changes:
- Add
Billing::InvoiceEmailTemplatesingleton model/table (seeded) with strict save-time Liquid validation and admin UI for viewing/editing/preview. - Add
InvoiceMaildomain object to render subject/bodies from the template usingBillingInvoice::InvoiceData(details: false)plus document metadata. - Add optional plain-text alternative support to outbound emails (
notifications.email_logs.text_msg) and update mailer/rendering + sender/logging + specs.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/services/billing_invoice/invoice_data_spec.rb | Adds coverage for details: false payload shape and query avoidance. |
| spec/services/billing_invoice/approve_spec.rb | Verifies invoice approval logs an email rendered from the template. |
| spec/models/billing/invoice_email_template_spec.rb | Tests singleton constraint, validation, seeded content, and render behavior. |
| spec/mailers/yeti_mail_spec.rb | Expands coverage for single-part vs multipart (text+html) and attachments. |
| spec/features/billing/invoice_email_templates/manage_template_spec.rb | Feature coverage for admin management/preview of the singleton template. |
| spec/factories/log/email_logs.rb | Updates schema annotation for text_msg. |
| spec/domain/invoice_mail_spec.rb | Tests rendering/fallback behavior and assigns/variable contract for InvoiceMail. |
| spec/domain/contact_email_sender_spec.rb | Adds expectations for forwarding text_message and logging text_msg. |
| db/structure.sql | Adds billing.invoice_email_templates and notifications.email_logs.text_msg to DB structure. |
| db/seeds/main/billing.sql | Seeds a default invoice email template row. |
| db/migrate/20260810120001_add_text_msg_to_email_logs.rb | Adds text_msg column to email logs. |
| db/migrate/20260810120000_create_invoice_email_template.rb | Creates and seeds the singleton invoice email template table. |
| app/services/billing_invoice/invoice_data.rb | Adds details: parameter to optionally omit expensive breakdown queries. |
| app/policies/billing/invoice_email_template_policy.rb | Adds policy for the new admin-managed resource. |
| app/models/log/email_log.rb | Updates schema annotation for text_msg. |
| app/models/concerns/liquid_template.rb | Introduces shared Liquid validation/rendering behavior for template models. |
| app/models/billing/notification_template.rb | Refactors to use LiquidTemplate concern for validation/rendering. |
| app/models/billing/invoice_email_template.rb | New model implementing singleton template behavior and render helpers. |
| app/models/billing/invoice_document.rb | Switches invoice email subject/body to rendered template via InvoiceMail. |
| app/mailers/yeti_mail.rb | Adds conditional rendering of a plain-text part from email_log.text_msg. |
| app/domain/invoice_mail.rb | New domain object encapsulating rendering, assigns, and fallback behavior. |
| app/domain/contact_email_sender.rb | Adds text_message plumbing and persists text_msg to email logs. |
| app/admin/logs/email_logs.rb | Displays stored plain-text body in admin email log view. |
| app/admin/billing/invoice_email_templates.rb | Adds ActiveAdmin resource (edit/show/preview) for the singleton template. |
Suppressed comments (1)
spec/domain/contact_email_sender_spec.rb:171
- Same issue as above: this expectation currently matches a positional Hash argument, but the production call passes keyword args. Use
with(**forwarded_params)so the expectation matches keyword arguments correctly.
it 'send emails to unique contacts' do
contacts.uniq.each do |contact|
sender_stub = instance_double(described_class)
expect(described_class).to receive(:new).with(contact).once.and_return(sender_stub)
expect(sender_stub).to receive(:send_email).with(forwarded_params).once
end
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| it 'send emails to all contacts' do | ||
| contacts.each do |contact| | ||
| sender_stub = instance_double(described_class) | ||
| expect(described_class).to receive(:new).with(contact).once.and_return(sender_stub) | ||
| expect(sender_stub).to receive(:send_email).with(service_params).once | ||
| expect(sender_stub).to receive(:send_email).with(forwarded_params).once | ||
| end |
| # Only senders that store a plain-text alternative produce a multipart | ||
| # message; the rest stay single-part HTML exactly as before. Wire order | ||
| # (text before html, so clients pick the richest part they understand) is | ||
| # ActionMailer's parts_order, not this block's order. | ||
| format.text { render plain: email_log.text_msg } if email_log.text_msg.present? | ||
| # Some body is required even when the message is really just its | ||
| # attachments, hence the blank placeholder. | ||
| format.html { render html: email_log.msg&.html_safe || ' ' } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (2)
db/seeds/main/billing.sql:275
- The generated HTML seed repeats raw Liquid substitutions. Because Liquid emits strings without HTML escaping, account names, invoice references, currencies, or filenames containing markup will be rendered as email HTML. Regenerate this seed after adding
| escapeto all dynamic values in the migration's HTML template.
<p style="margin:0 0 16px 0;">Dear {{ account.name }},</p>
<p style="margin:0 0 16px 0;">Please find attached invoice <strong>{{ invoice.reference }}</strong> covering the period from {{ invoice.start_date | date: "%Y-%m-%d" }} to {{ invoice.end_date | date: "%Y-%m-%d" }}.</p>
<table width="100%" cellpadding="0" cellspacing="0" border="0" style="border:1px solid #e8e8e8;border-collapse:collapse;">
<tr>
<td style="border:1px solid #e8e8e8;padding:8px;background-color:#f4f5f5;width:45%;">Invoice reference</td>
<td style="border:1px solid #e8e8e8;padding:8px;font-weight:bold;">{{ invoice.reference }}</td>
db/migrate/20260810120000_create_invoice_email_template.rb:33
- Liquid does not HTML-escape output by default, so interpolating fields such as
account.nameandinvoice.referencedirectly into the seeded HTML allows markup in those stored values to alter the outgoing email (and even ordinary names containing<can break it). Apply Liquid'sescapefilter to every dynamic value used in the HTML body, and regenerate the checked-in seed copy; subject and plain-text substitutions should remain unescaped.
<p style="margin:0 0 16px 0;">Dear {{ account.name }},</p>
<p style="margin:0 0 16px 0;">Please find attached invoice <strong>{{ invoice.reference }}</strong> covering the period from {{ invoice.start_date | date: "%Y-%m-%d" }} to {{ invoice.end_date | date: "%Y-%m-%d" }}.</p>
No description provided.