#86 - Add branding support to email templates by reshmabidikar · Pull Request #87 · killbill/killbill-email-notifications-plugin

reshmabidikar · 2026-08-12T10:50:00Z

Work for #86:

Branding support added to email templates
Fast tests modified and re-enabled (they were disabled during the 0.24.x work)
Additional fast test added for branding feature

Copilot

Pull request overview

Adds shared-tenant branding support (logo, company info, brand colors) to the email notifications plugin by introducing branding loaders and Mustache partial templates, and updates/enables fast tests to validate rendering with tenant-provided branding data.

Changes:

Introduces EmailBrandingLoader and associated data classes/keys to fetch and merge branding data from tenant KV.
Adds Mustache partials (tenantStyleInfo, tenantLogoInfo, tenantCompanyInfo) and a new invoice template variant consuming them.
Refactors and re-enables fast template-rendering tests, adding a new test covering tenant branding variables.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/test/resources/org/killbill/billing/plugin/notification/templates/logoInfo.json	Test logo payload used to validate tenant logo rendering.
src/test/resources/org/killbill/billing/plugin/notification/templates/InvoiceCreation-new-fields.mustache	New invoice template variant using branding partials.
src/test/resources/org/killbill/billing/plugin/notification/templates/companyInfo.json	Test company info JSON used by branding loader tests.
src/test/resources/org/killbill/billing/plugin/notification/templates/brandInfo.json	Test brand color JSON used by branding loader tests.
src/test/java/org/killbill/billing/plugin/notification/generator/TestTemplateRenderer.java	Re-enables/refactors fast tests and adds branding-specific test coverage.
src/main/resources/org/killbill/billing/plugin/notification/templates/tenantStyleInfo.mustache	Partial defining CSS variables from tenant brand info.
src/main/resources/org/killbill/billing/plugin/notification/templates/tenantLogoInfo.mustache	Partial to render tenant logo in templates.
src/main/resources/org/killbill/billing/plugin/notification/templates/tenantCompanyInfo.mustache	Partial to render tenant company information.
src/main/java/org/killbill/billing/plugin/notification/templates/MustacheTemplateEngine.java	Adds a shared Mustache compiler configured to resolve partials from classpath resources.
src/main/java/org/killbill/billing/plugin/notification/generator/TemplateRenderer.java	Injects company/logo/brand objects into the Mustache rendering data model.
src/main/java/org/killbill/billing/plugin/notification/email/branding/LogoInfo.java	New POJO for logo data.
src/main/java/org/killbill/billing/plugin/notification/email/branding/EmailBrandingLoader.java	New loader to retrieve and merge branding data from tenant KV.
src/main/java/org/killbill/billing/plugin/notification/email/branding/EmailBrandingKey.java	New enum defining tenant KV keys for branding.
src/main/java/org/killbill/billing/plugin/notification/email/branding/CompanyInfo.java	New POJO for company info data.
src/main/java/org/killbill/billing/plugin/notification/email/branding/BrandInfo.java	New POJO for brand color data.
.idea/misc.xml	Updates IDE metadata (project JDK and external storage config).

Files not reviewed (1)

.idea/misc.xml: Generated file

Suppressed comments (2)

src/main/java/org/killbill/billing/plugin/notification/email/branding/EmailBrandingLoader.java:75

Issue #86 calls for shared tenant branding to take precedence. getEmailTemplateLogoInfo currently prefers EMAIL_TEMPLATE_LOGO_INFO over LOGO_INFO, which makes template-specific branding override shared branding.

    public LogoInfo getEmailTemplateLogoInfo(final TenantContext context) {
        String value = getValue(EmailBrandingKey.EMAIL_TEMPLATE_LOGO_INFO, "email template logo info", context);
        if (value == null) {
            value = getValue(EmailBrandingKey.LOGO_INFO, "company info", context);
        }
        return deserializeJson(value, LogoInfo.class, new LogoInfo(DEFAULT_PROPERTIES.get("logo")));

src/main/java/org/killbill/billing/plugin/notification/email/branding/EmailBrandingLoader.java:82

Brand color precedence is currently template-specific first (EMAIL_TEMPLATE_BRAND_INFO overrides BRAND_INFO). Per issue #86, shared tenant branding should be used when available, with template-specific values only as fallback.

    public BrandInfo getEmailTemplateBrandInfo(final TenantContext context) {
        final BrandInfo invoiceTemplate = loadBrandInfo(EmailBrandingKey.EMAIL_TEMPLATE_BRAND_INFO, "email template brand info", context);
        final BrandInfo global = loadBrandInfo(EmailBrandingKey.BRAND_INFO, "brand info", context);
        return mergeBrandInfo(invoiceTemplate, global);
    }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

reshmabidikar · 2026-08-12T12:48:56Z

+        if (values.size() > 1) {
+            throw new IllegalStateException(String.format(
+                    "Unexpected number of values %d for %s and tenant %d",
+                    values.size(), msg, tenantContext.getTenantId()));
+        }


Addressed in e2489fd

reshmabidikar · 2026-08-12T12:49:33Z

+    public CompanyInfo getEmailTemplateCompanyInfo(final TenantContext context) {
+        String value = getValue(EmailBrandingKey.EMAIL_TEMPLATE_COMPANY_INFO, "email template company info", context);
+        if (value == null) {
+            value = getValue(EmailBrandingKey.COMPANY_INFO, "company info", context);
+        }
+        return deserializeJson(value, CompanyInfo.class, new CompanyInfo(DEFAULT_PROPERTIES.get("companyName"), DEFAULT_PROPERTIES.get("companyAddress"), DEFAULT_PROPERTIES.get("companyCityProvincePostalCode"), DEFAULT_PROPERTIES.get("companyCountry"), DEFAULT_PROPERTIES.get("companyUrl")));
+    }


The code behavior is correct, ignoring this one.

reshmabidikar · 2026-08-12T12:49:40Z

+    private String getValue(final EmailBrandingKey key,
+                            final String description,
+                            final TenantContext tenantContext) {
+        final List<String> values;
+        try {
+            values = tenantApi.getTenantValuesForKey(key.toString(), tenantContext);
+            return getUniqueValue(values, description, tenantContext);
+        } catch (final TenantApiException e) {
+            //TODO_86 - add a log statement
+
+        }
+        return null;
+    }


Addressed in e2489fd

reshmabidikar · 2026-08-12T12:49:46Z

@@ -0,0 +1 @@
+{{#logo}}<img src="{{{logo}}}" style="width:100%; max-width:300px;">{{/logo}}


Addressed in e2489fd

reshmabidikar · 2026-08-12T12:49:54Z

 <project version="4">
+  <component name="ExternalStorageConfigurationManager" enabled="true" />
  <component name="FrameworkDetectionExcludesConfiguration">
    <file type="Osmorc" url="file://$PROJECT_DIR$" />
  </component>


Addressed in e2489fd

reshmabidikar · 2026-08-12T12:50:00Z

    </option>
  </component>
-  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="false" project-jdk-name="1.8" project-jdk-type="JavaSDK" />
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" project-jdk-name="11" project-jdk-type="JavaSDK" />


Addressed in e2489fd

sbrossie

I only looked at the diff so I may be missing the big picture: Does this PR contain everything we need? Could you add some screenshots for the default branding, logo we provide?

sbrossie · 2026-08-14T17:32:47Z

+    EMAIL_TEMPLATE_COMPANY_INFO,
+    EMAIL_TEMPLATE_BRAND_INFO,
+    EMAIL_TEMPLATE_LOGO_INFO,
+    EMAIL_TEMPLATE_WITH_BRANDING


Do we need this key - where is it used?

Not required, was a copy paste error, I have removed it in 043fbe2.

sbrossie · 2026-08-14T17:33:47Z

@@ -0,0 +1,219 @@
+<!doctype html>


Do we need to modify the other template types - seems like from a symmetry aspect, if we modify InvoiceCreation, any other type should also be modified?

This template is created just for unit tests. See this test. I did not write unit tests for all the template types and so did not add email templates for the other email types.

reshmabidikar · 2026-08-17T03:50:11Z

I only looked at the diff so I may be missing the big picture: Does this PR contain everything we need? Could you add some screenshots for the default branding, logo we provide?

Here are some screenshots.

Default template (without the new branding variables):

Custom template (with new branding variables), default values for the company/brand/logo information (let me know if you think these default values should be changed, they are defined here in the code):

template-with-new-variables-default-key-valuea

Custom template (with new branding variables), custom values for the company/brand/logo information:

template-with-new-variables-custom-key-values

sbrossie

Were you able to deploy on snapshot - or simply make a release - on our QA env to test if this provides the full story with Aviate - i.e. Email Configuration?

Add branding support to email templates

9aad97a

reshmabidikar requested a lite review from Copilot August 12, 2026 10:55

Copilot started reviewing on behalf of reshmabidikar August 12, 2026 11:03 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Address copilot review comments

e2489fd

reshmabidikar requested review from pierre and sbrossie August 12, 2026 12:50

sbrossie reviewed Aug 14, 2026

View reviewed changes

Remove unnecessary key

043fbe2

sbrossie approved these changes Aug 21, 2026

View reviewed changes

		@@ -0,0 +1 @@
		{{#logo}}<img src="{{{logo}}}" style="width:100%; max-width:300px;">{{/logo}} No newline at end of file

Uh oh!

Uh oh!

Conversation

reshmabidikar commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbrossie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reshmabidikar commented Aug 17, 2026

Uh oh!

sbrossie left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reshmabidikar commented Aug 12, 2026 •

edited

Loading