From 8e926a1c1ba011e4d5a5d462f3e6cd207f996b51 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Mon, 31 Aug 2026 14:29:00 +0200 Subject: [PATCH 1/5] feat(templates): white-labelled template variants A campaign's templates were keyed by locale alone, so giving a client their own design or wording meant duplicating the whole campaign and keeping the copies in step by hand. Variants add a second dimension to the same lookup: a template is now identified by (campaign, locale, variant), where the empty variant is the house brand every existing template already belongs to. Which variant a send uses is resolved in two layers. A journey step or a broadcast can name one outright, which travels on the SendCampaign event. Otherwise the campaign's variant_selector - a Liquid expression such as "{{ user.data.tenant }}" - is rendered per recipient, so one broadcast over a list spanning several clients still reaches each of them under their own branding. A variant with no template for a campaign falls back to the default rather than failing: a missing white-label template must not stop a message going out. Because that fallback is silent by design, the selected variant is recorded on the send and counted under lunogram_campaign_variant_selections_total, and the campaign page marks variants that have no template yet. Sender identity was already per-template, so a variant sends from the client's own domain with no further work. Two things worth noting in the selection change. The single-template shortcut now runs after the variant filter - left where it was, a campaign holding one variant template would have answered every send with it. And a campaign with no templates at all returns an error instead of indexing an empty slice, which would have panicked the consumer. Declaring variants and creating a template for one are gated behind the enterprise build tag. The send path is deliberately not gated: with no variant configurable in an open-source build every template is the default variant and selection resolves to it unaided, so both builds share one code path through the render hot loop. --- console/src/oapi/management.generated.ts | 46 +++++ console/src/types.ts | 10 +- .../broadcast/broadcast-response.ts | 1 + .../validation/broadcast/create-broadcast.ts | 1 + .../views/broadcast/CreateBroadcastDialog.tsx | 29 +++ .../src/views/campaign/CampaignDetails.tsx | 30 +++ .../src/views/campaign/CampaignVariants.tsx | 178 +++++++++++++++++ .../src/views/campaign/template/Template.tsx | 61 ++++-- .../views/campaign/template/VariantSelect.tsx | 66 +++++++ console/src/views/journey/steps/Campaign.tsx | 30 ++- .../v1/management/broadcasts_enterprise.go | 17 ++ .../controllers/v1/management/campaigns.go | 43 +++- .../v1/management/campaigns_test.go | 12 +- .../v1/management/oapi/journeys.go | 5 +- .../v1/management/oapi/resources.yml | 65 +++++++ .../v1/management/oapi/resources_gen.go | 29 +++ .../v1/management/template_variants.go | 12 ++ .../template_variants_enterprise.go | 5 + .../template_variants_enterprise_test.go | 178 +++++++++++++++++ .../v1/management/template_variants_test.go | 71 +++++++ .../controllers/v1/management/templates.go | 21 +- .../v1/management/templates_test.go | 6 +- internal/journeys/campaign.go | 14 ++ internal/node/metrics/metrics.go | 14 ++ .../pubsub/consumer/broadcasts_enterprise.go | 4 + internal/pubsub/consumer/campaigns.go | 3 + internal/pubsub/consumer/campaigns_render.go | 115 +++++++++-- .../pubsub/consumer/campaigns_variant_test.go | 184 ++++++++++++++++++ internal/pubsub/schemas/events.go | 4 + internal/store/management/broadcasts.go | 35 ++-- internal/store/management/campaigns.go | 112 ++++++++--- .../1787000000007_template_variants.down.sql | 5 + .../1787000000007_template_variants.up.sql | 27 +++ internal/store/management/templates.go | 18 +- 34 files changed, 1356 insertions(+), 95 deletions(-) create mode 100644 console/src/views/campaign/CampaignVariants.tsx create mode 100644 console/src/views/campaign/template/VariantSelect.tsx create mode 100644 internal/http/controllers/v1/management/template_variants.go create mode 100644 internal/http/controllers/v1/management/template_variants_enterprise.go create mode 100644 internal/http/controllers/v1/management/template_variants_enterprise_test.go create mode 100644 internal/http/controllers/v1/management/template_variants_test.go create mode 100644 internal/pubsub/consumer/campaigns_variant_test.go create mode 100644 internal/store/management/migrations/1787000000007_template_variants.down.sql create mode 100644 internal/store/management/migrations/1787000000007_template_variants.up.sql diff --git a/console/src/oapi/management.generated.ts b/console/src/oapi/management.generated.ts index 18758fe0e..7d7a14567 100644 --- a/console/src/oapi/management.generated.ts +++ b/console/src/oapi/management.generated.ts @@ -3039,6 +3039,11 @@ export interface components { * @example 52f3f921-1343-48af-b795-87c0fd3b44aa */ campaign_id: string; + /** + * @description Template variant to send, as a static key or a Liquid expression resolved against the journey context. Overrides the campaign's own variant selector. + * @example {{ user.data.tenant }} + */ + variant?: string; }; /** @description Data for action step - execute WASM action */ ActionStepData: { @@ -3330,6 +3335,12 @@ export interface components { /** @example false */ transactional?: boolean; variables?: components["schemas"]["CampaignVariable"][]; + variants?: components["schemas"]["CampaignVariant"][]; + /** + * @description Liquid expression resolved per recipient when a send does not name a variant itself, for example "{{ user.data.tenant }}". An expression that resolves to an unknown variant falls back to the default one. + * @example {{ user.data.tenant }} + */ + variant_selector?: string | null; }; CampaignVariable: { /** @@ -3343,12 +3354,29 @@ export interface components { */ default?: string; }; + CampaignVariant: { + /** + * @description The value a send resolves against to pick this variant's templates + * @example acme + */ + key: string; + /** + * @description Human readable name shown in the console + * @example Acme Corp + */ + label?: string; + }; CreateTemplate: { /** * @description The locale/language code for the template * @example en */ locale: string; + /** + * @description The variant this template belongs to. Empty selects the default variant, which is the branding every campaign starts with. + * @example acme + */ + variant?: string; /** @description Template-specific data based on type. Structure varies by template type. */ data?: { [key: string]: unknown; @@ -3413,6 +3441,12 @@ export interface components { transactional: boolean; templates: components["schemas"]["Template"][]; variables?: components["schemas"]["CampaignVariable"][]; + variants?: components["schemas"]["CampaignVariant"][]; + /** + * @description Liquid expression resolved per recipient when a send does not name a variant itself + * @example {{ user.data.tenant }} + */ + variant_selector?: string | null; delivery: components["schemas"]["Delivery"]; /** * @description Whether the campaign has been archived @@ -3691,6 +3725,11 @@ export interface components { data: components["schemas"]["EmailTemplateData"] | components["schemas"]["SmsTemplateData"] | components["schemas"]["PushTemplateData"]; /** @example en */ locale: string; + /** + * @description The variant this template belongs to. Empty is the default variant. + * @example acme + */ + variant: string; /** * Format: uuid * @description The ID of the sender identity to use for this template @@ -5433,6 +5472,11 @@ export interface components { * @description Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. */ scheduled_at?: string; + /** + * @description Pins every message in this broadcast to one template variant. Leave unset to let the campaign's variant selector resolve a variant per recipient, which is what a mixed-tenant list needs. + * @example acme + */ + variant?: string | null; }; UpdateBroadcast: { /** @@ -5454,6 +5498,8 @@ export interface components { list_name: string; /** @description Snapshot of the list type at broadcast creation time */ list_type: string; + /** @description Template variant every message in this broadcast is pinned to */ + variant?: string | null; state: components["schemas"]["BroadcastState"]; /** * @description Total number of users in the audience at send time diff --git a/console/src/types.ts b/console/src/types.ts index a59b5a756..d004d60a1 100644 --- a/console/src/types.ts +++ b/console/src/types.ts @@ -741,6 +741,11 @@ export interface CampaignVariable { default?: string } +export interface CampaignVariant { + key: string + label?: string +} + export interface Campaign { id: UUID project_id: UUID @@ -752,6 +757,8 @@ export interface Campaign { transactional?: boolean templates: Template[] variables: CampaignVariable[] + variants: CampaignVariant[] + variant_selector?: string | null created_at: string updated_at: string } @@ -861,6 +868,7 @@ export type Template< campaign_id: UUID type: ChannelType locale: string + variant: string sender_identity_id: UUID | null data: DataObjectType screenshot_url: string @@ -885,7 +893,7 @@ export type Template< } ) -export type TemplateCreateParams = Pick +export type TemplateCreateParams = Pick & { variant?: string } export type TemplateUpdateParams = Pick export type VariantUpdateParams = { id?: UUID } diff --git a/console/src/validation/broadcast/broadcast-response.ts b/console/src/validation/broadcast/broadcast-response.ts index cc8dc07c6..fc8cea6d9 100644 --- a/console/src/validation/broadcast/broadcast-response.ts +++ b/console/src/validation/broadcast/broadcast-response.ts @@ -7,6 +7,7 @@ export const broadcastResponseSchema = z.object({ list_id: z.string(), list_name: z.string(), list_type: z.enum(["static", "dynamic"]), + variant: z.string().nullish(), state: z.enum(["scheduled", "pending", "sending", "completed", "failed", "cancelled"]), total: z.number(), sent: z.number().optional().default(0), diff --git a/console/src/validation/broadcast/create-broadcast.ts b/console/src/validation/broadcast/create-broadcast.ts index e1dd76cb1..64fb977b2 100644 --- a/console/src/validation/broadcast/create-broadcast.ts +++ b/console/src/validation/broadcast/create-broadcast.ts @@ -6,6 +6,7 @@ export const createBroadcastSchema = z list_id: z.string().min(1, "List is required"), is_scheduled: z.boolean(), scheduled_at: z.string().optional(), + variant: z.string().optional(), }) .refine((data) => !data.is_scheduled || data.scheduled_at, { message: "Scheduled time is required", diff --git a/console/src/views/broadcast/CreateBroadcastDialog.tsx b/console/src/views/broadcast/CreateBroadcastDialog.tsx index 328d1fe0c..54c44b035 100644 --- a/console/src/views/broadcast/CreateBroadcastDialog.tsx +++ b/console/src/views/broadcast/CreateBroadcastDialog.tsx @@ -47,6 +47,9 @@ const channelIcons: Record = { inbox: Inbox, } +import { VariantSelect } from "@/views/campaign/template/VariantSelect" +import { isEnterprise } from "@/config/enterprise" + interface CreateBroadcastDialogProps { open: boolean onOpenChange: (open: boolean) => void @@ -77,6 +80,7 @@ export function CreateBroadcastDialog({ list_id: preselectedListId ?? "", is_scheduled: false, scheduled_at: "", + variant: "", }, }) @@ -150,6 +154,7 @@ export function CreateBroadcastDialog({ ...(values.is_scheduled && values.scheduled_at ? { scheduled_at: new Date(values.scheduled_at).toISOString() } : {}), + ...(values.variant ? { variant: values.variant } : {}), }, }, ) @@ -246,6 +251,30 @@ export function CreateBroadcastDialog({ )} + {/* Variant Selector - only for campaigns that declare variants */} + {isEnterprise && (selectedCampaign?.variants?.length ?? 0) > 0 && ( +
+ + ( + + )} + /> +

+ {t( + "broadcast.variant_description", + "Sends every message under one design. Pick Default to let the campaign choose per recipient.", + )} +

+
+ )} + {/* List Selector */}
diff --git a/console/src/views/campaign/CampaignDetails.tsx b/console/src/views/campaign/CampaignDetails.tsx index 700d0fbc2..1da4e751a 100644 --- a/console/src/views/campaign/CampaignDetails.tsx +++ b/console/src/views/campaign/CampaignDetails.tsx @@ -11,6 +11,7 @@ import { useResolver } from "@/hooks" import { channels } from "./template/channels" import { CampaignVariables } from "./CampaignVariables" +import { CampaignVariants } from "./CampaignVariants" import { CampaignVariableProvider } from "./CampaignVariableContext" import { Tabs, TabsContent } from "@/components/ui/tabs" @@ -39,6 +40,8 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: const [isBroadcastOpen, setIsBroadcastOpen] = useState(false) const [isTransactional, setTransactional] = useState(campaign.transactional ?? false) const [subscriptionId, setSubscriptionId] = useState(campaign.subscription_id ?? "") + const [variants, setVariants] = useState(campaign.variants ?? []) + const [variantSelector, setVariantSelector] = useState(campaign.variant_selector ?? "") const [subscriptions] = useResolver( useCallback(async (): Promise => { @@ -89,6 +92,10 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: transactional: isTransactional, subscription_id: isTransactional ? undefined : effectiveSubscriptionId || undefined, variables: data.variables.filter((v) => v.name), + ...(isEnterprise && { + variants: variants.filter((v) => v.key), + variant_selector: variantSelector, + }), }) setCampaign(updatedCampaign) @@ -159,6 +166,29 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: /> + {isEnterprise && ( + + + + {t("campaign.variants.label", "Variants")} + + + {t( + "campaign.variants.description", + "Give a client this campaign in their own design or wording. Each variant gets its own templates.", + )} + + + + + )} +
{templateId && ( -
+
+ {isEnterprise && (campaign.variants?.length ?? 0) > 0 && ( + + )}
)} diff --git a/console/src/views/campaign/template/VariantSelect.tsx b/console/src/views/campaign/template/VariantSelect.tsx new file mode 100644 index 000000000..9c4c21431 --- /dev/null +++ b/console/src/views/campaign/template/VariantSelect.tsx @@ -0,0 +1,66 @@ +import { useMemo } from "react" +import { useTranslation } from "react-i18next" +import { Palette } from "lucide-react" + +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" +import type { CampaignVariant } from "@/types" + +export const DEFAULT_VARIANT_VALUE = "__default__" + +interface VariantSelectProps { + variants: CampaignVariant[] + value: string + onChange: (variant: string) => void + disabled?: boolean +} + +export function VariantSelect({ variants, value, onChange, disabled }: VariantSelectProps) { + const { t } = useTranslation() + + // The default variant is not a declared entry, so it gets a sentinel value: + // Radix treats an empty string as "nothing selected" and would render the + // placeholder instead of the option. + const options = useMemo( + () => [ + { value: DEFAULT_VARIANT_VALUE, label: t("campaign.variants.default", "Default") }, + ...variants + .filter((variant) => variant.key) + .map((variant) => ({ + value: variant.key, + label: variant.label || variant.key, + })), + ], + [variants, t], + ) + + return ( + + ) +} diff --git a/console/src/views/journey/steps/Campaign.tsx b/console/src/views/journey/steps/Campaign.tsx index ed967a5c6..061efe875 100644 --- a/console/src/views/journey/steps/Campaign.tsx +++ b/console/src/views/journey/steps/Campaign.tsx @@ -16,10 +16,12 @@ import { PlusIcon } from "lucide-react" import { Button } from "@/components/ui/button" import { TemplateInput } from "@/components/ui/template-input" import { useJourneyVariableContext } from "../JourneyVariableContext" +import { isEnterprise } from "@/config/enterprise" interface CampaignConfig { campaign_id: UUID data?: Record + variant?: string } type CampaignOption = Campaign & { path: string } @@ -90,6 +92,7 @@ export const campaignStep: JourneyStepType = { ) const variables = campaign?.variables ?? [] + const campaignVariants = campaign?.variants ?? [] const journeyVariables = nodeId ? getVariablesForNode(nodeId) : [] const handleVariableChange = (name: string, newValue: string) => { @@ -146,7 +149,12 @@ export const campaignStep: JourneyStepType = { value={value.campaign_id === NIL ? "" : value.campaign_id} displayValue={campaign?.name} onValueChange={(id) => - onChange({ ...value, campaign_id: (id || NIL) as UUID, data: {} }) + onChange({ + ...value, + campaign_id: (id || NIL) as UUID, + data: {}, + variant: undefined, + }) } placeholder={t("campaign.singular")} renderOption={(option) => option.name} @@ -167,6 +175,26 @@ export const campaignStep: JourneyStepType = { } /> + {isEnterprise && campaign && campaignVariants.length > 0 && ( +
+ +

+ {t( + "journey.campaign.variant_description", + "Which design this step sends. Leave empty to let the campaign decide per recipient.", + )} +

+ onChange({ ...value, variant: newValue })} + variables={journeyVariables} + placeholder={campaignVariants.map((v) => v.key).join(", ")} + /> +
+ )} + {campaign && variables.length > 0 && (
diff --git a/internal/http/controllers/v1/management/broadcasts_enterprise.go b/internal/http/controllers/v1/management/broadcasts_enterprise.go index 5e0f8e18b..50f1bc9c9 100644 --- a/internal/http/controllers/v1/management/broadcasts_enterprise.go +++ b/internal/http/controllers/v1/management/broadcasts_enterprise.go @@ -6,6 +6,7 @@ import ( "database/sql" "errors" "net/http" + "strings" "time" "github.com/google/uuid" @@ -87,6 +88,21 @@ func (srv *BroadcastsController) CreateBroadcast(w http.ResponseWriter, r *http. return } + // Pinning a broadcast to a variant the campaign never declared would send + // the whole list the default branding while the operator believes they + // picked a client, so refuse it outright rather than falling back. + var variant *string + if body.Variant != nil { + key := strings.TrimSpace(*body.Variant) + if key != "" { + if !campaign.Variants.Data.Has(key) { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("campaign does not declare variant "+key))) + return + } + variant = &key + } + } + list, err := srv.usrs.GetList(ctx, projectID, body.ListId) if errors.Is(err, sql.ErrNoRows) { logger.Info("list not found", zap.Stringer("list_id", body.ListId)) @@ -107,6 +123,7 @@ func (srv *BroadcastsController) CreateBroadcast(w http.ResponseWriter, r *http. ListName: list.Name, ListType: string(list.Type), ScheduledAt: body.ScheduledAt, + Variant: variant, }) if err != nil { logger.Error("failed to create broadcast", zap.Error(err)) diff --git a/internal/http/controllers/v1/management/campaigns.go b/internal/http/controllers/v1/management/campaigns.go index 8b4437e2d..1d6448ff6 100644 --- a/internal/http/controllers/v1/management/campaigns.go +++ b/internal/http/controllers/v1/management/campaigns.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "net/http" + "strings" "github.com/google/uuid" "github.com/jmoiron/sqlx" @@ -117,7 +118,7 @@ func (srv *CampaignsController) CreateCampaign(w http.ResponseWriter, r *http.Re // TODO: create audit log - _, err = templates.CreateTemplate(ctx, project.ID, campaignID, string(body.Channel), project.Locale, nil) + _, err = templates.CreateTemplate(ctx, project.ID, campaignID, string(body.Channel), project.Locale, "", nil) if err != nil { logger.Error("failed to create template", zap.Error(err)) oapi.WriteProblem(w, err) @@ -251,6 +252,46 @@ func (srv *CampaignsController) UpdateCampaign(w http.ResponseWriter, r *http.Re updated.Variables = &store.JSONB[management.CampaignVariables]{Data: vars} } + if body.Variants != nil || body.VariantSelector != nil { + if !variantsAvailable { + oapi.WriteProblem(w, problem.ErrNotFound(problem.Describe("template variants are not available in the open-source version"))) + return + } + } + + if body.Variants != nil { + variants := make(management.CampaignVariants, 0, len(*body.Variants)) + seen := make(map[string]bool, len(*body.Variants)) + for _, v := range *body.Variants { + key := strings.TrimSpace(v.Key) + + // The empty key is the default variant. It always exists and is + // never declared, so accepting it here would create a duplicate of + // something that cannot be removed. + if key == "" { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("variant key cannot be empty"))) + return + } + + if seen[key] { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("duplicate variant key "+key))) + return + } + seen[key] = true + + variant := management.CampaignVariant{Key: key} + if v.Label != nil { + variant.Label = strings.TrimSpace(*v.Label) + } + variants = append(variants, variant) + } + updated.Variants = &store.JSONB[management.CampaignVariants]{Data: variants} + } + + if body.VariantSelector != nil { + updated.VariantSelector = body.VariantSelector + } + if body.SubscriptionId != nil { subscription, err := srv.mgmt.SubscriptionsStore.GetSubscription(ctx, projectID, *body.SubscriptionId) if errors.Is(err, sql.ErrNoRows) { diff --git a/internal/http/controllers/v1/management/campaigns_test.go b/internal/http/controllers/v1/management/campaigns_test.go index 07bb14e06..02c155fef 100644 --- a/internal/http/controllers/v1/management/campaigns_test.go +++ b/internal/http/controllers/v1/management/campaigns_test.go @@ -112,7 +112,7 @@ func TestListCampaigns(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) } @@ -234,7 +234,7 @@ func TestGetCampaign(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -323,7 +323,7 @@ func TestUpdateCampaign(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -419,7 +419,7 @@ func TestDeleteCampaign(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -485,7 +485,7 @@ func TestDuplicateCampaign(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -555,7 +555,7 @@ func TestGetCampaignUsers(t *testing.T) { }) require.NoError(t, err) - _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + _, err = templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), diff --git a/internal/http/controllers/v1/management/oapi/journeys.go b/internal/http/controllers/v1/management/oapi/journeys.go index a03ad036b..42a5884dd 100644 --- a/internal/http/controllers/v1/management/oapi/journeys.go +++ b/internal/http/controllers/v1/management/oapi/journeys.go @@ -228,10 +228,13 @@ type DelayStepData struct { ExclusionDays *[]int `json:"exclusion_days,omitempty"` } -// CampaignStepData represents data for campaign step - send campaign +// CampaignStepData represents data for campaign step - send campaign. +// Variant is either a static variant key or a Liquid expression resolved +// against the journey context, and overrides the campaign's own selector. type CampaignStepData struct { CampaignId uuid.UUID `json:"campaign_id" yaml:"campaign_id"` Data map[string]string `json:"data,omitempty" yaml:"data,omitempty"` + Variant *string `json:"variant,omitempty" yaml:"variant,omitempty"` } // ActionStepData represents data for action step - execute WASM action diff --git a/internal/http/controllers/v1/management/oapi/resources.yml b/internal/http/controllers/v1/management/oapi/resources.yml index 969cd88ac..4f5570717 100644 --- a/internal/http/controllers/v1/management/oapi/resources.yml +++ b/internal/http/controllers/v1/management/oapi/resources.yml @@ -7194,6 +7194,13 @@ components: description: Campaign to send example: "52f3f921-1343-48af-b795-87c0fd3b44aa" x-go-type: uuid.UUID + variant: + type: string + description: >- + Template variant to send, as a static key or a Liquid expression + resolved against the journey context. Overrides the campaign's own + variant selector. + example: "{{ user.data.tenant }}" ActionStepData: type: object @@ -7617,6 +7624,18 @@ components: type: array items: $ref: "#/components/schemas/CampaignVariable" + variants: + type: array + items: + $ref: "#/components/schemas/CampaignVariant" + variant_selector: + type: string + nullable: true + description: >- + Liquid expression resolved per recipient when a send does not name a + variant itself, for example "{{ user.data.tenant }}". An expression + that resolves to an unknown variant falls back to the default one. + example: "{{ user.data.tenant }}" CampaignVariable: type: object @@ -7632,6 +7651,20 @@ components: description: Default value for the variable example: "there" + CampaignVariant: + type: object + required: + - key + properties: + key: + type: string + description: The value a send resolves against to pick this variant's templates + example: "acme" + label: + type: string + description: Human readable name shown in the console + example: "Acme Corp" + CreateTemplate: type: object required: @@ -7641,6 +7674,12 @@ components: type: string example: "en" description: The locale/language code for the template + variant: + type: string + example: "acme" + description: >- + The variant this template belongs to. Empty selects the default + variant, which is the branding every campaign starts with. data: type: object nullable: true @@ -7738,6 +7777,15 @@ components: type: array items: $ref: "#/components/schemas/CampaignVariable" + variants: + type: array + items: + $ref: "#/components/schemas/CampaignVariant" + variant_selector: + type: string + nullable: true + description: Liquid expression resolved per recipient when a send does not name a variant itself + example: "{{ user.data.tenant }}" delivery: $ref: "#/components/schemas/Delivery" archived: @@ -8098,6 +8146,7 @@ components: - data - type - locale + - variant - created_at - updated_at properties: @@ -8132,6 +8181,10 @@ components: locale: type: string example: "en" + variant: + type: string + example: "acme" + description: The variant this template belongs to. Empty is the default variant. sender_identity_id: type: string format: uuid @@ -10498,6 +10551,14 @@ components: type: string format: date-time description: Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. + variant: + type: string + nullable: true + description: >- + Pins every message in this broadcast to one template variant. Leave + unset to let the campaign's variant selector resolve a variant per + recipient, which is what a mixed-tenant list needs. + example: "acme" UpdateBroadcast: type: object @@ -10540,6 +10601,10 @@ components: list_type: type: string description: Snapshot of the list type at broadcast creation time + variant: + type: string + nullable: true + description: Template variant every message in this broadcast is pinned to state: $ref: "#/components/schemas/BroadcastState" total: diff --git a/internal/http/controllers/v1/management/oapi/resources_gen.go b/internal/http/controllers/v1/management/oapi/resources_gen.go index 2393c69ef..6fa7dd623 100644 --- a/internal/http/controllers/v1/management/oapi/resources_gen.go +++ b/internal/http/controllers/v1/management/oapi/resources_gen.go @@ -953,6 +953,9 @@ type Broadcast struct { // Total Total number of users in the audience at send time Total int `json:"total"` UpdatedAt time.Time `json:"updated_at"` + + // Variant Template variant every message in this broadcast is pinned to + Variant *string `json:"variant,omitempty"` } // BroadcastState Current state of the broadcast @@ -975,6 +978,10 @@ type Campaign struct { Transactional bool `json:"transactional"` UpdatedAt time.Time `json:"updated_at"` Variables *[]CampaignVariable `json:"variables,omitempty"` + + // VariantSelector Liquid expression resolved per recipient when a send does not name a variant itself + VariantSelector *string `json:"variant_selector,omitempty"` + Variants *[]CampaignVariant `json:"variants,omitempty"` } // CampaignUser defines model for CampaignUser. @@ -1000,6 +1007,15 @@ type CampaignVariable struct { Name string `json:"name"` } +// CampaignVariant defines model for CampaignVariant. +type CampaignVariant struct { + // Key The value a send resolves against to pick this variant's templates + Key string `json:"key"` + + // Label Human readable name shown in the console + Label *string `json:"label,omitempty"` +} + // ChangePasswordRequest defines model for ChangePasswordRequest. type ChangePasswordRequest struct { // CurrentPassword The password currently on the account @@ -1074,6 +1090,9 @@ type CreateBroadcast struct { // ScheduledAt Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. ScheduledAt *time.Time `json:"scheduled_at,omitempty"` + + // Variant Pins every message in this broadcast to one template variant. Leave unset to let the campaign's variant selector resolve a variant per recipient, which is what a mixed-tenant list needs. + Variant *string `json:"variant,omitempty"` } // CreateCampaign defines model for CreateCampaign. @@ -1228,6 +1247,9 @@ type CreateTemplate struct { // SenderIdentityId The ID of the sender identity to use for this template SenderIdentityId *openapi_types.UUID `json:"sender_identity_id,omitempty"` + + // Variant The variant this template belongs to. Empty selects the default variant, which is the branding every campaign starts with. + Variant *string `json:"variant,omitempty"` } // CreateUserDevice defines model for CreateUserDevice. @@ -2098,6 +2120,9 @@ type Template struct { // Type Communication channel type Type Channel `json:"type"` UpdatedAt time.Time `json:"updated_at"` + + // Variant The variant this template belongs to. Empty is the default variant. + Variant string `json:"variant"` } // TestActionFunctionRequest defines model for TestActionFunctionRequest. @@ -2191,6 +2216,10 @@ type UpdateCampaign struct { SubscriptionId *openapi_types.UUID `json:"subscription_id,omitempty"` Transactional *bool `json:"transactional,omitempty"` Variables *[]CampaignVariable `json:"variables,omitempty"` + + // VariantSelector Liquid expression resolved per recipient when a send does not name a variant itself, for example "{{ user.data.tenant }}". An expression that resolves to an unknown variant falls back to the default one. + VariantSelector *string `json:"variant_selector,omitempty"` + Variants *[]CampaignVariant `json:"variants,omitempty"` } // UpdateJourney defines model for UpdateJourney. diff --git a/internal/http/controllers/v1/management/template_variants.go b/internal/http/controllers/v1/management/template_variants.go new file mode 100644 index 000000000..db8196a43 --- /dev/null +++ b/internal/http/controllers/v1/management/template_variants.go @@ -0,0 +1,12 @@ +//go:build !enterprise + +package v1 + +// variantsAvailable reports whether a project may configure template variants. +// +// Variants are an enterprise capability, so open-source builds refuse to +// declare them or to create a template for one. The send path deliberately has +// no equivalent guard: with no variant configurable here, every template is the +// default variant and selection resolves to it on its own, which keeps one code +// path through the render hot loop for both builds. +const variantsAvailable = false diff --git a/internal/http/controllers/v1/management/template_variants_enterprise.go b/internal/http/controllers/v1/management/template_variants_enterprise.go new file mode 100644 index 000000000..23dc0f0ca --- /dev/null +++ b/internal/http/controllers/v1/management/template_variants_enterprise.go @@ -0,0 +1,5 @@ +//go:build enterprise + +package v1 + +const variantsAvailable = true diff --git a/internal/http/controllers/v1/management/template_variants_enterprise_test.go b/internal/http/controllers/v1/management/template_variants_enterprise_test.go new file mode 100644 index 000000000..5d5a65dac --- /dev/null +++ b/internal/http/controllers/v1/management/template_variants_enterprise_test.go @@ -0,0 +1,178 @@ +//go:build enterprise + +package v1 + +import ( + "bytes" + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" + "github.com/lunogram/platform/internal/ptr" + "github.com/lunogram/platform/internal/pubsub" + "github.com/lunogram/platform/internal/rbac" + "github.com/lunogram/platform/internal/store" + "github.com/lunogram/platform/internal/store/management" + teststore "github.com/lunogram/platform/internal/store/test" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +// A template may only be created for a variant the campaign declares. Without +// that check a typo produces a template no send can ever resolve to and the +// console has no name to list it under. +func TestCreateTemplateRejectsUndeclaredVariant(t *testing.T) { + t.Parallel() + + logger := zaptest.NewLogger(t) + ctx := t.Context() + mgmt, _, _ := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + campaigns := management.NewCampaignsStore(mgmt) + campaignID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, + Name: "Test Campaign", + Channel: "email", + }) + require.NoError(t, err) + + err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ + Variants: &store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, + }, + }) + require.NoError(t, err) + + actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), + rbac.WithOrganizationID(uuid.New()), + rbac.WithProjectID(projectID), + ) + engine, actorCtx := rbac.TestSetup(t, ctx, actor, "owner", "admin") + + controller := NewTemplatesController(logger, mgmt, mgmt, pubsub.NewEmailRenderer(pubsub.NewNoopCaller()), nil, engine, nil, "") + + tests := map[string]struct { + variant *string + code int + }{ + "declared variant": {variant: ptr.To("acme"), code: 201}, + "undeclared variant": {variant: ptr.To("globex"), code: 400}, + "default variant": {variant: nil, code: 201}, + "empty variant": {variant: ptr.To(""), code: 201}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + body, err := json.Marshal(oapi.CreateTemplate{Locale: "en", Variant: test.variant}) + require.NoError(t, err) + + res := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/v1/campaigns/"+campaignID.String()+"/templates", bytes.NewReader(body)) + req = req.WithContext(actorCtx) + controller.CreateTemplate(res, req, projectID, campaignID) + + require.Equal(t, test.code, res.Code, res.Body.String()) + }) + } +} + +// Variants and their selector round-trip through the campaign update path, and +// an empty selector clears it rather than being ignored as a no-op. +func TestUpdateCampaignVariants(t *testing.T) { + t.Parallel() + + ctx := t.Context() + mgmt, _, _ := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + campaigns := management.NewCampaignsStore(mgmt) + campaignID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, + Name: "Test Campaign", + Channel: "email", + }) + require.NoError(t, err) + + campaign, err := campaigns.GetCampaign(ctx, projectID, campaignID) + require.NoError(t, err) + require.Empty(t, campaign.Variants.Data) + require.Nil(t, campaign.VariantSelector) + + err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ + Variants: &store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, + }, + VariantSelector: ptr.To("{{ user.data.tenant }}"), + }) + require.NoError(t, err) + + campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) + require.NoError(t, err) + require.Equal(t, management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, campaign.Variants.Data) + require.Equal(t, "{{ user.data.tenant }}", *campaign.VariantSelector) + + // An update that touches neither field leaves both alone. + err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ + Name: ptr.To("Renamed"), + }) + require.NoError(t, err) + + campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) + require.NoError(t, err) + require.Len(t, campaign.Variants.Data, 1) + require.NotNil(t, campaign.VariantSelector) + + err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ + VariantSelector: ptr.To(""), + }) + require.NoError(t, err) + + campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) + require.NoError(t, err) + require.Nil(t, campaign.VariantSelector) +} + +// Duplicating a campaign has to carry variant templates across, or the copy +// silently loses every white-labelled edition. +func TestDuplicateTemplateCarriesVariant(t *testing.T) { + t.Parallel() + + ctx := t.Context() + mgmt, _, _ := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + campaigns := management.NewCampaignsStore(mgmt) + templates := management.NewTemplatesStore(mgmt) + + sourceID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, Name: "Source", Channel: "email", + }) + require.NoError(t, err) + + targetID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, Name: "Target", Channel: "email", + }) + require.NoError(t, err) + + templateID, err := templates.CreateTemplate(ctx, projectID, sourceID, "email", "en", "acme", nil) + require.NoError(t, err) + + require.NoError(t, templates.DuplicateTemplate(ctx, projectID, templateID, targetID)) + + copied, err := templates.ListTemplates(ctx, projectID, targetID) + require.NoError(t, err) + require.Len(t, copied, 1) + require.Equal(t, "acme", copied[0].Variant) +} diff --git a/internal/http/controllers/v1/management/template_variants_test.go b/internal/http/controllers/v1/management/template_variants_test.go new file mode 100644 index 000000000..bba515b28 --- /dev/null +++ b/internal/http/controllers/v1/management/template_variants_test.go @@ -0,0 +1,71 @@ +//go:build !enterprise + +package v1 + +import ( + "bytes" + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" + "github.com/lunogram/platform/internal/ptr" + "github.com/lunogram/platform/internal/pubsub" + "github.com/lunogram/platform/internal/rbac" + "github.com/lunogram/platform/internal/store/management" + teststore "github.com/lunogram/platform/internal/store/test" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +// Variants are an enterprise capability. An open-source build must refuse to +// create one rather than accept a template no build of the console can manage. +func TestCreateTemplateRejectsVariantInOSS(t *testing.T) { + t.Parallel() + + logger := zaptest.NewLogger(t) + ctx := t.Context() + mgmt, _, _ := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + campaigns := management.NewCampaignsStore(mgmt) + campaignID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, + Name: "Test Campaign", + Channel: "email", + }) + require.NoError(t, err) + + actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), + rbac.WithOrganizationID(uuid.New()), + rbac.WithProjectID(projectID), + ) + engine, actorCtx := rbac.TestSetup(t, ctx, actor, "owner", "admin") + + controller := NewTemplatesController(logger, mgmt, mgmt, pubsub.NewEmailRenderer(pubsub.NewNoopCaller()), nil, engine, nil, "") + + body, err := json.Marshal(oapi.CreateTemplate{Locale: "en", Variant: ptr.To("acme")}) + require.NoError(t, err) + + res := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/v1/campaigns/"+campaignID.String()+"/templates", bytes.NewReader(body)) + req = req.WithContext(actorCtx) + controller.CreateTemplate(res, req, projectID, campaignID) + + require.Equal(t, 404, res.Code, res.Body.String()) + + // The default variant is not gated: every open-source template is one. + body, err = json.Marshal(oapi.CreateTemplate{Locale: "en"}) + require.NoError(t, err) + + res = httptest.NewRecorder() + req = httptest.NewRequest("POST", "/v1/campaigns/"+campaignID.String()+"/templates", bytes.NewReader(body)) + req = req.WithContext(actorCtx) + controller.CreateTemplate(res, req, projectID, campaignID) + + require.Equal(t, 201, res.Code, res.Body.String()) +} diff --git a/internal/http/controllers/v1/management/templates.go b/internal/http/controllers/v1/management/templates.go index 335e247aa..91289a670 100644 --- a/internal/http/controllers/v1/management/templates.go +++ b/internal/http/controllers/v1/management/templates.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "net/http" + "strings" "github.com/google/uuid" "github.com/jmoiron/sqlx" @@ -151,7 +152,25 @@ func (srv *TemplatesController) CreateTemplate(w http.ResponseWriter, r *http.Re senderIdentityID = &id } - templateID, err := srv.store.TemplatesStore.CreateTemplate(ctx, projectID, campaignID, campaign.Channel, body.Locale, senderIdentityID) + variant := "" + if body.Variant != nil { + variant = strings.TrimSpace(*body.Variant) + } + + if variant != "" && !variantsAvailable { + oapi.WriteProblem(w, problem.ErrNotFound(problem.Describe("template variants are not available in the open-source version"))) + return + } + + // A template for a variant the campaign never declared is unreachable: no + // send resolves to it, and the console has no name to show it under. Reject + // it here rather than letting it accumulate as a template nobody can find. + if !campaign.Variants.Data.Has(variant) { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("campaign does not declare variant "+variant))) + return + } + + templateID, err := srv.store.TemplatesStore.CreateTemplate(ctx, projectID, campaignID, campaign.Channel, body.Locale, variant, senderIdentityID) if err != nil { logger.Error("failed to create template", zap.Error(err)) oapi.WriteProblem(w, err) diff --git a/internal/http/controllers/v1/management/templates_test.go b/internal/http/controllers/v1/management/templates_test.go index 8930460be..0f68b4753 100644 --- a/internal/http/controllers/v1/management/templates_test.go +++ b/internal/http/controllers/v1/management/templates_test.go @@ -40,7 +40,7 @@ func TestGetTemplate(t *testing.T) { }) require.NoError(t, err) - templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -168,7 +168,7 @@ func TestUpdateTemplate(t *testing.T) { }) require.NoError(t, err) - templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), @@ -238,7 +238,7 @@ func TestDeleteTemplate(t *testing.T) { }) require.NoError(t, err) - templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", nil) + templateID, err := templates.CreateTemplate(ctx, projectID, campaignID, "email", "en", "", nil) require.NoError(t, err) actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), diff --git a/internal/journeys/campaign.go b/internal/journeys/campaign.go index 874890e63..390e1fc66 100644 --- a/internal/journeys/campaign.go +++ b/internal/journeys/campaign.go @@ -30,6 +30,19 @@ func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state j } } + // A step that names a variant decides the branding for this send outright, + // so it is resolved here against the journey context and travels with the + // event. Left unset, the campaign's own selector resolves one per recipient + // at render time. + var variant *string + if config.Variant != nil && *config.Variant != "" { + resolved, err := render.RenderString(*config.Variant, ctx.Data) + if err != nil { + return state, nil, fmt.Errorf("failed to render campaign variant: %w", err) + } + variant = &resolved + } + msg := schemas.SendCampaign{ ProjectID: ctx.ProjectID, UserID: ctx.UserID, @@ -40,6 +53,7 @@ func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state j JourneyStepID: &step.ExternalID, }, Variables: data, + Variant: variant, } err = ctx.Publisher.Publish(ctx, schemas.Subject(schemas.CampaignsSend(ctx.ProjectID, config.CampaignId)), msg) diff --git a/internal/node/metrics/metrics.go b/internal/node/metrics/metrics.go index 099372036..3de2f6cb9 100644 --- a/internal/node/metrics/metrics.go +++ b/internal/node/metrics/metrics.go @@ -89,6 +89,20 @@ var JourneyExperimentAssignmentsTotal = promauto.NewCounterVec(prometheus.Counte Help: "Total experiment/A-B test branch assignments", }, []string{"project_id", "branch"}) +// ============================================================================ +// Campaign Sends +// ============================================================================ + +// CampaignVariantSelectionsTotal counts template variant selections by +// outcome: "matched" when the requested variant had a template, "fallback" +// when it did not and the send used another variant's template instead. A +// fallback still delivers a message, so this counter is the only signal that a +// white-label template is missing. +var CampaignVariantSelectionsTotal = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "lunogram_campaign_variant_selections_total", + Help: "Total campaign template variant selections by outcome", +}, []string{"project_id", "outcome"}) + // ============================================================================ // Reconciliation (Leader Scheduler) // ============================================================================ diff --git a/internal/pubsub/consumer/broadcasts_enterprise.go b/internal/pubsub/consumer/broadcasts_enterprise.go index d68e0d222..01ce35bc2 100644 --- a/internal/pubsub/consumer/broadcasts_enterprise.go +++ b/internal/pubsub/consumer/broadcasts_enterprise.go @@ -111,6 +111,10 @@ func BroadcastBatchHandler(logger *zap.Logger, mgmt *management.State, usrs *sub UserID: userID, CampaignID: broadcast.CampaignID, BroadcastID: &event.BroadcastID, + // Nil when the broadcast is not pinned to one client, which + // leaves the campaign's selector to resolve a variant per + // recipient - what a list spanning several clients needs. + Variant: broadcast.Variant, } // Deterministic message ID so that if NATS redelivers this batch diff --git a/internal/pubsub/consumer/campaigns.go b/internal/pubsub/consumer/campaigns.go index 8c782f8c1..76b588597 100644 --- a/internal/pubsub/consumer/campaigns.go +++ b/internal/pubsub/consumer/campaigns.go @@ -130,6 +130,9 @@ func createCampaignInboxMessageAndPublish(ctx context.Context, db *sqlx.DB, pub "template_id": item.TemplateID.String(), "campaign_id": event.CampaignID.String(), } + if item.Variant != "" { + provenance["variant"] = item.Variant + } if event.BroadcastID != nil { provenance["broadcast_id"] = event.BroadcastID.String() } diff --git a/internal/pubsub/consumer/campaigns_render.go b/internal/pubsub/consumer/campaigns_render.go index 81b2da3e8..1eb6d6ba6 100644 --- a/internal/pubsub/consumer/campaigns_render.go +++ b/internal/pubsub/consumer/campaigns_render.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "github.com/lunogram/platform/internal/node/metrics" "github.com/lunogram/platform/internal/providers/channels" "github.com/lunogram/platform/internal/pubsub" "github.com/lunogram/platform/internal/pubsub/schemas" @@ -56,31 +57,96 @@ func userToMap(user *subjects.User) map[string]any { return m } -// selectTemplate picks the best template for a user based on locale. -// Priority: user's locale → project's default locale → first template. -func selectTemplate(templates management.Templates, user *subjects.User, project *management.Project) management.Template { - if len(templates) == 1 { - return templates[0] +// resolveVariant works out which template variant a send must use. +// +// An explicit variant on the event wins: the publisher - a journey step, or a +// broadcast pinned to one client - has already decided. Otherwise the +// campaign's selector is rendered against the same context the templates +// render against, so a project can drive branding off recipient data such as +// "{{ user.data.tenant }}" without configuring anything per send. +// +// A selector that fails to render resolves to the default variant. The +// expression is customer-authored and a broken one must not take a campaign +// down; the fallback is counted where the template is selected. +func resolveVariant(logger *zap.Logger, campaign *management.Campaign, event schemas.SendCampaign, data map[string]any) string { + if event.Variant != nil { + return strings.TrimSpace(*event.Variant) + } + + if campaign.VariantSelector == nil || *campaign.VariantSelector == "" { + return "" + } + + resolved, err := render.RenderString(*campaign.VariantSelector, data) + if err != nil { + logger.Warn("failed to render campaign variant selector, using default variant", + zap.Error(err), + zap.String("selector", *campaign.VariantSelector)) + return "" + } + + return strings.TrimSpace(resolved) +} + +// selectTemplate picks the template for a send, narrowing by variant before +// applying the locale rules. +// +// Locale priority within a variant is unchanged: user's locale → project's +// default locale → first template. A variant carrying no template for this +// campaign falls back to the default variant rather than failing, because a +// missing white-label template must not stop a message going out; callers see +// that happened by comparing the returned template's Variant against the one +// they asked for. +func selectTemplate(templates management.Templates, variant string, user *subjects.User, project *management.Project) (management.Template, error) { + if len(templates) == 0 { + return management.Template{}, Permanentf("campaign has no templates") + } + + candidates := templatesForVariant(templates, variant) + if len(candidates) == 0 { + candidates = templatesForVariant(templates, "") + } + + // Neither the requested variant nor the default has a template. Every + // remaining template belongs to some other variant, so there is no + // on-brand answer left - send one anyway rather than dropping the message, + // and let the caller's fallback counter surface it. + if len(candidates) == 0 { + candidates = templates + } + + if len(candidates) == 1 { + return candidates[0], nil } - byLocale := make(map[string]management.Template, len(templates)) - for _, t := range templates { + byLocale := make(map[string]management.Template, len(candidates)) + for _, t := range candidates { byLocale[t.Locale] = t } if user != nil && user.Locale != nil { if t, ok := byLocale[*user.Locale]; ok { - return t + return t, nil } } if project != nil { if t, ok := byLocale[project.Locale]; ok { - return t + return t, nil } } - return templates[0] + return candidates[0], nil +} + +func templatesForVariant(templates management.Templates, variant string) management.Templates { + matched := make(management.Templates, 0, len(templates)) + for _, template := range templates { + if template.Variant == variant { + matched = append(matched, template) + } + } + return matched } // buildRenderData builds the Liquid render context for a campaign send. @@ -127,7 +193,12 @@ type renderedCampaignInboxMessage struct { Channel providers.Channel SenderIdentityID *uuid.UUID TemplateID uuid.UUID - RenderedPayload json.RawMessage + // Variant is the variant actually rendered, which is not always the one + // the send asked for: a missing white-label template falls back to the + // default. Recording it makes the branding a recipient received auditable + // without re-deriving it from the template. + Variant string + RenderedPayload json.RawMessage } type renderedPushDispatch struct { @@ -146,13 +217,27 @@ type renderedPushPayload struct { } func renderCampaignInboxMessages(ctx context.Context, logger *zap.Logger, mgmt *management.State, usrs *subjects.State, renderer *pubsub.EmailRenderer, publicURL string, linkKey []byte, trackingURL string, event schemas.SendCampaign, campaign *management.Campaign, project *management.Project, user *subjects.User) ([]renderedCampaignInboxMessage, error) { - template := selectTemplate(campaign.Templates, user, project) channel := providers.Channel(campaign.Channel) data := buildRenderData(publicURL, user, campaign, event.Variables) + variant := resolveVariant(logger, campaign, event, data) + template, err := selectTemplate(campaign.Templates, variant, user, project) + if err != nil { + return nil, err + } + + outcome := "matched" + if template.Variant != variant { + outcome = "fallback" + logger.Warn("no template for requested variant, falling back", + zap.String("requested_variant", variant), + zap.String("selected_variant", template.Variant), + zap.Stringer("template_id", template.ID)) + } + metrics.CampaignVariantSelectionsTotal.WithLabelValues(event.ProjectID.String(), outcome).Inc() + switch channel { case providers.ChannelEmail: - var err error template.Data, err = channels.ComposeEmailTemplateData(ctx, renderer, event.ProjectID, template.Data, data) if err != nil { return nil, fmt.Errorf("compose email template data: %w", err) @@ -163,7 +248,6 @@ func renderCampaignInboxMessages(ctx context.Context, logger *zap.Logger, mgmt * return nil, Permanent(err) } case providers.ChannelSMS, providers.ChannelPush, providers.ChannelInbox: - var err error template.Data, err = render.RenderJSON(template.Data, data) if err != nil { return nil, Permanent(err) @@ -180,6 +264,7 @@ func renderCampaignInboxMessages(ctx context.Context, logger *zap.Logger, mgmt * return []renderedCampaignInboxMessage{{ Channel: channel, TemplateID: template.ID, + Variant: template.Variant, RenderedPayload: template.Data, }}, nil } @@ -236,6 +321,7 @@ func renderCampaignInboxMessages(ctx context.Context, logger *zap.Logger, mgmt * return []renderedCampaignInboxMessage{{ Channel: channel, TemplateID: template.ID, + Variant: template.Variant, RenderedPayload: payload, }}, nil } @@ -275,6 +361,7 @@ func renderCampaignInboxMessages(ctx context.Context, logger *zap.Logger, mgmt * Channel: channel, SenderIdentityID: template.SenderIdentityID, TemplateID: template.ID, + Variant: template.Variant, RenderedPayload: request.Payload, }}, nil } diff --git a/internal/pubsub/consumer/campaigns_variant_test.go b/internal/pubsub/consumer/campaigns_variant_test.go new file mode 100644 index 000000000..62005972e --- /dev/null +++ b/internal/pubsub/consumer/campaigns_variant_test.go @@ -0,0 +1,184 @@ +package consumer + +import ( + "testing" + + "github.com/google/uuid" + "github.com/lunogram/platform/internal/ptr" + "github.com/lunogram/platform/internal/pubsub/schemas" + "github.com/lunogram/platform/internal/store/management" + "github.com/lunogram/platform/internal/store/subjects" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +func template(variant, locale string) management.Template { + return management.Template{ID: uuid.New(), Variant: variant, Locale: locale} +} + +func TestSelectTemplate(t *testing.T) { + t.Parallel() + + house := template("", "en") + houseNL := template("", "nl") + acme := template("acme", "en") + acmeNL := template("acme", "nl") + + project := &management.Project{Locale: "en"} + + tests := []struct { + name string + templates management.Templates + variant string + user *subjects.User + want management.Template + }{ + { + name: "picks the requested variant over the default", + templates: management.Templates{house, acme}, + variant: "acme", + user: &subjects.User{Locale: ptr.To("en")}, + want: acme, + }, + { + name: "applies locale within the requested variant", + templates: management.Templates{house, houseNL, acme, acmeNL}, + variant: "acme", + user: &subjects.User{Locale: ptr.To("nl")}, + want: acmeNL, + }, + { + name: "falls back to the project locale within the variant", + templates: management.Templates{house, acme, acmeNL}, + variant: "acme", + user: &subjects.User{Locale: ptr.To("de")}, + want: acme, + }, + { + name: "falls back to the default variant when the variant has no template", + templates: management.Templates{house, houseNL}, + variant: "acme", + user: &subjects.User{Locale: ptr.To("nl")}, + want: houseNL, + }, + { + // The single-template shortcut must not run before the variant + // filter, or a campaign holding one variant template would answer + // every send with it. + name: "does not hand a lone variant template to a default send", + templates: management.Templates{acme, house}, + variant: "", + user: nil, + want: house, + }, + { + // No on-brand answer exists. Sending the wrong branding beats + // dropping the message; the caller counts this as a fallback. + name: "sends something when neither the variant nor the default has a template", + templates: management.Templates{acme}, + variant: "globex", + user: nil, + want: acme, + }, + { + name: "uses the only template when it is the default variant", + templates: management.Templates{house}, + variant: "", + user: nil, + want: house, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + got, err := selectTemplate(test.templates, test.variant, test.user, project) + require.NoError(t, err) + require.Equal(t, test.want.ID, got.ID) + }) + } +} + +func TestSelectTemplateWithoutTemplates(t *testing.T) { + t.Parallel() + + _, err := selectTemplate(nil, "", nil, &management.Project{Locale: "en"}) + require.Error(t, err) +} + +func TestResolveVariant(t *testing.T) { + t.Parallel() + + logger := zaptest.NewLogger(t) + data := map[string]any{ + "user": map[string]any{ + "data": map[string]any{"tenant": "acme"}, + }, + } + + tests := []struct { + name string + selector *string + event schemas.SendCampaign + want string + }{ + { + name: "explicit variant on the event wins over the selector", + selector: ptr.To("{{ user.data.tenant }}"), + event: schemas.SendCampaign{Variant: ptr.To("globex")}, + want: "globex", + }, + { + name: "renders the campaign selector when the event names nothing", + selector: ptr.To("{{ user.data.tenant }}"), + want: "acme", + }, + { + name: "trims whitespace a Liquid expression leaves behind", + selector: ptr.To(" {{ user.data.tenant }} "), + want: "acme", + }, + { + name: "resolves to the default variant when the selector matches nothing", + selector: ptr.To("{{ user.data.missing }}"), + want: "", + }, + { + name: "resolves to the default variant when no selector is configured", + want: "", + }, + { + name: "a broken expression resolves to the default variant rather than failing the send", + selector: ptr.To("{{ user.data.tenant | }}"), + want: "", + }, + { + // A selector with no Liquid in it is a static pin, which is how a + // campaign sends every message under one brand. + name: "a literal selector is used as the variant key", + selector: ptr.To("acme"), + want: "acme", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + campaign := &management.Campaign{VariantSelector: test.selector} + require.Equal(t, test.want, resolveVariant(logger, campaign, test.event, data)) + }) + } +} + +func TestCampaignVariantsHas(t *testing.T) { + t.Parallel() + + variants := management.CampaignVariants{{Key: "acme"}, {Key: "globex"}} + + require.True(t, variants.Has("acme")) + require.True(t, variants.Has(""), "the default variant always exists") + require.False(t, variants.Has("initech")) + require.True(t, management.CampaignVariants{}.Has("")) +} diff --git a/internal/pubsub/schemas/events.go b/internal/pubsub/schemas/events.go index 9a0a92aa4..4593db496 100644 --- a/internal/pubsub/schemas/events.go +++ b/internal/pubsub/schemas/events.go @@ -136,6 +136,10 @@ type SendCampaign struct { BroadcastID *uuid.UUID `json:"broadcast_id,omitempty"` Data *SendCampaignData `json:"data,omitempty"` Variables map[string]string `json:"variables,omitempty"` + // Variant names the template variant this send must use, already resolved + // by the publisher. Nil hands the choice to the campaign's variant + // selector, which resolves one per recipient at render time. + Variant *string `json:"variant,omitempty"` } // InboxOrigin resolves the inbox source label and the external_id key used diff --git a/internal/store/management/broadcasts.go b/internal/store/management/broadcasts.go index 108b9d8bc..603d14ea9 100644 --- a/internal/store/management/broadcasts.go +++ b/internal/store/management/broadcasts.go @@ -25,13 +25,17 @@ const ( type Broadcasts []Broadcast type Broadcast struct { - ID uuid.UUID `db:"id"` - ProjectID uuid.UUID `db:"project_id"` - CampaignID uuid.UUID `db:"campaign_id"` - ListID uuid.UUID `db:"list_id"` - ListName string `db:"list_name"` - ListType string `db:"list_type"` - State BroadcastState `db:"state"` + ID uuid.UUID `db:"id"` + ProjectID uuid.UUID `db:"project_id"` + CampaignID uuid.UUID `db:"campaign_id"` + ListID uuid.UUID `db:"list_id"` + ListName string `db:"list_name"` + ListType string `db:"list_type"` + // Variant pins every message in this broadcast to one template variant. Nil + // leaves the choice to the campaign's variant selector, which resolves a + // variant per recipient - what a list spanning several clients needs. + Variant *string `db:"variant"` + State BroadcastState `db:"state"` // Total is the number of messages published to NATS during broadcast // processing. Sent tracks the number of messages actually delivered, and // Failed the number that terminally will not be - a suppressed recipient or @@ -61,6 +65,7 @@ func (b Broadcast) OAPI() oapi.Broadcast { ListId: b.ListID, ListName: b.ListName, ListType: b.ListType, + Variant: b.Variant, State: oapi.BroadcastState(b.State), Total: b.Total, Sent: &b.Sent, @@ -117,12 +122,12 @@ func (s *BroadcastsStore) CreateBroadcast(ctx context.Context, broadcast Broadca } stmt := ` - INSERT INTO campaign_broadcasts (project_id, campaign_id, list_id, list_name, list_type, state, scheduled_at) - VALUES ($1, $2, $3, $4, $5, $6, $7) - RETURNING id, project_id, campaign_id, list_id, list_name, list_type, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at` + INSERT INTO campaign_broadcasts (project_id, campaign_id, list_id, list_name, list_type, state, scheduled_at, variant) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + RETURNING id, project_id, campaign_id, list_id, list_name, list_type, variant, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at` var result Broadcast - err := s.db.GetContext(ctx, &result, stmt, broadcast.ProjectID, broadcast.CampaignID, broadcast.ListID, broadcast.ListName, broadcast.ListType, string(state), broadcast.ScheduledAt) + err := s.db.GetContext(ctx, &result, stmt, broadcast.ProjectID, broadcast.CampaignID, broadcast.ListID, broadcast.ListName, broadcast.ListType, string(state), broadcast.ScheduledAt, broadcast.Variant) if err != nil { return Broadcast{}, err } @@ -133,7 +138,7 @@ func (s *BroadcastsStore) CreateBroadcast(ctx context.Context, broadcast Broadca func (s *BroadcastsStore) GetBroadcast(ctx context.Context, projectID, broadcastID uuid.UUID) (*Broadcast, error) { query := ` SELECT - cb.id, cb.project_id, cb.campaign_id, cb.list_id, cb.list_name, cb.list_type, + cb.id, cb.project_id, cb.campaign_id, cb.list_id, cb.list_name, cb.list_type, cb.variant, cb.state, cb.total, cb.sent, cb.failed, cb.error, cb.scheduled_at, cb.created_at, cb.updated_at, cb.started_at, cb.completed_at, c.name AS campaign_name, c.channel AS campaign_channel FROM campaign_broadcasts cb @@ -164,7 +169,7 @@ func (s *BroadcastsStore) GetBroadcast(ctx context.Context, projectID, broadcast func (s *BroadcastsStore) ListBroadcasts(ctx context.Context, projectID uuid.UUID, pagination store.Pagination, search string, campaignID *uuid.UUID, listID *uuid.UUID, state *BroadcastState) (Broadcasts, int, error) { query := ` SELECT - cb.id, cb.project_id, cb.campaign_id, cb.list_id, cb.list_name, cb.list_type, + cb.id, cb.project_id, cb.campaign_id, cb.list_id, cb.list_name, cb.list_type, cb.variant, cb.state, cb.total, cb.sent, cb.failed, cb.error, cb.scheduled_at, cb.created_at, cb.updated_at, cb.started_at, cb.completed_at, c.name AS campaign_name, c.channel AS campaign_channel, COUNT(*) OVER () AS total_count @@ -380,7 +385,7 @@ func (s *BroadcastsStore) CancelBroadcast(ctx context.Context, projectID, broadc // briefly both act as leader. func (s *BroadcastsStore) ScanScheduledBroadcasts(ctx context.Context, limit int, scanner func(Broadcast) error) (int, error) { query := ` - SELECT id, project_id, campaign_id, list_id, list_name, list_type, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at + SELECT id, project_id, campaign_id, list_id, list_name, list_type, variant, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at FROM campaign_broadcasts WHERE state = 'scheduled' AND scheduled_at IS NOT NULL @@ -515,7 +520,7 @@ func (s *BroadcastsStore) GetBroadcastUsers(ctx context.Context, usersDB store.D // the consumer failed without updating state. func (s *BroadcastsStore) ScanStuckBroadcasts(ctx context.Context, stuckThreshold time.Duration, scanner func(Broadcast) error) (int, error) { query := ` - SELECT id, project_id, campaign_id, list_id, list_name, list_type, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at + SELECT id, project_id, campaign_id, list_id, list_name, list_type, variant, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at FROM campaign_broadcasts WHERE state = 'sending' AND started_at IS NOT NULL diff --git a/internal/store/management/campaigns.go b/internal/store/management/campaigns.go index 7c86a3336..462ae3c9b 100644 --- a/internal/store/management/campaigns.go +++ b/internal/store/management/campaigns.go @@ -26,19 +26,45 @@ type CampaignVariable struct { type CampaignVariables []CampaignVariable +// CampaignVariant declares one white-labelled edition of a campaign. Key is +// what a send resolves against to pick a set of templates; the empty key is the +// default variant every campaign starts with and is never declared here. +type CampaignVariant struct { + Key string `json:"key"` + Label string `json:"label,omitempty"` +} + +type CampaignVariants []CampaignVariant + +// Has reports whether key names a declared variant. The default variant is +// always available and is not part of the declared set. +func (variants CampaignVariants) Has(key string) bool { + if key == "" { + return true + } + for _, variant := range variants { + if variant.Key == key { + return true + } + } + return false +} + type Campaign struct { - ID uuid.UUID `db:"id"` - ProjectID uuid.UUID `db:"project_id"` - Name string `db:"name"` - Channel string `db:"channel"` - SubscriptionID *uuid.UUID `db:"subscription_id"` - Transactional bool `db:"transactional"` - Delivery store.JSONB[Delivery] `db:"delivery"` - Variables store.JSONB[CampaignVariables] `db:"variables"` - Templates Templates `db:"-"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` - DeletedAt *time.Time `db:"deleted_at"` + ID uuid.UUID `db:"id"` + ProjectID uuid.UUID `db:"project_id"` + Name string `db:"name"` + Channel string `db:"channel"` + SubscriptionID *uuid.UUID `db:"subscription_id"` + Transactional bool `db:"transactional"` + Delivery store.JSONB[Delivery] `db:"delivery"` + Variables store.JSONB[CampaignVariables] `db:"variables"` + Variants store.JSONB[CampaignVariants] `db:"variants"` + VariantSelector *string `db:"variant_selector"` + Templates Templates `db:"-"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` + DeletedAt *time.Time `db:"deleted_at"` } func (campaign Campaign) OAPI() oapi.Campaign { @@ -50,20 +76,30 @@ func (campaign Campaign) OAPI() oapi.Campaign { } } + variants := make([]oapi.CampaignVariant, len(campaign.Variants.Data)) + for i, v := range campaign.Variants.Data { + variants[i] = oapi.CampaignVariant{Key: v.Key} + if v.Label != "" { + variants[i].Label = &v.Label + } + } + archived := campaign.DeletedAt != nil result := oapi.Campaign{ - Id: campaign.ID, - ProjectId: campaign.ProjectID, - Name: campaign.Name, - Channel: oapi.Channel(campaign.Channel), - SubscriptionId: campaign.SubscriptionID, - Transactional: campaign.Transactional, - Delivery: campaign.Delivery.Data.OAPI(), - Variables: &variables, - Templates: campaign.Templates.OAPI(), - CreatedAt: campaign.CreatedAt, - UpdatedAt: campaign.UpdatedAt, - Archived: &archived, + Id: campaign.ID, + ProjectId: campaign.ProjectID, + Name: campaign.Name, + Channel: oapi.Channel(campaign.Channel), + SubscriptionId: campaign.SubscriptionID, + Transactional: campaign.Transactional, + Delivery: campaign.Delivery.Data.OAPI(), + Variables: &variables, + Variants: &variants, + VariantSelector: campaign.VariantSelector, + Templates: campaign.Templates.OAPI(), + CreatedAt: campaign.CreatedAt, + UpdatedAt: campaign.UpdatedAt, + Archived: &archived, } return result @@ -114,7 +150,7 @@ func (s *CampaignsStore) CreateCampaign(ctx context.Context, campaign Campaign) func (s *CampaignsStore) ListCampaigns(ctx context.Context, project uuid.UUID, pagination store.Pagination, search string, archivedOnly bool) (Campaigns, int, error) { query := ` - SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, created_at, updated_at, deleted_at, + SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, variant_selector, created_at, updated_at, deleted_at, COUNT(*) OVER () AS total_count FROM campaigns WHERE project_id = $1 @@ -147,7 +183,7 @@ func (s *CampaignsStore) ListCampaigns(ctx context.Context, project uuid.UUID, p func (s *CampaignsStore) GetCampaign(ctx context.Context, projectID, campaignID uuid.UUID) (*Campaign, error) { query := ` - SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, created_at, updated_at, deleted_at + SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, variant_selector, created_at, updated_at, deleted_at FROM campaigns WHERE project_id = $1 AND id = $2 @@ -172,6 +208,9 @@ type CampaignUpdate struct { SubscriptionID *uuid.UUID Transactional *bool Variables *store.JSONB[CampaignVariables] + Variants *store.JSONB[CampaignVariants] + // VariantSelector is a Liquid expression; an empty string clears it. + VariantSelector *string } func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaignID uuid.UUID, update CampaignUpdate) error { @@ -184,9 +223,15 @@ func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaign subscription_id = CASE WHEN COALESCE($3, transactional) THEN NULL ELSE COALESCE($4, subscription_id) + END, + variants = COALESCE($5, variants), + variant_selector = CASE + WHEN $6::text IS NULL THEN variant_selector + WHEN $6::text = '' THEN NULL + ELSE $6::text END - WHERE project_id = $5 - AND id = $6 + WHERE project_id = $7 + AND id = $8 AND deleted_at IS NULL` var variablesVal any @@ -198,7 +243,16 @@ func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaign variablesVal = v } - _, err := s.db.ExecContext(ctx, query, update.Name, variablesVal, update.Transactional, update.SubscriptionID, projectID, campaignID) + var variantsVal any + if update.Variants != nil { + v, err := update.Variants.Value() + if err != nil { + return err + } + variantsVal = v + } + + _, err := s.db.ExecContext(ctx, query, update.Name, variablesVal, update.Transactional, update.SubscriptionID, variantsVal, update.VariantSelector, projectID, campaignID) return err } diff --git a/internal/store/management/migrations/1787000000007_template_variants.down.sql b/internal/store/management/migrations/1787000000007_template_variants.down.sql new file mode 100644 index 000000000..cbc558572 --- /dev/null +++ b/internal/store/management/migrations/1787000000007_template_variants.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE campaign_broadcasts DROP COLUMN IF EXISTS variant; +ALTER TABLE campaigns DROP COLUMN IF EXISTS variant_selector; +ALTER TABLE campaigns DROP COLUMN IF EXISTS variants; +DROP INDEX IF EXISTS templates_campaign_variant_locale_idx; +ALTER TABLE templates DROP COLUMN IF EXISTS variant; diff --git a/internal/store/management/migrations/1787000000007_template_variants.up.sql b/internal/store/management/migrations/1787000000007_template_variants.up.sql new file mode 100644 index 000000000..10baaf9fd --- /dev/null +++ b/internal/store/management/migrations/1787000000007_template_variants.up.sql @@ -0,0 +1,27 @@ +-- A campaign's templates are keyed by locale today. White-labelling adds a +-- second, orthogonal dimension: the same campaign carries one template per +-- (locale, variant) pair, where the empty variant is the house brand every +-- existing row already belongs to. Defaulting to '' rather than NULL keeps the +-- lookup a plain equality match instead of forcing every query to spell out an +-- IS NULL branch. +ALTER TABLE templates ADD COLUMN variant VARCHAR(255) NOT NULL DEFAULT ''; + +-- Send-time selection narrows by campaign, then variant, then locale, which is +-- exactly this column order. No unique index yet: (campaign_id, locale) has +-- never been constrained, so existing projects may already hold duplicate rows +-- that a unique index over the new triple would refuse to build. +CREATE INDEX templates_campaign_variant_locale_idx ON templates(campaign_id, variant, locale); + +-- Variants are declared per campaign and edited on the campaign page, mirroring +-- how campaign variables are stored and edited. Each entry is {key, label}; the +-- label is what the console shows, the key is what a send resolves against. +ALTER TABLE campaigns ADD COLUMN variants JSONB NOT NULL DEFAULT '[]'::jsonb; + +-- A Liquid expression evaluated per recipient when a send does not name a +-- variant outright -- e.g. '{{ user.data.tenant }}'. This is what lets a single +-- broadcast over a mixed-tenant list resolve a different brand per recipient. +ALTER TABLE campaigns ADD COLUMN variant_selector TEXT; + +-- A broadcast that targets one client pins its variant here; left NULL, the +-- campaign selector decides per recipient. +ALTER TABLE campaign_broadcasts ADD COLUMN variant VARCHAR(255); diff --git a/internal/store/management/templates.go b/internal/store/management/templates.go index d1bfb1f6c..c18b2359a 100644 --- a/internal/store/management/templates.go +++ b/internal/store/management/templates.go @@ -27,6 +27,7 @@ type Template struct { Type string `db:"type"` Data json.RawMessage `db:"data"` Locale string `db:"locale"` + Variant string `db:"variant"` SenderIdentityID *uuid.UUID `db:"sender_identity_id"` CreatedAt time.Time `db:"created_at"` UpdatedAt time.Time `db:"updated_at"` @@ -39,6 +40,7 @@ func (template Template) OAPI() oapi.Template { Type: oapi.Channel(template.Type), Data: template.Data, Locale: template.Locale, + Variant: template.Variant, SenderIdentityId: template.SenderIdentityID, ProjectId: template.ProjectID, UpdatedAt: template.UpdatedAt, @@ -54,15 +56,15 @@ type TemplatesStore struct { db store.DB } -func (s *TemplatesStore) CreateTemplate(ctx context.Context, projectID, campaignID uuid.UUID, channel string, locale string, senderIdentityID *uuid.UUID) (uuid.UUID, error) { +func (s *TemplatesStore) CreateTemplate(ctx context.Context, projectID, campaignID uuid.UUID, channel string, locale string, variant string, senderIdentityID *uuid.UUID) (uuid.UUID, error) { // TODO: remove channel type, this type is needed within the "legacy" NodeJS back-end stmt := ` - INSERT INTO templates (project_id, campaign_id, type, locale, sender_identity_id) - VALUES ($1, $2, $3, $4, $5) + INSERT INTO templates (project_id, campaign_id, type, locale, variant, sender_identity_id) + VALUES ($1, $2, $3, $4, $5, $6) RETURNING id` var id uuid.UUID - err := s.db.QueryRowContext(ctx, stmt, projectID, campaignID, channel, locale, senderIdentityID).Scan(&id) + err := s.db.QueryRowContext(ctx, stmt, projectID, campaignID, channel, locale, variant, senderIdentityID).Scan(&id) if err != nil { return uuid.Nil, err } @@ -72,7 +74,7 @@ func (s *TemplatesStore) CreateTemplate(ctx context.Context, projectID, campaign func (s *TemplatesStore) GetTemplate(ctx context.Context, projectID, templateID uuid.UUID) (*Template, error) { query := ` - SELECT templates.id, templates.project_id, templates.campaign_id, campaigns.channel AS type, templates.data, templates.locale, templates.sender_identity_id, templates.created_at, templates.updated_at + SELECT templates.id, templates.project_id, templates.campaign_id, campaigns.channel AS type, templates.data, templates.locale, templates.variant, templates.sender_identity_id, templates.created_at, templates.updated_at FROM templates JOIN campaigns ON templates.campaign_id = campaigns.id WHERE templates.project_id = $1 @@ -89,7 +91,7 @@ func (s *TemplatesStore) GetTemplate(ctx context.Context, projectID, templateID func (s *TemplatesStore) ListTemplates(ctx context.Context, projectID, campaignID uuid.UUID) ([]Template, error) { query := ` - SELECT templates.id, templates.project_id, templates.campaign_id, campaigns.channel AS type, templates.data, templates.locale, templates.sender_identity_id, templates.created_at, templates.updated_at + SELECT templates.id, templates.project_id, templates.campaign_id, campaigns.channel AS type, templates.data, templates.locale, templates.variant, templates.sender_identity_id, templates.created_at, templates.updated_at FROM templates JOIN campaigns ON templates.campaign_id = campaigns.id WHERE templates.project_id = $1 @@ -139,8 +141,8 @@ func (s *TemplatesStore) DeleteTemplate(ctx context.Context, projectID, template func (s *TemplatesStore) DuplicateTemplate(ctx context.Context, projectID, templateID, newCampaignID uuid.UUID) error { query := ` - INSERT INTO templates (project_id, campaign_id, type, data, locale, sender_identity_id) - SELECT project_id, $1, type, data, locale, sender_identity_id + INSERT INTO templates (project_id, campaign_id, type, data, locale, variant, sender_identity_id) + SELECT project_id, $1, type, data, locale, variant, sender_identity_id FROM templates WHERE project_id = $2 AND id = $3` From 8d1bb4089d7ccab63ee0020a5a4d246cd8025642 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Mon, 31 Aug 2026 14:32:27 +0200 Subject: [PATCH 2/5] refactor(console): drop unused VariantUpdateParams type The type had no reference anywhere in the repo and now reads as though it belongs to template variants, which it never did - a reader hitting it next to CampaignVariant would reasonably assume the two are related. --- console/src/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/console/src/types.ts b/console/src/types.ts index d004d60a1..eb07ae647 100644 --- a/console/src/types.ts +++ b/console/src/types.ts @@ -895,7 +895,6 @@ export type Template< export type TemplateCreateParams = Pick & { variant?: string } export type TemplateUpdateParams = Pick -export type VariantUpdateParams = { id?: UUID } export interface TemplatePreviewParams { user: Record From 58947612e7f7c9ac6d620c504490abdf20a2dd0c Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Mon, 31 Aug 2026 14:50:18 +0200 Subject: [PATCH 3/5] refactor(templates): make the variant selector one explicit type The first cut split a campaign's variants across two sibling fields - an array of declared variants and a separate variant_selector string - and then let each call site invent its own rule. A broadcast could only pin a declared key and never write an expression; a journey step could only write an expression and was never validated against the declared set. Neither could do what the other did, and a static key worked in the journey only because render.RenderString happens to return its input untouched when it contains no "{{". Variants now travel as one object holding both the declared options and the rule that picks between them, and that rule is a VariantSelector - either {type: static, key} or {type: expression, expression} - shared by the campaign, the journey campaign step and the broadcast. The mode is declared rather than inferred from whether the string happens to contain Liquid syntax. Making it explicit buys two things beyond symmetry. A static key is now validated wherever it is written, so a stale or mistyped one is refused on save instead of quietly falling back to house branding on every send forever - previously only broadcasts checked this. And the console offers one component with a mode toggle rather than a dropdown at one call site and a Liquid box at another, hardcoded per surface. A broadcast passes its selector through to the send unresolved, since an expression there has to run once per recipient. A journey step still resolves before publishing and sends a static selector, because the journey context it reads - entrance data, earlier step state - is gone by the time the send is rendered. campaign_broadcasts.variant becomes JSONB to hold a selector. The migration is rewritten in place rather than stacked on, since the array shape it replaces was never merged. Also renames the template editor's VariantSelect to VariantSwitcher: it navigates the editor between variants and has nothing to do with what a send resolves to, which the old name did not distinguish. --- console/src/oapi/management.generated.ts | 54 ++--- console/src/types.ts | 20 +- .../broadcast/broadcast-response.ts | 8 +- .../validation/broadcast/create-broadcast.ts | 8 +- .../views/broadcast/CreateBroadcastDialog.tsx | 21 +- .../src/views/campaign/CampaignDetails.tsx | 13 +- .../src/views/campaign/CampaignVariants.tsx | 66 ++++--- .../views/campaign/VariantSelectorInput.tsx | 127 ++++++++++++ .../src/views/campaign/template/Template.tsx | 8 +- ...{VariantSelect.tsx => VariantSwitcher.tsx} | 10 +- console/src/views/journey/steps/Campaign.tsx | 21 +- .../v1/management/broadcasts_enterprise.go | 22 ++- .../controllers/v1/management/campaigns.go | 37 +--- .../v1/management/oapi/journeys.go | 6 +- .../v1/management/oapi/resources.yml | 81 ++++---- .../v1/management/oapi/resources_gen.go | 60 +++++- .../template_variants_enterprise_test.go | 178 +++++++++++++++-- internal/journeys/campaign.go | 26 ++- .../pubsub/consumer/broadcasts_enterprise.go | 8 +- internal/pubsub/consumer/campaigns_render.go | 31 +-- .../pubsub/consumer/campaigns_variant_test.go | 117 +++++++++-- internal/pubsub/schemas/events.go | 14 +- internal/store/management/broadcasts.go | 29 ++- internal/store/management/campaigns.go | 105 ++++------ .../1787000000007_template_variants.down.sql | 1 - .../1787000000007_template_variants.up.sql | 24 +-- internal/store/management/variants.go | 186 ++++++++++++++++++ 27 files changed, 946 insertions(+), 335 deletions(-) create mode 100644 console/src/views/campaign/VariantSelectorInput.tsx rename console/src/views/campaign/template/{VariantSelect.tsx => VariantSwitcher.tsx} (86%) create mode 100644 internal/store/management/variants.go diff --git a/console/src/oapi/management.generated.ts b/console/src/oapi/management.generated.ts index 7d7a14567..431a6e1db 100644 --- a/console/src/oapi/management.generated.ts +++ b/console/src/oapi/management.generated.ts @@ -3039,11 +3039,7 @@ export interface components { * @example 52f3f921-1343-48af-b795-87c0fd3b44aa */ campaign_id: string; - /** - * @description Template variant to send, as a static key or a Liquid expression resolved against the journey context. Overrides the campaign's own variant selector. - * @example {{ user.data.tenant }} - */ - variant?: string; + variant?: components["schemas"]["VariantSelector"]; }; /** @description Data for action step - execute WASM action */ ActionStepData: { @@ -3335,12 +3331,7 @@ export interface components { /** @example false */ transactional?: boolean; variables?: components["schemas"]["CampaignVariable"][]; - variants?: components["schemas"]["CampaignVariant"][]; - /** - * @description Liquid expression resolved per recipient when a send does not name a variant itself, for example "{{ user.data.tenant }}". An expression that resolves to an unknown variant falls back to the default one. - * @example {{ user.data.tenant }} - */ - variant_selector?: string | null; + variants?: components["schemas"]["CampaignVariants"]; }; CampaignVariable: { /** @@ -3366,6 +3357,31 @@ export interface components { */ label?: string; }; + /** @description Decides which template variant a send uses. The same shape appears on a campaign, on a journey campaign step and on a broadcast; the most specific layer wins. A static selector pins one variant for every recipient and is rejected when the campaign does not declare its key. An expression selector resolves a variant per recipient and can only be judged at send time, so one that resolves to an unknown variant falls back to the default variant. */ + VariantSelector: { + /** + * @description Whether the variant is pinned or resolved per recipient + * @example expression + * @enum {string} + */ + type: "static" | "expression"; + /** + * @description Variant key. Required when type is static. + * @example acme + */ + key?: string; + /** + * @description Liquid expression. Required when type is expression. + * @example {{ user.data.tenant }} + */ + expression?: string; + }; + /** @description A campaign's declared variants together with the rule that picks between them when a send does not pick one itself. */ + CampaignVariants: { + selector?: components["schemas"]["VariantSelector"]; + /** @description The variants this campaign declares. The default variant is always available and is not listed here. */ + options?: components["schemas"]["CampaignVariant"][]; + }; CreateTemplate: { /** * @description The locale/language code for the template @@ -3441,12 +3457,7 @@ export interface components { transactional: boolean; templates: components["schemas"]["Template"][]; variables?: components["schemas"]["CampaignVariable"][]; - variants?: components["schemas"]["CampaignVariant"][]; - /** - * @description Liquid expression resolved per recipient when a send does not name a variant itself - * @example {{ user.data.tenant }} - */ - variant_selector?: string | null; + variants?: components["schemas"]["CampaignVariants"]; delivery: components["schemas"]["Delivery"]; /** * @description Whether the campaign has been archived @@ -5472,11 +5483,7 @@ export interface components { * @description Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. */ scheduled_at?: string; - /** - * @description Pins every message in this broadcast to one template variant. Leave unset to let the campaign's variant selector resolve a variant per recipient, which is what a mixed-tenant list needs. - * @example acme - */ - variant?: string | null; + variant?: components["schemas"]["VariantSelector"]; }; UpdateBroadcast: { /** @@ -5498,8 +5505,7 @@ export interface components { list_name: string; /** @description Snapshot of the list type at broadcast creation time */ list_type: string; - /** @description Template variant every message in this broadcast is pinned to */ - variant?: string | null; + variant?: components["schemas"]["VariantSelector"]; state: components["schemas"]["BroadcastState"]; /** * @description Total number of users in the audience at send time diff --git a/console/src/types.ts b/console/src/types.ts index eb07ae647..e653b23e3 100644 --- a/console/src/types.ts +++ b/console/src/types.ts @@ -746,6 +746,23 @@ export interface CampaignVariant { label?: string } +export type VariantSelectorType = "static" | "expression" + +/** + * Decides which template variant a send uses. The same shape appears on a + * campaign, a journey campaign step and a broadcast; the most specific wins. + */ +export interface VariantSelector { + type: VariantSelectorType + key?: string + expression?: string +} + +export interface CampaignVariants { + selector?: VariantSelector + options?: CampaignVariant[] +} + export interface Campaign { id: UUID project_id: UUID @@ -757,8 +774,7 @@ export interface Campaign { transactional?: boolean templates: Template[] variables: CampaignVariable[] - variants: CampaignVariant[] - variant_selector?: string | null + variants: CampaignVariants created_at: string updated_at: string } diff --git a/console/src/validation/broadcast/broadcast-response.ts b/console/src/validation/broadcast/broadcast-response.ts index fc8cea6d9..6af378664 100644 --- a/console/src/validation/broadcast/broadcast-response.ts +++ b/console/src/validation/broadcast/broadcast-response.ts @@ -7,7 +7,13 @@ export const broadcastResponseSchema = z.object({ list_id: z.string(), list_name: z.string(), list_type: z.enum(["static", "dynamic"]), - variant: z.string().nullish(), + variant: z + .object({ + type: z.enum(["static", "expression"]), + key: z.string().optional(), + expression: z.string().optional(), + }) + .nullish(), state: z.enum(["scheduled", "pending", "sending", "completed", "failed", "cancelled"]), total: z.number(), sent: z.number().optional().default(0), diff --git a/console/src/validation/broadcast/create-broadcast.ts b/console/src/validation/broadcast/create-broadcast.ts index 64fb977b2..1fe5c626f 100644 --- a/console/src/validation/broadcast/create-broadcast.ts +++ b/console/src/validation/broadcast/create-broadcast.ts @@ -6,7 +6,13 @@ export const createBroadcastSchema = z list_id: z.string().min(1, "List is required"), is_scheduled: z.boolean(), scheduled_at: z.string().optional(), - variant: z.string().optional(), + variant: z + .object({ + type: z.enum(["static", "expression"]), + key: z.string().optional(), + expression: z.string().optional(), + }) + .optional(), }) .refine((data) => !data.is_scheduled || data.scheduled_at, { message: "Scheduled time is required", diff --git a/console/src/views/broadcast/CreateBroadcastDialog.tsx b/console/src/views/broadcast/CreateBroadcastDialog.tsx index 54c44b035..678410239 100644 --- a/console/src/views/broadcast/CreateBroadcastDialog.tsx +++ b/console/src/views/broadcast/CreateBroadcastDialog.tsx @@ -47,7 +47,7 @@ const channelIcons: Record = { inbox: Inbox, } -import { VariantSelect } from "@/views/campaign/template/VariantSelect" +import { VariantSelectorInput } from "@/views/campaign/VariantSelectorInput" import { isEnterprise } from "@/config/enterprise" interface CreateBroadcastDialogProps { @@ -80,7 +80,6 @@ export function CreateBroadcastDialog({ list_id: preselectedListId ?? "", is_scheduled: false, scheduled_at: "", - variant: "", }, }) @@ -252,26 +251,24 @@ export function CreateBroadcastDialog({
{/* Variant Selector - only for campaigns that declare variants */} - {isEnterprise && (selectedCampaign?.variants?.length ?? 0) > 0 && ( + {isEnterprise && (selectedCampaign?.variants?.options?.length ?? 0) > 0 && (
( - )} /> -

- {t( - "broadcast.variant_description", - "Sends every message under one design. Pick Default to let the campaign choose per recipient.", - )} -

)} diff --git a/console/src/views/campaign/CampaignDetails.tsx b/console/src/views/campaign/CampaignDetails.tsx index 1da4e751a..9dae27534 100644 --- a/console/src/views/campaign/CampaignDetails.tsx +++ b/console/src/views/campaign/CampaignDetails.tsx @@ -40,8 +40,7 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: const [isBroadcastOpen, setIsBroadcastOpen] = useState(false) const [isTransactional, setTransactional] = useState(campaign.transactional ?? false) const [subscriptionId, setSubscriptionId] = useState(campaign.subscription_id ?? "") - const [variants, setVariants] = useState(campaign.variants ?? []) - const [variantSelector, setVariantSelector] = useState(campaign.variant_selector ?? "") + const [variants, setVariants] = useState(campaign.variants ?? {}) const [subscriptions] = useResolver( useCallback(async (): Promise => { @@ -93,8 +92,10 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: subscription_id: isTransactional ? undefined : effectiveSubscriptionId || undefined, variables: data.variables.filter((v) => v.name), ...(isEnterprise && { - variants: variants.filter((v) => v.key), - variant_selector: variantSelector, + variants: { + ...variants, + options: (variants.options ?? []).filter((v) => v.key), + }, }), }) @@ -179,11 +180,9 @@ function CampaignReview({ campaign, template }: { campaign: Campaign; template: )} diff --git a/console/src/views/campaign/CampaignVariants.tsx b/console/src/views/campaign/CampaignVariants.tsx index aef61204b..2155bc692 100644 --- a/console/src/views/campaign/CampaignVariants.tsx +++ b/console/src/views/campaign/CampaignVariants.tsx @@ -1,18 +1,18 @@ -import { useCallback } from "react" +import { useCallback, useMemo } from "react" import { useTranslation } from "react-i18next" import { Palette, Plus, Trash2 } from "lucide-react" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" import { Label } from "@/components/ui/label" -import type { CampaignVariant, Template } from "@/types" +import type { CampaignVariant, CampaignVariants as CampaignVariantsValue, Template } from "@/types" + +import { VariantSelectorInput } from "./VariantSelectorInput" interface CampaignVariantsProps { - variants: CampaignVariant[] - selector: string + value: CampaignVariantsValue templates: Template[] - onChange: (variants: CampaignVariant[]) => void - onSelectorChange: (selector: string) => void + onChange: (variants: CampaignVariantsValue) => void } const VARIANT_KEY_REGEX = /^[a-z0-9][a-z0-9_-]*$/ @@ -25,31 +25,42 @@ function validateKey(key: string, variants: CampaignVariant[], index: number): s return undefined } -export function CampaignVariants({ - variants, - selector, - templates, - onChange, - onSelectorChange, -}: CampaignVariantsProps) { +export function CampaignVariants({ value, templates, onChange }: CampaignVariantsProps) { const { t } = useTranslation() + const variants = useMemo(() => value.options ?? [], [value.options]) + + const setOptions = useCallback( + (options: CampaignVariant[]) => { + // Dropping the variant a static selector points at would leave the + // campaign pinned to something it no longer declares, which the API + // rejects on save. Clear the selector instead. + const selector = + value.selector?.type === "static" && + !options.some((option) => option.key === value.selector?.key) + ? undefined + : value.selector + onChange({ selector, options }) + }, + [value.selector, onChange], + ) + const addVariant = useCallback(() => { - onChange([...variants, { key: "" }]) - }, [variants, onChange]) + setOptions([...variants, { key: "" }]) + }, [variants, setOptions]) const updateVariant = useCallback( (index: number, updates: Partial) => { - onChange(variants.map((v, i) => (i === index ? { ...v, ...updates } : v))) + setOptions(variants.map((v, i) => (i === index ? { ...v, ...updates } : v))) }, - [variants, onChange], + [variants, setOptions], ) const removeVariant = useCallback( (index: number) => { - onChange(variants.filter((_, i) => i !== index)) + setOptions(variants.filter((_, i) => i !== index)) }, - [variants, onChange], + [variants, setOptions], ) const editor = @@ -157,20 +168,19 @@ export function CampaignVariants({ {variants.length > 0 && (
- onSelectorChange(e.target.value)} - placeholder="{{ user.data.tenant }}" - className="h-8 bg-background font-mono text-sm shadow-none" - /> -

+

{t( "campaign.variants.selector.description", - "Resolved per recipient when a journey step or broadcast does not pick a variant itself. A value that matches no variant falls back to the default design.", + "Used when a journey step or broadcast does not pick a variant itself.", )}

+ variant.key)} + onChange={(selector) => onChange({ ...value, selector })} + />
)}
diff --git a/console/src/views/campaign/VariantSelectorInput.tsx b/console/src/views/campaign/VariantSelectorInput.tsx new file mode 100644 index 000000000..fa410103e --- /dev/null +++ b/console/src/views/campaign/VariantSelectorInput.tsx @@ -0,0 +1,127 @@ +import { useCallback } from "react" +import { useTranslation } from "react-i18next" + +import { Input } from "@/components/ui/input" +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" +import { TemplateInput } from "@/components/ui/template-input" +import type { CampaignVariant, VariantSelector, VariantSelectorType } from "@/types" + +const NONE = "__none__" + +interface VariantSelectorInputProps { + value?: VariantSelector + options: CampaignVariant[] + onChange: (selector: VariantSelector | undefined) => void + /** Liquid variables offered by the surrounding context, when there are any. */ + variables?: React.ComponentProps["variables"] + /** Label for the "no selector" choice, which differs per surface. */ + emptyLabel?: string +} + +/** + * Picks a template variant either by pinning one or by writing a Liquid + * expression resolved per recipient. Shared by the campaign, the journey + * campaign step and the broadcast dialog so the three offer the same choices. + */ +export function VariantSelectorInput({ + value, + options, + onChange, + variables, + emptyLabel, +}: VariantSelectorInputProps) { + const { t } = useTranslation() + + const mode: string = value?.type ?? NONE + + const handleModeChange = useCallback( + (next: string) => { + if (next === NONE) return onChange(undefined) + onChange( + next === "static" + ? { type: "static", key: options[0]?.key ?? "" } + : { type: "expression", expression: "" }, + ) + }, + [onChange, options], + ) + + return ( +
+ + + {value?.type === "static" && ( + + )} + + {value?.type === "expression" && + (variables ? ( + + onChange({ type: "expression", expression }) + } + variables={variables} + placeholder="{{ user.data.tenant }}" + /> + ) : ( + + onChange({ type: "expression", expression: e.target.value }) + } + placeholder="{{ user.data.tenant }}" + className="h-9 font-mono text-sm shadow-none" + /> + ))} + + {value?.type === "expression" && ( +

+ {t( + "campaign.variants.selector.expression_help", + "A value matching no variant falls back to the default design.", + )} +

+ )} +
+ ) +} + +export type { VariantSelectorType } diff --git a/console/src/views/campaign/template/Template.tsx b/console/src/views/campaign/template/Template.tsx index dc2425185..e6e4acce1 100644 --- a/console/src/views/campaign/template/Template.tsx +++ b/console/src/views/campaign/template/Template.tsx @@ -8,7 +8,7 @@ import api from "@/api" import { Pagination, PaginationContent, PaginationItem } from "@/components/ui/pagination" import { LocaleSelect } from "@/components/locale/select" -import { VariantSelect } from "./VariantSelect" +import { VariantSwitcher } from "./VariantSwitcher" import { isEnterprise } from "@/config/enterprise" import { Button } from "@/components/ui/button" import { TemplateWorkflowContext } from "./contexts" @@ -256,9 +256,9 @@ export default function Template() { {templateId && (
- {isEnterprise && (campaign.variants?.length ?? 0) > 0 && ( - 0 && ( + diff --git a/console/src/views/campaign/template/VariantSelect.tsx b/console/src/views/campaign/template/VariantSwitcher.tsx similarity index 86% rename from console/src/views/campaign/template/VariantSelect.tsx rename to console/src/views/campaign/template/VariantSwitcher.tsx index 9c4c21431..50285e184 100644 --- a/console/src/views/campaign/template/VariantSelect.tsx +++ b/console/src/views/campaign/template/VariantSwitcher.tsx @@ -13,14 +13,20 @@ import type { CampaignVariant } from "@/types" export const DEFAULT_VARIANT_VALUE = "__default__" -interface VariantSelectProps { +/** + * Switches which variant's template the editor is showing. This navigates the + * editor; it does not decide what a send resolves to - that is + * VariantSelectorInput. + */ + +interface VariantSwitcherProps { variants: CampaignVariant[] value: string onChange: (variant: string) => void disabled?: boolean } -export function VariantSelect({ variants, value, onChange, disabled }: VariantSelectProps) { +export function VariantSwitcher({ variants, value, onChange, disabled }: VariantSwitcherProps) { const { t } = useTranslation() // The default variant is not a declared entry, so it gets a sentinel value: diff --git a/console/src/views/journey/steps/Campaign.tsx b/console/src/views/journey/steps/Campaign.tsx index 061efe875..c1377b183 100644 --- a/console/src/views/journey/steps/Campaign.tsx +++ b/console/src/views/journey/steps/Campaign.tsx @@ -1,6 +1,6 @@ import { useCallback } from "react" import api from "../../../api" -import type { Campaign, CampaignVariable, JourneyStepType } from "../../../types" +import type { Campaign, CampaignVariable, JourneyStepType, VariantSelector } from "../../../types" import type { VariableGroup } from "../JourneyVariableContext" import { Combobox } from "@/components/ui/combobox" import { Label } from "@/components/ui/label" @@ -17,11 +17,12 @@ import { Button } from "@/components/ui/button" import { TemplateInput } from "@/components/ui/template-input" import { useJourneyVariableContext } from "../JourneyVariableContext" import { isEnterprise } from "@/config/enterprise" +import { VariantSelectorInput } from "../../campaign/VariantSelectorInput" interface CampaignConfig { campaign_id: UUID data?: Record - variant?: string + variant?: VariantSelector } type CampaignOption = Campaign & { path: string } @@ -92,7 +93,7 @@ export const campaignStep: JourneyStepType = { ) const variables = campaign?.variables ?? [] - const campaignVariants = campaign?.variants ?? [] + const campaignVariants = campaign?.variants?.options ?? [] const journeyVariables = nodeId ? getVariablesForNode(nodeId) : [] const handleVariableChange = (name: string, newValue: string) => { @@ -183,14 +184,18 @@ export const campaignStep: JourneyStepType = {

{t( "journey.campaign.variant_description", - "Which design this step sends. Leave empty to let the campaign decide per recipient.", + "Which design this step sends.", )}

- onChange({ ...value, variant: newValue })} + onChange({ ...value, variant })} variables={journeyVariables} - placeholder={campaignVariants.map((v) => v.key).join(", ")} + emptyLabel={t( + "journey.campaign.variant_inherit", + "Whatever the campaign decides", + )} />
)} diff --git a/internal/http/controllers/v1/management/broadcasts_enterprise.go b/internal/http/controllers/v1/management/broadcasts_enterprise.go index 50f1bc9c9..2354fa687 100644 --- a/internal/http/controllers/v1/management/broadcasts_enterprise.go +++ b/internal/http/controllers/v1/management/broadcasts_enterprise.go @@ -6,7 +6,6 @@ import ( "database/sql" "errors" "net/http" - "strings" "time" "github.com/google/uuid" @@ -91,16 +90,19 @@ func (srv *BroadcastsController) CreateBroadcast(w http.ResponseWriter, r *http. // Pinning a broadcast to a variant the campaign never declared would send // the whole list the default branding while the operator believes they // picked a client, so refuse it outright rather than falling back. - var variant *string + var variant *management.VariantSelector if body.Variant != nil { - key := strings.TrimSpace(*body.Variant) - if key != "" { - if !campaign.Variants.Data.Has(key) { - oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("campaign does not declare variant "+key))) - return - } - variant = &key + if !variantsAvailable { + oapi.WriteProblem(w, problem.ErrNotFound(problem.Describe("template variants are not available in the open-source version"))) + return + } + + selector := management.VariantSelectorFromOAPI(*body.Variant) + if err := selector.Validate(campaign.Variants.Data); err != nil { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe(err.Error()))) + return } + variant = &selector } list, err := srv.usrs.GetList(ctx, projectID, body.ListId) @@ -123,7 +125,7 @@ func (srv *BroadcastsController) CreateBroadcast(w http.ResponseWriter, r *http. ListName: list.Name, ListType: string(list.Type), ScheduledAt: body.ScheduledAt, - Variant: variant, + Variant: store.JSONB[*management.VariantSelector]{Data: variant}, }) if err != nil { logger.Error("failed to create broadcast", zap.Error(err)) diff --git a/internal/http/controllers/v1/management/campaigns.go b/internal/http/controllers/v1/management/campaigns.go index 1d6448ff6..d44497fe1 100644 --- a/internal/http/controllers/v1/management/campaigns.go +++ b/internal/http/controllers/v1/management/campaigns.go @@ -4,7 +4,6 @@ import ( "database/sql" "errors" "net/http" - "strings" "github.com/google/uuid" "github.com/jmoiron/sqlx" @@ -252,46 +251,20 @@ func (srv *CampaignsController) UpdateCampaign(w http.ResponseWriter, r *http.Re updated.Variables = &store.JSONB[management.CampaignVariables]{Data: vars} } - if body.Variants != nil || body.VariantSelector != nil { + if body.Variants != nil { if !variantsAvailable { oapi.WriteProblem(w, problem.ErrNotFound(problem.Describe("template variants are not available in the open-source version"))) return } - } - - if body.Variants != nil { - variants := make(management.CampaignVariants, 0, len(*body.Variants)) - seen := make(map[string]bool, len(*body.Variants)) - for _, v := range *body.Variants { - key := strings.TrimSpace(v.Key) - - // The empty key is the default variant. It always exists and is - // never declared, so accepting it here would create a duplicate of - // something that cannot be removed. - if key == "" { - oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("variant key cannot be empty"))) - return - } - - if seen[key] { - oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("duplicate variant key "+key))) - return - } - seen[key] = true - variant := management.CampaignVariant{Key: key} - if v.Label != nil { - variant.Label = strings.TrimSpace(*v.Label) - } - variants = append(variants, variant) + variants, err := management.CampaignVariantsFromOAPI(*body.Variants) + if err != nil { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe(err.Error()))) + return } updated.Variants = &store.JSONB[management.CampaignVariants]{Data: variants} } - if body.VariantSelector != nil { - updated.VariantSelector = body.VariantSelector - } - if body.SubscriptionId != nil { subscription, err := srv.mgmt.SubscriptionsStore.GetSubscription(ctx, projectID, *body.SubscriptionId) if errors.Is(err, sql.ErrNoRows) { diff --git a/internal/http/controllers/v1/management/oapi/journeys.go b/internal/http/controllers/v1/management/oapi/journeys.go index 42a5884dd..7abaf14d8 100644 --- a/internal/http/controllers/v1/management/oapi/journeys.go +++ b/internal/http/controllers/v1/management/oapi/journeys.go @@ -229,12 +229,12 @@ type DelayStepData struct { } // CampaignStepData represents data for campaign step - send campaign. -// Variant is either a static variant key or a Liquid expression resolved -// against the journey context, and overrides the campaign's own selector. +// Variant overrides the campaign's own selector for this step; an expression is +// resolved against the journey context. type CampaignStepData struct { CampaignId uuid.UUID `json:"campaign_id" yaml:"campaign_id"` Data map[string]string `json:"data,omitempty" yaml:"data,omitempty"` - Variant *string `json:"variant,omitempty" yaml:"variant,omitempty"` + Variant *VariantSelector `json:"variant,omitempty" yaml:"variant,omitempty"` } // ActionStepData represents data for action step - execute WASM action diff --git a/internal/http/controllers/v1/management/oapi/resources.yml b/internal/http/controllers/v1/management/oapi/resources.yml index 4f5570717..b5f7c4333 100644 --- a/internal/http/controllers/v1/management/oapi/resources.yml +++ b/internal/http/controllers/v1/management/oapi/resources.yml @@ -7195,12 +7195,7 @@ components: example: "52f3f921-1343-48af-b795-87c0fd3b44aa" x-go-type: uuid.UUID variant: - type: string - description: >- - Template variant to send, as a static key or a Liquid expression - resolved against the journey context. Overrides the campaign's own - variant selector. - example: "{{ user.data.tenant }}" + $ref: "#/components/schemas/VariantSelector" ActionStepData: type: object @@ -7625,17 +7620,7 @@ components: items: $ref: "#/components/schemas/CampaignVariable" variants: - type: array - items: - $ref: "#/components/schemas/CampaignVariant" - variant_selector: - type: string - nullable: true - description: >- - Liquid expression resolved per recipient when a send does not name a - variant itself, for example "{{ user.data.tenant }}". An expression - that resolves to an unknown variant falls back to the default one. - example: "{{ user.data.tenant }}" + $ref: "#/components/schemas/CampaignVariants" CampaignVariable: type: object @@ -7665,6 +7650,47 @@ components: description: Human readable name shown in the console example: "Acme Corp" + VariantSelector: + type: object + description: >- + Decides which template variant a send uses. The same shape appears on a + campaign, on a journey campaign step and on a broadcast; the most + specific layer wins. A static selector pins one variant for every + recipient and is rejected when the campaign does not declare its key. An + expression selector resolves a variant per recipient and can only be + judged at send time, so one that resolves to an unknown variant falls + back to the default variant. + required: + - type + properties: + type: + type: string + enum: [static, expression] + description: Whether the variant is pinned or resolved per recipient + example: "expression" + key: + type: string + description: Variant key. Required when type is static. + example: "acme" + expression: + type: string + description: Liquid expression. Required when type is expression. + example: "{{ user.data.tenant }}" + + CampaignVariants: + type: object + description: >- + A campaign's declared variants together with the rule that picks between + them when a send does not pick one itself. + properties: + selector: + $ref: "#/components/schemas/VariantSelector" + options: + type: array + description: The variants this campaign declares. The default variant is always available and is not listed here. + items: + $ref: "#/components/schemas/CampaignVariant" + CreateTemplate: type: object required: @@ -7778,14 +7804,7 @@ components: items: $ref: "#/components/schemas/CampaignVariable" variants: - type: array - items: - $ref: "#/components/schemas/CampaignVariant" - variant_selector: - type: string - nullable: true - description: Liquid expression resolved per recipient when a send does not name a variant itself - example: "{{ user.data.tenant }}" + $ref: "#/components/schemas/CampaignVariants" delivery: $ref: "#/components/schemas/Delivery" archived: @@ -10552,13 +10571,7 @@ components: format: date-time description: Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. variant: - type: string - nullable: true - description: >- - Pins every message in this broadcast to one template variant. Leave - unset to let the campaign's variant selector resolve a variant per - recipient, which is what a mixed-tenant list needs. - example: "acme" + $ref: "#/components/schemas/VariantSelector" UpdateBroadcast: type: object @@ -10602,9 +10615,7 @@ components: type: string description: Snapshot of the list type at broadcast creation time variant: - type: string - nullable: true - description: Template variant every message in this broadcast is pinned to + $ref: "#/components/schemas/VariantSelector" state: $ref: "#/components/schemas/BroadcastState" total: diff --git a/internal/http/controllers/v1/management/oapi/resources_gen.go b/internal/http/controllers/v1/management/oapi/resources_gen.go index 6fa7dd623..c087b145c 100644 --- a/internal/http/controllers/v1/management/oapi/resources_gen.go +++ b/internal/http/controllers/v1/management/oapi/resources_gen.go @@ -598,6 +598,24 @@ func (e UpdateUserScheduledRequestResume) Valid() bool { } } +// Defines values for VariantSelectorType. +const ( + Expression VariantSelectorType = "expression" + Static VariantSelectorType = "static" +) + +// Valid indicates whether the value is a known member of the VariantSelectorType enum. +func (e VariantSelectorType) Valid() bool { + switch e { + case Expression: + return true + case Static: + return true + default: + return false + } +} + // Defines values for ListProjectInvitesParamsStatus. const ( Accepted ListProjectInvitesParamsStatus = "accepted" @@ -954,8 +972,8 @@ type Broadcast struct { Total int `json:"total"` UpdatedAt time.Time `json:"updated_at"` - // Variant Template variant every message in this broadcast is pinned to - Variant *string `json:"variant,omitempty"` + // Variant Decides which template variant a send uses. The same shape appears on a campaign, on a journey campaign step and on a broadcast; the most specific layer wins. A static selector pins one variant for every recipient and is rejected when the campaign does not declare its key. An expression selector resolves a variant per recipient and can only be judged at send time, so one that resolves to an unknown variant falls back to the default variant. + Variant *VariantSelector `json:"variant,omitempty"` } // BroadcastState Current state of the broadcast @@ -979,9 +997,8 @@ type Campaign struct { UpdatedAt time.Time `json:"updated_at"` Variables *[]CampaignVariable `json:"variables,omitempty"` - // VariantSelector Liquid expression resolved per recipient when a send does not name a variant itself - VariantSelector *string `json:"variant_selector,omitempty"` - Variants *[]CampaignVariant `json:"variants,omitempty"` + // Variants A campaign's declared variants together with the rule that picks between them when a send does not pick one itself. + Variants *CampaignVariants `json:"variants,omitempty"` } // CampaignUser defines model for CampaignUser. @@ -1016,6 +1033,15 @@ type CampaignVariant struct { Label *string `json:"label,omitempty"` } +// CampaignVariants A campaign's declared variants together with the rule that picks between them when a send does not pick one itself. +type CampaignVariants struct { + // Options The variants this campaign declares. The default variant is always available and is not listed here. + Options *[]CampaignVariant `json:"options,omitempty"` + + // Selector Decides which template variant a send uses. The same shape appears on a campaign, on a journey campaign step and on a broadcast; the most specific layer wins. A static selector pins one variant for every recipient and is rejected when the campaign does not declare its key. An expression selector resolves a variant per recipient and can only be judged at send time, so one that resolves to an unknown variant falls back to the default variant. + Selector *VariantSelector `json:"selector,omitempty"` +} + // ChangePasswordRequest defines model for ChangePasswordRequest. type ChangePasswordRequest struct { // CurrentPassword The password currently on the account @@ -1091,8 +1117,8 @@ type CreateBroadcast struct { // ScheduledAt Optional scheduled send time. If provided, the broadcast is created in 'scheduled' state. ScheduledAt *time.Time `json:"scheduled_at,omitempty"` - // Variant Pins every message in this broadcast to one template variant. Leave unset to let the campaign's variant selector resolve a variant per recipient, which is what a mixed-tenant list needs. - Variant *string `json:"variant,omitempty"` + // Variant Decides which template variant a send uses. The same shape appears on a campaign, on a journey campaign step and on a broadcast; the most specific layer wins. A static selector pins one variant for every recipient and is rejected when the campaign does not declare its key. An expression selector resolves a variant per recipient and can only be judged at send time, so one that resolves to an unknown variant falls back to the default variant. + Variant *VariantSelector `json:"variant,omitempty"` } // CreateCampaign defines model for CreateCampaign. @@ -2217,9 +2243,8 @@ type UpdateCampaign struct { Transactional *bool `json:"transactional,omitempty"` Variables *[]CampaignVariable `json:"variables,omitempty"` - // VariantSelector Liquid expression resolved per recipient when a send does not name a variant itself, for example "{{ user.data.tenant }}". An expression that resolves to an unknown variant falls back to the default one. - VariantSelector *string `json:"variant_selector,omitempty"` - Variants *[]CampaignVariant `json:"variants,omitempty"` + // Variants A campaign's declared variants together with the rule that picks between them when a send does not pick one itself. + Variants *CampaignVariants `json:"variants,omitempty"` } // UpdateJourney defines model for UpdateJourney. @@ -2568,6 +2593,21 @@ type UserSubscriptionList struct { Total int `json:"total"` } +// VariantSelector Decides which template variant a send uses. The same shape appears on a campaign, on a journey campaign step and on a broadcast; the most specific layer wins. A static selector pins one variant for every recipient and is rejected when the campaign does not declare its key. An expression selector resolves a variant per recipient and can only be judged at send time, so one that resolves to an unknown variant falls back to the default variant. +type VariantSelector struct { + // Expression Liquid expression. Required when type is expression. + Expression *string `json:"expression,omitempty"` + + // Key Variant key. Required when type is static. + Key *string `json:"key,omitempty"` + + // Type Whether the variant is pinned or resolved per recipient + Type VariantSelectorType `json:"type"` +} + +// VariantSelectorType Whether the variant is pinned or resolved per recipient +type VariantSelectorType string + // IncludeDeleted defines model for IncludeDeleted. type IncludeDeleted = bool diff --git a/internal/http/controllers/v1/management/template_variants_enterprise_test.go b/internal/http/controllers/v1/management/template_variants_enterprise_test.go index 5d5a65dac..e85d8c2a8 100644 --- a/internal/http/controllers/v1/management/template_variants_enterprise_test.go +++ b/internal/http/controllers/v1/management/template_variants_enterprise_test.go @@ -44,7 +44,9 @@ func TestCreateTemplateRejectsUndeclaredVariant(t *testing.T) { err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ Variants: &store.JSONB[management.CampaignVariants]{ - Data: management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, + Data: management.CampaignVariants{ + Options: []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, + }, }, }) require.NoError(t, err) @@ -104,23 +106,29 @@ func TestUpdateCampaignVariants(t *testing.T) { campaign, err := campaigns.GetCampaign(ctx, projectID, campaignID) require.NoError(t, err) - require.Empty(t, campaign.Variants.Data) - require.Nil(t, campaign.VariantSelector) + require.Empty(t, campaign.Variants.Data.Options) + require.Nil(t, campaign.Variants.Data.Selector) + selector := management.VariantSelector{ + Type: management.VariantSelectorExpression, + Expression: "{{ user.data.tenant }}", + } err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ Variants: &store.JSONB[management.CampaignVariants]{ - Data: management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, + Data: management.CampaignVariants{ + Selector: &selector, + Options: []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, + }, }, - VariantSelector: ptr.To("{{ user.data.tenant }}"), }) require.NoError(t, err) campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) require.NoError(t, err) - require.Equal(t, management.CampaignVariants{{Key: "acme", Label: "Acme Corp"}}, campaign.Variants.Data) - require.Equal(t, "{{ user.data.tenant }}", *campaign.VariantSelector) + require.Equal(t, []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, campaign.Variants.Data.Options) + require.Equal(t, selector, *campaign.Variants.Data.Selector) - // An update that touches neither field leaves both alone. + // An update that does not touch variants leaves the whole object alone. err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ Name: ptr.To("Renamed"), }) @@ -128,17 +136,81 @@ func TestUpdateCampaignVariants(t *testing.T) { campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) require.NoError(t, err) - require.Len(t, campaign.Variants.Data, 1) - require.NotNil(t, campaign.VariantSelector) + require.Len(t, campaign.Variants.Data.Options, 1) + require.NotNil(t, campaign.Variants.Data.Selector) + // Sending the object without a selector clears it. err = campaigns.UpdateCampaign(ctx, projectID, campaignID, management.CampaignUpdate{ - VariantSelector: ptr.To(""), + Variants: &store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{ + Options: []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, + }, + }, }) require.NoError(t, err) campaign, err = campaigns.GetCampaign(ctx, projectID, campaignID) require.NoError(t, err) - require.Nil(t, campaign.VariantSelector) + require.Nil(t, campaign.Variants.Data.Selector) + require.Len(t, campaign.Variants.Data.Options, 1) +} + +// A request body is rejected before it reaches the database when it declares a +// duplicate or empty key, or a selector pointing at a variant that is not in +// the same body. +func TestCampaignVariantsFromOAPI(t *testing.T) { + t.Parallel() + + options := []oapi.CampaignVariant{{Key: "acme"}} + + tests := map[string]struct { + body oapi.CampaignVariants + wantErr bool + }{ + "options only": { + body: oapi.CampaignVariants{Options: &options}, + }, + "selector naming a declared variant": { + body: oapi.CampaignVariants{ + Options: &options, + Selector: &oapi.VariantSelector{ + Type: "static", + Key: ptr.To("acme"), + }, + }, + }, + "selector naming an undeclared variant": { + body: oapi.CampaignVariants{ + Options: &options, + Selector: &oapi.VariantSelector{ + Type: "static", + Key: ptr.To("globex"), + }, + }, + wantErr: true, + }, + "empty key": { + body: oapi.CampaignVariants{Options: &[]oapi.CampaignVariant{{Key: " "}}}, + wantErr: true, + }, + "duplicate key": { + body: oapi.CampaignVariants{Options: &[]oapi.CampaignVariant{{Key: "acme"}, {Key: "acme"}}}, + wantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := management.CampaignVariantsFromOAPI(test.body) + if test.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } } // Duplicating a campaign has to carry variant templates across, or the copy @@ -176,3 +248,85 @@ func TestDuplicateTemplateCarriesVariant(t *testing.T) { require.Len(t, copied, 1) require.Equal(t, "acme", copied[0].Variant) } + +// A broadcast may pin one variant or carry its own expression. A static key is +// checked against the campaign's declared variants when the broadcast is +// created: falling back at send time would quietly put the whole list under +// house branding while the operator believes they picked a client. +func TestCreateBroadcastVariantSelector(t *testing.T) { + t.Parallel() + env := newBroadcastTestEnv(t) + + err := env.mgmtState.CampaignsStore.UpdateCampaign(t.Context(), env.projectID, env.campaignID, management.CampaignUpdate{ + Variants: &store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{ + Options: []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, + }, + }, + }) + require.NoError(t, err) + + tests := map[string]struct { + selector *oapi.VariantSelector + code int + }{ + "no selector defers to the campaign": { + code: 201, + }, + "static key the campaign declares": { + selector: &oapi.VariantSelector{Type: "static", Key: ptr.To("acme")}, + code: 201, + }, + "static key the campaign does not declare": { + selector: &oapi.VariantSelector{Type: "static", Key: ptr.To("globex")}, + code: 400, + }, + "static without a key": { + selector: &oapi.VariantSelector{Type: "static"}, + code: 400, + }, + "expression is accepted unvalidated": { + selector: &oapi.VariantSelector{Type: "expression", Expression: ptr.To("{{ user.data.tenant }}")}, + code: 201, + }, + "expression without an expression": { + selector: &oapi.VariantSelector{Type: "expression"}, + code: 400, + }, + "unknown selector type": { + selector: &oapi.VariantSelector{Type: "whatever", Key: ptr.To("acme")}, + code: 400, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + body, err := json.Marshal(oapi.CreateBroadcastJSONRequestBody{ + CampaignId: env.campaignID, + ListId: env.listID, + Variant: test.selector, + }) + require.NoError(t, err) + + res := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/v1/broadcasts", bytes.NewReader(body)) + req = env.actorCtx(req) + env.controller.CreateBroadcast(res, req, env.projectID) + + require.Equal(t, test.code, res.Code, res.Body.String()) + if test.code != 201 { + return + } + + var broadcast oapi.Broadcast + require.NoError(t, json.Unmarshal(res.Body.Bytes(), &broadcast)) + + if test.selector == nil { + require.Nil(t, broadcast.Variant) + return + } + require.NotNil(t, broadcast.Variant) + require.Equal(t, test.selector.Type, broadcast.Variant.Type) + }) + } +} diff --git a/internal/journeys/campaign.go b/internal/journeys/campaign.go index 390e1fc66..7ec30d435 100644 --- a/internal/journeys/campaign.go +++ b/internal/journeys/campaign.go @@ -7,6 +7,7 @@ import ( "github.com/lunogram/platform/internal/pubsub/schemas" "github.com/lunogram/platform/internal/render" "github.com/lunogram/platform/internal/store/journey" + "github.com/lunogram/platform/internal/store/management" ) func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state journey.JourneyUserState) (journey.JourneyUserState, journey.JourneyVersionStepChildren, error) { @@ -30,17 +31,24 @@ func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state j } } - // A step that names a variant decides the branding for this send outright, - // so it is resolved here against the journey context and travels with the - // event. Left unset, the campaign's own selector resolves one per recipient - // at render time. - var variant *string - if config.Variant != nil && *config.Variant != "" { - resolved, err := render.RenderString(*config.Variant, ctx.Data) + // A step that names a variant decides the branding for this send. An + // expression is resolved here rather than at render time because it reads + // the journey context - entrance data, earlier step state - which no longer + // exists once the send is rendered; the event therefore always carries a + // static selector. Left unset, the campaign's own selector applies. + var variant *management.VariantSelector + if config.Variant != nil { + selector := management.VariantSelectorFromOAPI(*config.Variant) + resolved, err := selector.Resolve(ctx.Data) if err != nil { - return state, nil, fmt.Errorf("failed to render campaign variant: %w", err) + return state, nil, fmt.Errorf("failed to resolve campaign variant: %w", err) + } + if resolved != "" { + variant = &management.VariantSelector{ + Type: management.VariantSelectorStatic, + Key: resolved, + } } - variant = &resolved } msg := schemas.SendCampaign{ diff --git a/internal/pubsub/consumer/broadcasts_enterprise.go b/internal/pubsub/consumer/broadcasts_enterprise.go index 01ce35bc2..f67870010 100644 --- a/internal/pubsub/consumer/broadcasts_enterprise.go +++ b/internal/pubsub/consumer/broadcasts_enterprise.go @@ -111,10 +111,10 @@ func BroadcastBatchHandler(logger *zap.Logger, mgmt *management.State, usrs *sub UserID: userID, CampaignID: broadcast.CampaignID, BroadcastID: &event.BroadcastID, - // Nil when the broadcast is not pinned to one client, which - // leaves the campaign's selector to resolve a variant per - // recipient - what a list spanning several clients needs. - Variant: broadcast.Variant, + // Passed through unresolved: an expression selector here has + // to run once per recipient, and only the render step has the + // context to do that. Nil defers to the campaign's selector. + Variant: broadcast.Variant.Data, } // Deterministic message ID so that if NATS redelivers this batch diff --git a/internal/pubsub/consumer/campaigns_render.go b/internal/pubsub/consumer/campaigns_render.go index 1eb6d6ba6..cf60ba7f8 100644 --- a/internal/pubsub/consumer/campaigns_render.go +++ b/internal/pubsub/consumer/campaigns_render.go @@ -59,33 +59,34 @@ func userToMap(user *subjects.User) map[string]any { // resolveVariant works out which template variant a send must use. // -// An explicit variant on the event wins: the publisher - a journey step, or a -// broadcast pinned to one client - has already decided. Otherwise the -// campaign's selector is rendered against the same context the templates -// render against, so a project can drive branding off recipient data such as -// "{{ user.data.tenant }}" without configuring anything per send. +// A selector on the event wins - a journey step or a broadcast has decided for +// this send in particular. Otherwise the campaign's own selector runs, against +// the same context the templates render against, so a project can drive +// branding off recipient data such as "{{ user.data.tenant }}" without +// configuring anything per send. Neither present means the default variant. // -// A selector that fails to render resolves to the default variant. The -// expression is customer-authored and a broken one must not take a campaign -// down; the fallback is counted where the template is selected. +// A selector that fails to resolve also yields the default variant. Expressions +// are customer-authored and a broken one must not take a campaign down; the +// fallback is counted where the template is selected. func resolveVariant(logger *zap.Logger, campaign *management.Campaign, event schemas.SendCampaign, data map[string]any) string { - if event.Variant != nil { - return strings.TrimSpace(*event.Variant) + selector := event.Variant + if selector == nil { + selector = campaign.Variants.Data.Selector } - if campaign.VariantSelector == nil || *campaign.VariantSelector == "" { + if selector == nil { return "" } - resolved, err := render.RenderString(*campaign.VariantSelector, data) + resolved, err := selector.Resolve(data) if err != nil { - logger.Warn("failed to render campaign variant selector, using default variant", + logger.Warn("failed to resolve variant selector, using default variant", zap.Error(err), - zap.String("selector", *campaign.VariantSelector)) + zap.String("selector_type", string(selector.Type))) return "" } - return strings.TrimSpace(resolved) + return resolved } // selectTemplate picks the template for a send, narrowing by variant before diff --git a/internal/pubsub/consumer/campaigns_variant_test.go b/internal/pubsub/consumer/campaigns_variant_test.go index 62005972e..39b804224 100644 --- a/internal/pubsub/consumer/campaigns_variant_test.go +++ b/internal/pubsub/consumer/campaigns_variant_test.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "github.com/lunogram/platform/internal/ptr" "github.com/lunogram/platform/internal/pubsub/schemas" + "github.com/lunogram/platform/internal/store" "github.com/lunogram/platform/internal/store/management" "github.com/lunogram/platform/internal/store/subjects" "github.com/stretchr/testify/require" @@ -117,48 +118,66 @@ func TestResolveVariant(t *testing.T) { }, } + expression := func(value string) *management.VariantSelector { + return &management.VariantSelector{ + Type: management.VariantSelectorExpression, + Expression: value, + } + } + static := func(key string) *management.VariantSelector { + return &management.VariantSelector{ + Type: management.VariantSelectorStatic, + Key: key, + } + } + tests := []struct { name string - selector *string - event schemas.SendCampaign + campaign *management.VariantSelector + event *management.VariantSelector want string }{ { - name: "explicit variant on the event wins over the selector", - selector: ptr.To("{{ user.data.tenant }}"), - event: schemas.SendCampaign{Variant: ptr.To("globex")}, + name: "the event selector wins over the campaign selector", + campaign: expression("{{ user.data.tenant }}"), + event: static("globex"), want: "globex", }, { - name: "renders the campaign selector when the event names nothing", - selector: ptr.To("{{ user.data.tenant }}"), + name: "renders the campaign expression when the event carries nothing", + campaign: expression("{{ user.data.tenant }}"), want: "acme", }, { - name: "trims whitespace a Liquid expression leaves behind", - selector: ptr.To(" {{ user.data.tenant }} "), + name: "trims whitespace an expression leaves behind", + campaign: expression(" {{ user.data.tenant }} "), want: "acme", }, { - name: "resolves to the default variant when the selector matches nothing", - selector: ptr.To("{{ user.data.missing }}"), + name: "a static campaign selector pins every recipient", + campaign: static("acme"), + want: "acme", + }, + { + name: "an expression matching nothing resolves to the default variant", + campaign: expression("{{ user.data.missing }}"), want: "", }, { - name: "resolves to the default variant when no selector is configured", + name: "no selector anywhere resolves to the default variant", want: "", }, { name: "a broken expression resolves to the default variant rather than failing the send", - selector: ptr.To("{{ user.data.tenant | }}"), + campaign: expression("{{ user.data.tenant | }}"), want: "", }, { - // A selector with no Liquid in it is a static pin, which is how a - // campaign sends every message under one brand. - name: "a literal selector is used as the variant key", - selector: ptr.To("acme"), - want: "acme", + // Static never renders, so a key that happens to look like Liquid + // is still used verbatim rather than evaluated. + name: "a static key is never rendered", + campaign: static("{{ user.data.tenant }}"), + want: "{{ user.data.tenant }}", }, } @@ -166,8 +185,13 @@ func TestResolveVariant(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - campaign := &management.Campaign{VariantSelector: test.selector} - require.Equal(t, test.want, resolveVariant(logger, campaign, test.event, data)) + campaign := &management.Campaign{ + Variants: store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{Selector: test.campaign}, + }, + } + event := schemas.SendCampaign{Variant: test.event} + require.Equal(t, test.want, resolveVariant(logger, campaign, event, data)) }) } } @@ -175,10 +199,61 @@ func TestResolveVariant(t *testing.T) { func TestCampaignVariantsHas(t *testing.T) { t.Parallel() - variants := management.CampaignVariants{{Key: "acme"}, {Key: "globex"}} + variants := management.CampaignVariants{ + Options: []management.CampaignVariant{{Key: "acme"}, {Key: "globex"}}, + } require.True(t, variants.Has("acme")) require.True(t, variants.Has(""), "the default variant always exists") require.False(t, variants.Has("initech")) require.True(t, management.CampaignVariants{}.Has("")) } + +func TestVariantSelectorValidate(t *testing.T) { + t.Parallel() + + variants := management.CampaignVariants{ + Options: []management.CampaignVariant{{Key: "acme"}}, + } + + tests := map[string]struct { + selector management.VariantSelector + wantErr bool + }{ + "declared static key": { + selector: management.VariantSelector{Type: management.VariantSelectorStatic, Key: "acme"}, + }, + "undeclared static key": { + selector: management.VariantSelector{Type: management.VariantSelectorStatic, Key: "globex"}, + wantErr: true, + }, + "static without a key": { + selector: management.VariantSelector{Type: management.VariantSelectorStatic}, + wantErr: true, + }, + "expression": { + selector: management.VariantSelector{Type: management.VariantSelectorExpression, Expression: "{{ user.data.tenant }}"}, + }, + "expression without an expression": { + selector: management.VariantSelector{Type: management.VariantSelectorExpression}, + wantErr: true, + }, + "unknown type": { + selector: management.VariantSelector{Type: "whatever"}, + wantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := test.selector.Validate(variants) + if test.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} diff --git a/internal/pubsub/schemas/events.go b/internal/pubsub/schemas/events.go index 4593db496..48203ce6f 100644 --- a/internal/pubsub/schemas/events.go +++ b/internal/pubsub/schemas/events.go @@ -7,6 +7,7 @@ import ( "time" "github.com/google/uuid" + "github.com/lunogram/platform/internal/store/management" "github.com/lunogram/platform/internal/store/subjects" ) @@ -136,10 +137,15 @@ type SendCampaign struct { BroadcastID *uuid.UUID `json:"broadcast_id,omitempty"` Data *SendCampaignData `json:"data,omitempty"` Variables map[string]string `json:"variables,omitempty"` - // Variant names the template variant this send must use, already resolved - // by the publisher. Nil hands the choice to the campaign's variant - // selector, which resolves one per recipient at render time. - Variant *string `json:"variant,omitempty"` + // Variant overrides the campaign's own rule for picking a template variant. + // Nil defers to the campaign. + // + // A journey step resolves its expression before publishing and sends a + // static selector, because the journey context it renders against - entrance + // data, earlier step state - does not exist by the time the send is + // rendered. A broadcast passes its selector through untouched, since an + // expression there has to run once per recipient. + Variant *management.VariantSelector `json:"variant,omitempty"` } // InboxOrigin resolves the inbox source label and the external_id key used diff --git a/internal/store/management/broadcasts.go b/internal/store/management/broadcasts.go index 603d14ea9..0863f4e5d 100644 --- a/internal/store/management/broadcasts.go +++ b/internal/store/management/broadcasts.go @@ -31,11 +31,11 @@ type Broadcast struct { ListID uuid.UUID `db:"list_id"` ListName string `db:"list_name"` ListType string `db:"list_type"` - // Variant pins every message in this broadcast to one template variant. Nil - // leaves the choice to the campaign's variant selector, which resolves a - // variant per recipient - what a list spanning several clients needs. - Variant *string `db:"variant"` - State BroadcastState `db:"state"` + // Variant overrides the campaign's rule for this broadcast, either pinning + // one variant or carrying its own expression for a list spanning several + // clients. Nil defers to the campaign. + Variant store.JSONB[*VariantSelector] `db:"variant"` + State BroadcastState `db:"state"` // Total is the number of messages published to NATS during broadcast // processing. Sent tracks the number of messages actually delivered, and // Failed the number that terminally will not be - a suppressed recipient or @@ -65,7 +65,6 @@ func (b Broadcast) OAPI() oapi.Broadcast { ListId: b.ListID, ListName: b.ListName, ListType: b.ListType, - Variant: b.Variant, State: oapi.BroadcastState(b.State), Total: b.Total, Sent: &b.Sent, @@ -78,6 +77,11 @@ func (b Broadcast) OAPI() oapi.Broadcast { UpdatedAt: b.UpdatedAt, } + if b.Variant.Data != nil { + selector := b.Variant.Data.OAPI() + result.Variant = &selector + } + if b.Campaign != nil { channel := b.Campaign.Channel result.Campaign = &struct { @@ -126,8 +130,19 @@ func (s *BroadcastsStore) CreateBroadcast(ctx context.Context, broadcast Broadca VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id, project_id, campaign_id, list_id, list_name, list_type, variant, state, total, sent, failed, error, scheduled_at, created_at, updated_at, started_at, completed_at` + // Absent selectors are stored as SQL NULL rather than a JSON null, so a + // broadcast that defers to the campaign reads back as one. + var variantVal any + if broadcast.Variant.Data != nil { + v, err := broadcast.Variant.Value() + if err != nil { + return Broadcast{}, err + } + variantVal = v + } + var result Broadcast - err := s.db.GetContext(ctx, &result, stmt, broadcast.ProjectID, broadcast.CampaignID, broadcast.ListID, broadcast.ListName, broadcast.ListType, string(state), broadcast.ScheduledAt, broadcast.Variant) + err := s.db.GetContext(ctx, &result, stmt, broadcast.ProjectID, broadcast.CampaignID, broadcast.ListID, broadcast.ListName, broadcast.ListType, string(state), broadcast.ScheduledAt, variantVal) if err != nil { return Broadcast{}, err } diff --git a/internal/store/management/campaigns.go b/internal/store/management/campaigns.go index 462ae3c9b..c49677c55 100644 --- a/internal/store/management/campaigns.go +++ b/internal/store/management/campaigns.go @@ -26,45 +26,20 @@ type CampaignVariable struct { type CampaignVariables []CampaignVariable -// CampaignVariant declares one white-labelled edition of a campaign. Key is -// what a send resolves against to pick a set of templates; the empty key is the -// default variant every campaign starts with and is never declared here. -type CampaignVariant struct { - Key string `json:"key"` - Label string `json:"label,omitempty"` -} - -type CampaignVariants []CampaignVariant - -// Has reports whether key names a declared variant. The default variant is -// always available and is not part of the declared set. -func (variants CampaignVariants) Has(key string) bool { - if key == "" { - return true - } - for _, variant := range variants { - if variant.Key == key { - return true - } - } - return false -} - type Campaign struct { - ID uuid.UUID `db:"id"` - ProjectID uuid.UUID `db:"project_id"` - Name string `db:"name"` - Channel string `db:"channel"` - SubscriptionID *uuid.UUID `db:"subscription_id"` - Transactional bool `db:"transactional"` - Delivery store.JSONB[Delivery] `db:"delivery"` - Variables store.JSONB[CampaignVariables] `db:"variables"` - Variants store.JSONB[CampaignVariants] `db:"variants"` - VariantSelector *string `db:"variant_selector"` - Templates Templates `db:"-"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` - DeletedAt *time.Time `db:"deleted_at"` + ID uuid.UUID `db:"id"` + ProjectID uuid.UUID `db:"project_id"` + Name string `db:"name"` + Channel string `db:"channel"` + SubscriptionID *uuid.UUID `db:"subscription_id"` + Transactional bool `db:"transactional"` + Delivery store.JSONB[Delivery] `db:"delivery"` + Variables store.JSONB[CampaignVariables] `db:"variables"` + Variants store.JSONB[CampaignVariants] `db:"variants"` + Templates Templates `db:"-"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` + DeletedAt *time.Time `db:"deleted_at"` } func (campaign Campaign) OAPI() oapi.Campaign { @@ -76,30 +51,23 @@ func (campaign Campaign) OAPI() oapi.Campaign { } } - variants := make([]oapi.CampaignVariant, len(campaign.Variants.Data)) - for i, v := range campaign.Variants.Data { - variants[i] = oapi.CampaignVariant{Key: v.Key} - if v.Label != "" { - variants[i].Label = &v.Label - } - } + variants := campaign.Variants.Data.OAPI() archived := campaign.DeletedAt != nil result := oapi.Campaign{ - Id: campaign.ID, - ProjectId: campaign.ProjectID, - Name: campaign.Name, - Channel: oapi.Channel(campaign.Channel), - SubscriptionId: campaign.SubscriptionID, - Transactional: campaign.Transactional, - Delivery: campaign.Delivery.Data.OAPI(), - Variables: &variables, - Variants: &variants, - VariantSelector: campaign.VariantSelector, - Templates: campaign.Templates.OAPI(), - CreatedAt: campaign.CreatedAt, - UpdatedAt: campaign.UpdatedAt, - Archived: &archived, + Id: campaign.ID, + ProjectId: campaign.ProjectID, + Name: campaign.Name, + Channel: oapi.Channel(campaign.Channel), + SubscriptionId: campaign.SubscriptionID, + Transactional: campaign.Transactional, + Delivery: campaign.Delivery.Data.OAPI(), + Variables: &variables, + Variants: &variants, + Templates: campaign.Templates.OAPI(), + CreatedAt: campaign.CreatedAt, + UpdatedAt: campaign.UpdatedAt, + Archived: &archived, } return result @@ -150,7 +118,7 @@ func (s *CampaignsStore) CreateCampaign(ctx context.Context, campaign Campaign) func (s *CampaignsStore) ListCampaigns(ctx context.Context, project uuid.UUID, pagination store.Pagination, search string, archivedOnly bool) (Campaigns, int, error) { query := ` - SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, variant_selector, created_at, updated_at, deleted_at, + SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, created_at, updated_at, deleted_at, COUNT(*) OVER () AS total_count FROM campaigns WHERE project_id = $1 @@ -183,7 +151,7 @@ func (s *CampaignsStore) ListCampaigns(ctx context.Context, project uuid.UUID, p func (s *CampaignsStore) GetCampaign(ctx context.Context, projectID, campaignID uuid.UUID) (*Campaign, error) { query := ` - SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, variant_selector, created_at, updated_at, deleted_at + SELECT id, project_id, COALESCE(name, '') AS name, channel, subscription_id, transactional, delivery, variables, variants, created_at, updated_at, deleted_at FROM campaigns WHERE project_id = $1 AND id = $2 @@ -209,8 +177,6 @@ type CampaignUpdate struct { Transactional *bool Variables *store.JSONB[CampaignVariables] Variants *store.JSONB[CampaignVariants] - // VariantSelector is a Liquid expression; an empty string clears it. - VariantSelector *string } func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaignID uuid.UUID, update CampaignUpdate) error { @@ -224,14 +190,9 @@ func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaign WHEN COALESCE($3, transactional) THEN NULL ELSE COALESCE($4, subscription_id) END, - variants = COALESCE($5, variants), - variant_selector = CASE - WHEN $6::text IS NULL THEN variant_selector - WHEN $6::text = '' THEN NULL - ELSE $6::text - END - WHERE project_id = $7 - AND id = $8 + variants = COALESCE($5, variants) + WHERE project_id = $6 + AND id = $7 AND deleted_at IS NULL` var variablesVal any @@ -252,7 +213,7 @@ func (s *CampaignsStore) UpdateCampaign(ctx context.Context, projectID, campaign variantsVal = v } - _, err := s.db.ExecContext(ctx, query, update.Name, variablesVal, update.Transactional, update.SubscriptionID, variantsVal, update.VariantSelector, projectID, campaignID) + _, err := s.db.ExecContext(ctx, query, update.Name, variablesVal, update.Transactional, update.SubscriptionID, variantsVal, projectID, campaignID) return err } diff --git a/internal/store/management/migrations/1787000000007_template_variants.down.sql b/internal/store/management/migrations/1787000000007_template_variants.down.sql index cbc558572..b2a22a22e 100644 --- a/internal/store/management/migrations/1787000000007_template_variants.down.sql +++ b/internal/store/management/migrations/1787000000007_template_variants.down.sql @@ -1,5 +1,4 @@ ALTER TABLE campaign_broadcasts DROP COLUMN IF EXISTS variant; -ALTER TABLE campaigns DROP COLUMN IF EXISTS variant_selector; ALTER TABLE campaigns DROP COLUMN IF EXISTS variants; DROP INDEX IF EXISTS templates_campaign_variant_locale_idx; ALTER TABLE templates DROP COLUMN IF EXISTS variant; diff --git a/internal/store/management/migrations/1787000000007_template_variants.up.sql b/internal/store/management/migrations/1787000000007_template_variants.up.sql index 10baaf9fd..30ba7e0d4 100644 --- a/internal/store/management/migrations/1787000000007_template_variants.up.sql +++ b/internal/store/management/migrations/1787000000007_template_variants.up.sql @@ -13,15 +13,17 @@ ALTER TABLE templates ADD COLUMN variant VARCHAR(255) NOT NULL DEFAULT ''; CREATE INDEX templates_campaign_variant_locale_idx ON templates(campaign_id, variant, locale); -- Variants are declared per campaign and edited on the campaign page, mirroring --- how campaign variables are stored and edited. Each entry is {key, label}; the --- label is what the console shows, the key is what a send resolves against. -ALTER TABLE campaigns ADD COLUMN variants JSONB NOT NULL DEFAULT '[]'::jsonb; +-- how campaign variables are stored and edited. The declared set and the rule +-- that picks between them are one concept, so they live in one object: +-- +-- {"selector": {"type": "expression", "expression": "{{ user.data.tenant }}"}, +-- "options": [{"key": "acme", "label": "Acme Corp"}]} +-- +-- selector is optional; without one, a send that names no variant of its own +-- gets the default variant. +ALTER TABLE campaigns ADD COLUMN variants JSONB NOT NULL DEFAULT '{}'::jsonb; --- A Liquid expression evaluated per recipient when a send does not name a --- variant outright -- e.g. '{{ user.data.tenant }}'. This is what lets a single --- broadcast over a mixed-tenant list resolve a different brand per recipient. -ALTER TABLE campaigns ADD COLUMN variant_selector TEXT; - --- A broadcast that targets one client pins its variant here; left NULL, the --- campaign selector decides per recipient. -ALTER TABLE campaign_broadcasts ADD COLUMN variant VARCHAR(255); +-- A broadcast may override the campaign's rule, either pinning one variant +-- ({"type": "static", "key": "acme"}) or carrying its own expression for a list +-- that spans several clients. NULL defers to the campaign. +ALTER TABLE campaign_broadcasts ADD COLUMN variant JSONB; diff --git a/internal/store/management/variants.go b/internal/store/management/variants.go new file mode 100644 index 000000000..ec3f89eb4 --- /dev/null +++ b/internal/store/management/variants.go @@ -0,0 +1,186 @@ +package management + +import ( + "fmt" + "strings" + + "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" + "github.com/lunogram/platform/internal/render" +) + +// VariantSelectorType distinguishes the two ways a send decides which template +// variant it wants. The mode is explicit rather than inferred from whether the +// value happens to contain Liquid syntax: a static key can then be checked +// against the campaign's declared variants when it is saved, and the console +// knows which editor to offer without inspecting the string. +type VariantSelectorType string + +const ( + // VariantSelectorStatic pins one variant for every recipient. + VariantSelectorStatic VariantSelectorType = "static" + // VariantSelectorExpression resolves a variant per recipient from a Liquid + // expression such as "{{ user.data.tenant }}". + VariantSelectorExpression VariantSelectorType = "expression" +) + +// VariantSelector decides which template variant a send uses. The same type +// appears on a campaign, on a journey campaign step and on a broadcast, so the +// three layers offer identical choices; the more specific layer wins. +type VariantSelector struct { + Type VariantSelectorType `json:"type"` + Key string `json:"key,omitempty"` + Expression string `json:"expression,omitempty"` +} + +// Resolve produces the variant key this selector points at, rendering the +// expression against the supplied context when there is one. An empty result +// means the default variant. +func (selector VariantSelector) Resolve(data map[string]any) (string, error) { + switch selector.Type { + case VariantSelectorStatic: + return strings.TrimSpace(selector.Key), nil + case VariantSelectorExpression: + resolved, err := render.RenderString(selector.Expression, data) + if err != nil { + return "", err + } + return strings.TrimSpace(resolved), nil + default: + return "", fmt.Errorf("unknown variant selector type %q", selector.Type) + } +} + +// Validate reports whether the selector is usable against a campaign's declared +// variants. A static key is checked here so a stale or mistyped one is refused +// when it is saved rather than silently falling back to house branding on every +// send; an expression can only be judged at send time. +func (selector VariantSelector) Validate(variants CampaignVariants) error { + switch selector.Type { + case VariantSelectorStatic: + key := strings.TrimSpace(selector.Key) + if key == "" { + return fmt.Errorf("static variant selector requires a key") + } + if !variants.Has(key) { + return fmt.Errorf("campaign does not declare variant %s", key) + } + return nil + case VariantSelectorExpression: + if strings.TrimSpace(selector.Expression) == "" { + return fmt.Errorf("expression variant selector requires an expression") + } + return nil + default: + return fmt.Errorf("unknown variant selector type %q", selector.Type) + } +} + +// CampaignVariant declares one white-labelled edition of a campaign. Key is +// what a send resolves against to pick a set of templates; the empty key is the +// default variant every campaign starts with and is never declared here. +type CampaignVariant struct { + Key string `json:"key"` + Label string `json:"label,omitempty"` +} + +// CampaignVariants holds a campaign's declared variants together with the rule +// that picks between them when a send does not pick one itself. They are one +// concept - a selector is meaningless without the set it resolves into - so +// they are stored and edited as one object. +type CampaignVariants struct { + Selector *VariantSelector `json:"selector,omitempty"` + Options []CampaignVariant `json:"options,omitempty"` +} + +// Has reports whether key names a declared variant. The default variant is +// always available and is not part of the declared set. +func (variants CampaignVariants) Has(key string) bool { + if key == "" { + return true + } + for _, option := range variants.Options { + if option.Key == key { + return true + } + } + return false +} + +func (selector VariantSelector) OAPI() oapi.VariantSelector { + result := oapi.VariantSelector{Type: oapi.VariantSelectorType(selector.Type)} + if selector.Key != "" { + result.Key = &selector.Key + } + if selector.Expression != "" { + result.Expression = &selector.Expression + } + return result +} + +func VariantSelectorFromOAPI(selector oapi.VariantSelector) VariantSelector { + result := VariantSelector{Type: VariantSelectorType(selector.Type)} + if selector.Key != nil { + result.Key = strings.TrimSpace(*selector.Key) + } + if selector.Expression != nil { + result.Expression = strings.TrimSpace(*selector.Expression) + } + return result +} + +func (variants CampaignVariants) OAPI() oapi.CampaignVariants { + options := make([]oapi.CampaignVariant, len(variants.Options)) + for i, option := range variants.Options { + options[i] = oapi.CampaignVariant{Key: option.Key} + if option.Label != "" { + options[i].Label = &variants.Options[i].Label + } + } + + result := oapi.CampaignVariants{Options: &options} + if variants.Selector != nil { + selector := variants.Selector.OAPI() + result.Selector = &selector + } + return result +} + +// CampaignVariantsFromOAPI converts a request body into the stored shape, +// rejecting duplicate and empty keys. The empty key is the default variant: it +// always exists and is never declared, so accepting it here would create a +// duplicate of something that cannot be removed. +func CampaignVariantsFromOAPI(variants oapi.CampaignVariants) (CampaignVariants, error) { + var result CampaignVariants + + if variants.Options != nil { + result.Options = make([]CampaignVariant, 0, len(*variants.Options)) + seen := make(map[string]bool, len(*variants.Options)) + + for _, option := range *variants.Options { + key := strings.TrimSpace(option.Key) + if key == "" { + return result, fmt.Errorf("variant key cannot be empty") + } + if seen[key] { + return result, fmt.Errorf("duplicate variant key %s", key) + } + seen[key] = true + + converted := CampaignVariant{Key: key} + if option.Label != nil { + converted.Label = strings.TrimSpace(*option.Label) + } + result.Options = append(result.Options, converted) + } + } + + if variants.Selector != nil { + selector := VariantSelectorFromOAPI(*variants.Selector) + if err := selector.Validate(result); err != nil { + return result, err + } + result.Selector = &selector + } + + return result, nil +} From 7770c95a50e926c12300b298b62bbd4e5f030485 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Mon, 31 Aug 2026 15:05:23 +0200 Subject: [PATCH 4/5] feat(templates): let a send pin the default variant A step or broadcast could inherit the campaign's rule or name a declared variant, but it could not ask for the default variant outright: Validate rejected a static selector with an empty key, and the console's dropdown only listed declared options. Carrying no selector is not the same thing - that defers to the campaign, so a campaign resolving a client brand per recipient had no way to send one message under house branding. A security or account notice inside an otherwise white-labelled journey is exactly that case. A static selector with an empty key now pins the default variant, which CampaignVariants.Has already reported as valid, so Validate needed no special case beyond dropping the empty-key rejection. resolveVariant already treated a present selector as beating the campaign's, so pinning the default correctly stops the fall-through rather than reading as "nothing set". The consequence of allowing an empty key is that a malformed {type: static} with no key at all now resolves to the default variant instead of being refused. That is the safe direction to be lax in - house branding, never another client's - and it matches the fallback behaviour everywhere else in this feature. Static mode in the console now starts on the default variant rather than on whichever client sorts first, so an unfinished edit cannot pin someone else's branding. --- console/src/oapi/management.generated.ts | 2 +- .../views/campaign/VariantSelectorInput.tsx | 20 +++++++++++++++---- .../v1/management/oapi/resources.yml | 6 +++++- .../v1/management/oapi/resources_gen.go | 2 +- .../template_variants_enterprise_test.go | 8 ++++++-- .../pubsub/consumer/campaigns_variant_test.go | 13 ++++++++++-- internal/store/management/variants.go | 15 ++++++++------ 7 files changed, 49 insertions(+), 17 deletions(-) diff --git a/console/src/oapi/management.generated.ts b/console/src/oapi/management.generated.ts index 431a6e1db..98176f598 100644 --- a/console/src/oapi/management.generated.ts +++ b/console/src/oapi/management.generated.ts @@ -3366,7 +3366,7 @@ export interface components { */ type: "static" | "expression"; /** - * @description Variant key. Required when type is static. + * @description Variant key, used when type is static. An empty key pins the default variant, which is how one send is forced back to house branding past a campaign that resolves a client brand per recipient. That differs from omitting the selector entirely, which defers to the campaign. * @example acme */ key?: string; diff --git a/console/src/views/campaign/VariantSelectorInput.tsx b/console/src/views/campaign/VariantSelectorInput.tsx index fa410103e..c4513290d 100644 --- a/console/src/views/campaign/VariantSelectorInput.tsx +++ b/console/src/views/campaign/VariantSelectorInput.tsx @@ -14,6 +14,10 @@ import type { CampaignVariant, VariantSelector, VariantSelectorType } from "@/ty const NONE = "__none__" +// Radix reads an empty string as "nothing selected" and would show the +// placeholder, so the default variant needs a stand-in value in the dropdown. +const DEFAULT_KEY = "__default__" + interface VariantSelectorInputProps { value?: VariantSelector options: CampaignVariant[] @@ -43,13 +47,16 @@ export function VariantSelectorInput({ const handleModeChange = useCallback( (next: string) => { if (next === NONE) return onChange(undefined) + // Static mode starts on the default variant rather than on + // whichever client happens to sort first, so an unfinished edit + // cannot pin someone else's branding. onChange( next === "static" - ? { type: "static", key: options[0]?.key ?? "" } + ? { type: "static", key: "" } : { type: "expression", expression: "" }, ) }, - [onChange, options], + [onChange], ) return ( @@ -73,8 +80,10 @@ export function VariantSelectorInput({ {value?.type === "static" && ( { + // Variant keys are declared per + // campaign, so one left over from the + // previous pick either fails the save + // or, worse, matches a key another + // client happens to use. + form.setValue("variant", undefined) + field.onChange(value) + }} disabled={!!preselectedCampaignId} > diff --git a/console/src/views/campaign/CampaignVariants.tsx b/console/src/views/campaign/CampaignVariants.tsx index 2155bc692..3cf6d2c1b 100644 --- a/console/src/views/campaign/CampaignVariants.tsx +++ b/console/src/views/campaign/CampaignVariants.tsx @@ -34,10 +34,12 @@ export function CampaignVariants({ value, templates, onChange }: CampaignVariant (options: CampaignVariant[]) => { // Dropping the variant a static selector points at would leave the // campaign pinned to something it no longer declares, which the API - // rejects on save. Clear the selector instead. + // rejects on save. Clear the selector instead. The empty key is the + // default variant: always available, never in options, so a + // campaign pinned to house branding survives every edit here. + const pinned = value.selector?.type === "static" ? value.selector.key : undefined const selector = - value.selector?.type === "static" && - !options.some((option) => option.key === value.selector?.key) + pinned && !options.some((option) => option.key === pinned) ? undefined : value.selector onChange({ selector, options }) @@ -93,7 +95,14 @@ export function CampaignVariants({ value, templates, onChange }: CampaignVariant
{variants.map((variant, index) => { const keyError = validateKey(variant.key, variants, index) - const templateCount = templates.filter((t) => t.variant === variant.key).length + const templateCount = variant.key + ? templates.filter((t) => t.variant === variant.key).length + : 0 + // Templates are stored under the key, and nothing moves + // them when it changes, so a rename here would strand them: + // hidden from the switcher and unreachable by any send. The + // API refuses the same edit. + const keyLocked = templateCount > 0 return (
@@ -105,8 +114,17 @@ export function CampaignVariants({ value, templates, onChange }: CampaignVariant key: e.target.value.toLowerCase().replace(/\s/g, "-"), }) } + readOnly={keyLocked} + title={ + keyLocked + ? t( + "campaign.variants.key_locked", + "Delete this variant's templates before renaming or removing it.", + ) + : undefined + } placeholder="key" - className="h-8 w-36 rounded-r-none font-mono text-sm shadow-none focus:z-10" + className="h-8 w-36 rounded-r-none font-mono text-sm shadow-none focus:z-10 read-only:cursor-not-allowed read-only:text-muted-foreground" /> removeVariant(index)} - className="-ml-px flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-r-md border border-l-0 bg-background text-muted-foreground/60 transition-colors hover:bg-destructive/5 hover:text-destructive" + disabled={keyLocked} + title={ + keyLocked + ? t( + "campaign.variants.key_locked", + "Delete this variant's templates before renaming or removing it.", + ) + : undefined + } + className="-ml-px flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-r-md border border-l-0 bg-background text-muted-foreground/60 transition-colors hover:bg-destructive/5 hover:text-destructive disabled:cursor-not-allowed disabled:opacity-40 disabled:hover:bg-background disabled:hover:text-muted-foreground/60" aria-label={t("campaign.variants.delete", "Delete variant")} > diff --git a/internal/http/controllers/v1/management/campaigns.go b/internal/http/controllers/v1/management/campaigns.go index d44497fe1..ce018819c 100644 --- a/internal/http/controllers/v1/management/campaigns.go +++ b/internal/http/controllers/v1/management/campaigns.go @@ -3,7 +3,10 @@ package v1 import ( "database/sql" "errors" + "fmt" "net/http" + "sort" + "strings" "github.com/google/uuid" "github.com/jmoiron/sqlx" @@ -262,6 +265,19 @@ func (srv *CampaignsController) UpdateCampaign(w http.ResponseWriter, r *http.Re oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe(err.Error()))) return } + + // Templates are keyed by variant, so undeclaring a key - by removing it + // or by renaming it, which reaches the API as the same edit - would + // strand its templates: invisible in the console and unreachable by any + // send. Refuse while they exist rather than orphaning them. + if orphaned := undeclaredTemplateVariants(campaign.Templates, variants); len(orphaned) > 0 { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe(fmt.Sprintf( + "variant %s still has templates; delete them before removing or renaming the variant", + strings.Join(orphaned, ", "), + )))) + return + } + updated.Variants = &store.JSONB[management.CampaignVariants]{Data: variants} } @@ -406,12 +422,16 @@ func (srv *CampaignsController) DuplicateCampaign(w http.ResponseWriter, r *http campaigns := management.NewCampaignsStore(tx) templates := management.NewTemplatesStore(tx) + // The declared variants travel with the copy: the templates below carry + // their variant key, and a key the campaign no longer declares hides those + // templates from the console and drops their sends back to house branding. newCampaignID, err := campaigns.CreateCampaign(ctx, management.Campaign{ ProjectID: campaign.ProjectID, Name: "Copy of " + campaign.Name, Channel: campaign.Channel, SubscriptionID: campaign.SubscriptionID, Transactional: campaign.Transactional, + Variants: campaign.Variants, }) if err != nil { logger.Error("failed to create duplicated campaign", zap.Error(err)) @@ -490,3 +510,22 @@ func (srv *CampaignsController) GetCampaignUsers(w http.ResponseWriter, r *http. "offset": pagination.Offset, }) } + +// undeclaredTemplateVariants reports the variant keys that templates still +// carry but the supplied declaration no longer names, sorted for a stable +// error message. The default variant is always declared and never appears. +func undeclaredTemplateVariants(templates management.Templates, variants management.CampaignVariants) []string { + var orphaned []string + seen := make(map[string]bool) + + for _, template := range templates { + if template.Variant == "" || seen[template.Variant] || variants.Has(template.Variant) { + continue + } + seen[template.Variant] = true + orphaned = append(orphaned, template.Variant) + } + + sort.Strings(orphaned) + return orphaned +} diff --git a/internal/http/controllers/v1/management/journeys.go b/internal/http/controllers/v1/management/journeys.go index 2b084d8cc..5d3651af8 100644 --- a/internal/http/controllers/v1/management/journeys.go +++ b/internal/http/controllers/v1/management/journeys.go @@ -1,6 +1,7 @@ package v1 import ( + "context" "database/sql" stdjson "encoding/json" "errors" @@ -1051,12 +1052,20 @@ func (srv *JourneysController) PublishJourney(w http.ResponseWriter, r *http.Req return } - if err := validateEntranceSteps(draftSteps.OAPI()); err != nil { + steps := draftSteps.OAPI() + + if err := validateEntranceSteps(steps); err != nil { logger.Info("journey failed entrance validation", zap.Error(err)) oapi.WriteProblem(w, err) return } + if err := srv.validateCampaignStepVariants(ctx, projectID, steps); err != nil { + logger.Info("journey failed campaign variant validation", zap.Error(err)) + oapi.WriteProblem(w, err) + return + } + tx, err := srv.journeyDB.BeginTxx(ctx, nil) if err != nil { logger.Error("failed to begin transaction", zap.Error(err)) @@ -1185,3 +1194,48 @@ func validateEntranceSteps(steps oapi.JourneyStepMap) error { return nil } + +// validateCampaignStepVariants checks every campaign step's variant selector +// against the campaign it sends. Draft saves stay lenient, so this is the first +// point a mistyped static key can be refused; left unchecked it would resolve +// to nothing at send time and quietly deliver house branding to a client's +// recipients. An expression is only checked for being present, since what it +// resolves to is per-recipient. +func (srv *JourneysController) validateCampaignStepVariants(ctx context.Context, projectID uuid.UUID, steps oapi.JourneyStepMap) error { + campaigns := make(map[uuid.UUID]*management.Campaign) + + for id, step := range steps { + if step.Type != oapi.JourneyStepTypeCampaign { + continue + } + + var data oapi.CampaignStepData + if err := json.Unmarshal(step.Data, &data); err != nil { + return problem.ErrBadRequest(problem.Describe(fmt.Sprintf("campaign step %q: %v", id, err))) + } + + if data.Variant == nil { + continue + } + + campaign, ok := campaigns[data.CampaignId] + if !ok { + found, err := srv.mgmt.GetCampaign(ctx, projectID, data.CampaignId) + if errors.Is(err, sql.ErrNoRows) { + return problem.ErrBadRequest(problem.Describe(fmt.Sprintf("campaign step %q: campaign not found", id))) + } + if err != nil { + return err + } + campaign = found + campaigns[data.CampaignId] = found + } + + selector := management.VariantSelectorFromOAPI(*data.Variant) + if err := selector.Validate(campaign.Variants.Data); err != nil { + return problem.ErrBadRequest(problem.Describe(fmt.Sprintf("campaign step %q: %v", id, err))) + } + } + + return nil +} diff --git a/internal/http/controllers/v1/management/oapi/resources.yml b/internal/http/controllers/v1/management/oapi/resources.yml index 3fad00577..18d83e61c 100644 --- a/internal/http/controllers/v1/management/oapi/resources.yml +++ b/internal/http/controllers/v1/management/oapi/resources.yml @@ -7643,7 +7643,12 @@ components: properties: key: type: string - description: The value a send resolves against to pick this variant's templates + pattern: "^[a-z0-9][a-z0-9_-]*$" + description: >- + The value a send resolves against to pick this variant's templates. + Lowercase letters, digits, dashes and underscores, starting with a + letter or digit. The empty key is the default variant and is never + declared here. example: "acme" label: type: string diff --git a/internal/http/controllers/v1/management/oapi/resources_gen.go b/internal/http/controllers/v1/management/oapi/resources_gen.go index d1e8d5abd..13d55a58c 100644 --- a/internal/http/controllers/v1/management/oapi/resources_gen.go +++ b/internal/http/controllers/v1/management/oapi/resources_gen.go @@ -1026,7 +1026,7 @@ type CampaignVariable struct { // CampaignVariant defines model for CampaignVariant. type CampaignVariant struct { - // Key The value a send resolves against to pick this variant's templates + // Key The value a send resolves against to pick this variant's templates. Lowercase letters, digits, dashes and underscores, starting with a letter or digit. The empty key is the default variant and is never declared here. Key string `json:"key"` // Label Human readable name shown in the console diff --git a/internal/http/controllers/v1/management/template_variants_enterprise_test.go b/internal/http/controllers/v1/management/template_variants_enterprise_test.go index 18cbf6224..65058c043 100644 --- a/internal/http/controllers/v1/management/template_variants_enterprise_test.go +++ b/internal/http/controllers/v1/management/template_variants_enterprise_test.go @@ -12,6 +12,7 @@ import ( "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" "github.com/lunogram/platform/internal/ptr" "github.com/lunogram/platform/internal/pubsub" + "github.com/lunogram/platform/internal/pubsub/consumer" "github.com/lunogram/platform/internal/rbac" "github.com/lunogram/platform/internal/store" "github.com/lunogram/platform/internal/store/management" @@ -197,6 +198,17 @@ func TestCampaignVariantsFromOAPI(t *testing.T) { body: oapi.CampaignVariants{Options: &[]oapi.CampaignVariant{{Key: "acme"}, {Key: "acme"}}}, wantErr: true, }, + // The console reserves values starting with an underscore as its + // "default" and "none" dropdown sentinels. A saved key that collides + // with one is unselectable once it is stored. + "key using a console sentinel": { + body: oapi.CampaignVariants{Options: &[]oapi.CampaignVariant{{Key: "__default__"}}}, + wantErr: true, + }, + "key with uppercase and spaces": { + body: oapi.CampaignVariants{Options: &[]oapi.CampaignVariant{{Key: "Acme Corp"}}}, + wantErr: true, + }, } for name, test := range tests { @@ -334,3 +346,139 @@ func TestCreateBroadcastVariantSelector(t *testing.T) { }) } } + +// Undeclaring a variant that still has templates strands those rows: hidden +// from the console switcher and unreachable by any send. A rename reaches the +// API as the same edit, so one guard covers both. +func TestUndeclaredTemplateVariants(t *testing.T) { + t.Parallel() + + templates := management.Templates{ + {Variant: ""}, + {Variant: "acme"}, + {Variant: "acme"}, + {Variant: "globex"}, + } + + declared := management.CampaignVariants{Options: []management.CampaignVariant{{Key: "acme"}}} + require.Equal(t, []string{"globex"}, undeclaredTemplateVariants(templates, declared)) + + both := management.CampaignVariants{Options: []management.CampaignVariant{{Key: "acme"}, {Key: "globex"}}} + require.Empty(t, undeclaredTemplateVariants(templates, both)) + + // Renaming acme to acme-corp undeclares acme while its templates remain. + renamed := management.CampaignVariants{Options: []management.CampaignVariant{{Key: "acme-corp"}, {Key: "globex"}}} + require.Equal(t, []string{"acme"}, undeclaredTemplateVariants(templates, renamed)) +} + +// Duplicating a campaign has to carry its variant declaration across as well as +// the template rows: templates keyed by a variant the copy no longer declares +// are invisible in the console and fall back to house branding on every send. +func TestDuplicateCampaignCarriesVariants(t *testing.T) { + t.Parallel() + + ctx := t.Context() + mgmt, _, _ := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + campaigns := management.NewCampaignsStore(mgmt) + + variants := management.CampaignVariants{ + Selector: &management.VariantSelector{Type: management.VariantSelectorStatic, Key: "acme"}, + Options: []management.CampaignVariant{{Key: "acme", Label: "Acme Corp"}}, + } + + copyID, err := campaigns.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, + Name: "Copy of Source", + Channel: "email", + Variants: store.JSONB[management.CampaignVariants]{Data: variants}, + }) + require.NoError(t, err) + + copied, err := campaigns.GetCampaign(ctx, projectID, copyID) + require.NoError(t, err) + require.Equal(t, variants.Options, copied.Variants.Data.Options) + require.Equal(t, *variants.Selector, *copied.Variants.Data.Selector) +} + +// A journey step's variant selector is only checked when the journey is +// published: draft saves stay lenient, so without this a mistyped static key +// reaches the send path and quietly delivers house branding to a client's +// recipients. +func TestValidateCampaignStepVariants(t *testing.T) { + t.Parallel() + + logger := zaptest.NewLogger(t) + ctx := t.Context() + mgmt, usrs, jrny := teststore.RunPostgreSQL(t) + + projects := management.NewProjectsStore(mgmt) + projectID, err := projects.CreateProject(ctx, DefaultProject) + require.NoError(t, err) + + mgmtState := management.NewState(mgmt) + campaignID, err := mgmtState.CampaignsStore.CreateCampaign(ctx, management.Campaign{ + ProjectID: projectID, + Name: "Test Campaign", + Channel: "email", + Variants: store.JSONB[management.CampaignVariants]{ + Data: management.CampaignVariants{Options: []management.CampaignVariant{{Key: "acme"}}}, + }, + }) + require.NoError(t, err) + + actor := rbac.NewActor(rbac.ActorAdmin, uuid.New().String(), + rbac.WithOrganizationID(uuid.New()), + rbac.WithProjectID(projectID), + ) + engine, _ := rbac.TestSetup(t, ctx, actor, "owner", "admin") + journeys := NewJourneysController(logger, jrny, usrs, mgmtState, nil, nil, engine, consumer.Namespace("")) + + campaignStep := func(t *testing.T, selector *oapi.VariantSelector) oapi.JourneyStepMap { + t.Helper() + raw, err := json.Marshal(oapi.CampaignStepData{CampaignId: campaignID, Variant: selector}) + require.NoError(t, err) + return oapi.JourneyStepMap{ + "step": oapi.JourneyStep{Type: oapi.JourneyStepTypeCampaign, Data: raw}, + } + } + + tests := map[string]struct { + selector *oapi.VariantSelector + wantErr bool + }{ + "no selector defers to the campaign": {selector: nil}, + "declared static key": { + selector: &oapi.VariantSelector{Type: "static", Key: ptr.To("acme")}, + }, + "empty static key pins the default variant": { + selector: &oapi.VariantSelector{Type: "static", Key: ptr.To("")}, + }, + "undeclared static key": { + selector: &oapi.VariantSelector{Type: "static", Key: ptr.To("globex")}, + wantErr: true, + }, + "expression": { + selector: &oapi.VariantSelector{Type: "expression", Expression: ptr.To("{{ user.data.tenant }}")}, + }, + "empty expression": { + selector: &oapi.VariantSelector{Type: "expression", Expression: ptr.To(" ")}, + wantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := journeys.validateCampaignStepVariants(ctx, projectID, campaignStep(t, test.selector)) + if test.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} diff --git a/internal/journeys/campaign.go b/internal/journeys/campaign.go index 7ec30d435..26e8ea182 100644 --- a/internal/journeys/campaign.go +++ b/internal/journeys/campaign.go @@ -36,6 +36,12 @@ func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state j // the journey context - entrance data, earlier step state - which no longer // exists once the send is rendered; the event therefore always carries a // static selector. Left unset, the campaign's own selector applies. + // + // An empty result is still a decision: the step asked for the default + // variant, or its expression matched nothing. Either way it must reach the + // send as a static selector pinning the default, because falling through to + // the campaign selector would re-brand a message the step deliberately kept + // on the house design. var variant *management.VariantSelector if config.Variant != nil { selector := management.VariantSelectorFromOAPI(*config.Variant) @@ -43,11 +49,9 @@ func HandleCampaign(ctx HandlerContext, step journey.JourneyVersionStep, state j if err != nil { return state, nil, fmt.Errorf("failed to resolve campaign variant: %w", err) } - if resolved != "" { - variant = &management.VariantSelector{ - Type: management.VariantSelectorStatic, - Key: resolved, - } + variant = &management.VariantSelector{ + Type: management.VariantSelectorStatic, + Key: resolved, } } diff --git a/internal/journeys/campaign_test.go b/internal/journeys/campaign_test.go new file mode 100644 index 000000000..5466a5741 --- /dev/null +++ b/internal/journeys/campaign_test.go @@ -0,0 +1,89 @@ +package journeys + +import ( + "context" + "encoding/json" + "testing" + + "github.com/google/uuid" + "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" + "github.com/lunogram/platform/internal/ptr" + "github.com/lunogram/platform/internal/pubsub/schemas" + "github.com/lunogram/platform/internal/store/journey" + "github.com/lunogram/platform/internal/store/management" + "github.com/stretchr/testify/require" +) + +// A campaign step resolves its variant against the journey context and hands +// the send a static selector, because the context it reads no longer exists at +// render time. An empty result is still a decision - the step asked for house +// branding, or its expression matched nothing - and has to reach the send as a +// pinned default rather than falling through to the campaign's own selector. +func TestHandleCampaignVariant(t *testing.T) { + t.Parallel() + + campaignID := uuid.New() + + tests := map[string]struct { + variant *oapi.VariantSelector + data map[string]any + want *management.VariantSelector + }{ + "no selector defers to the campaign": { + variant: nil, + want: nil, + }, + "static key is pinned as written": { + variant: &oapi.VariantSelector{Type: "static", Key: ptr.To("acme")}, + want: &management.VariantSelector{Type: management.VariantSelectorStatic, Key: "acme"}, + }, + "empty static key pins the default variant": { + variant: &oapi.VariantSelector{Type: "static", Key: ptr.To("")}, + want: &management.VariantSelector{Type: management.VariantSelectorStatic, Key: ""}, + }, + "expression resolves against the journey context": { + variant: &oapi.VariantSelector{Type: "expression", Expression: ptr.To("{{ user.data.tenant }}")}, + data: map[string]any{"user": map[string]any{"data": map[string]any{"tenant": "acme"}}}, + want: &management.VariantSelector{Type: management.VariantSelectorStatic, Key: "acme"}, + }, + "expression matching nothing still pins the default": { + variant: &oapi.VariantSelector{Type: "expression", Expression: ptr.To("{{ user.data.tenant }}")}, + data: map[string]any{"user": map[string]any{"data": map[string]any{}}}, + want: &management.VariantSelector{Type: management.VariantSelectorStatic, Key: ""}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + data, err := json.Marshal(oapi.CampaignStepData{ + CampaignId: campaignID, + Variant: test.variant, + }) + require.NoError(t, err) + + pub := &mockPublisher{} + hctx := HandlerContext{ + Context: context.Background(), + Publisher: pub, + ProjectID: uuid.New(), + UserID: uuid.New(), + Data: test.data, + } + + _, _, err = HandleCampaign(hctx, journey.JourneyVersionStep{Data: data}, journey.JourneyUserState{}) + require.NoError(t, err) + require.Len(t, pub.publishedEvents, 1) + + msg, ok := pub.publishedEvents[0].data.(schemas.SendCampaign) + require.True(t, ok) + + if test.want == nil { + require.Nil(t, msg.Variant) + return + } + require.Equal(t, *test.want, *msg.Variant) + }) + } +} diff --git a/internal/pubsub/consumer/campaigns.go b/internal/pubsub/consumer/campaigns.go index 76b588597..9b55add78 100644 --- a/internal/pubsub/consumer/campaigns.go +++ b/internal/pubsub/consumer/campaigns.go @@ -130,9 +130,10 @@ func createCampaignInboxMessageAndPublish(ctx context.Context, db *sqlx.DB, pub "template_id": item.TemplateID.String(), "campaign_id": event.CampaignID.String(), } - if item.Variant != "" { - provenance["variant"] = item.Variant - } + // Recorded unconditionally, empty string included: variant selection falls + // back silently by design, so an audit needs to see that a send went out + // under the default variant rather than infer it from a missing field. + provenance["variant"] = item.Variant if event.BroadcastID != nil { provenance["broadcast_id"] = event.BroadcastID.String() } diff --git a/internal/pubsub/consumer/campaigns_render.go b/internal/pubsub/consumer/campaigns_render.go index cf60ba7f8..cc9fd2799 100644 --- a/internal/pubsub/consumer/campaigns_render.go +++ b/internal/pubsub/consumer/campaigns_render.go @@ -108,12 +108,13 @@ func selectTemplate(templates management.Templates, variant string, user *subjec candidates = templatesForVariant(templates, "") } - // Neither the requested variant nor the default has a template. Every - // remaining template belongs to some other variant, so there is no - // on-brand answer left - send one anyway rather than dropping the message, - // and let the caller's fallback counter surface it. + // Neither the requested variant nor the default has a template - the + // default one can be deleted, so this is reachable. Every remaining + // template belongs to some other variant, and sending one would put another + // client's wording and sending domain in front of this recipient. Refuse: + // crossing a variant boundary is worse than not sending. if len(candidates) == 0 { - candidates = templates + return management.Template{}, Permanentf("campaign has no template for variant %q and none for the default variant", variant) } if len(candidates) == 1 { diff --git a/internal/pubsub/consumer/campaigns_variant_test.go b/internal/pubsub/consumer/campaigns_variant_test.go index e044e2797..956f69e42 100644 --- a/internal/pubsub/consumer/campaigns_variant_test.go +++ b/internal/pubsub/consumer/campaigns_variant_test.go @@ -72,15 +72,6 @@ func TestSelectTemplate(t *testing.T) { user: nil, want: house, }, - { - // No on-brand answer exists. Sending the wrong branding beats - // dropping the message; the caller counts this as a fallback. - name: "sends something when neither the variant nor the default has a template", - templates: management.Templates{acme}, - variant: "globex", - user: nil, - want: acme, - }, { name: "uses the only template when it is the default variant", templates: management.Templates{house}, @@ -108,6 +99,18 @@ func TestSelectTemplateWithoutTemplates(t *testing.T) { require.Error(t, err) } +// A campaign whose only templates belong to other variants has no on-brand +// answer left. Handing one over would send another client's wording and sending +// domain to this recipient, so the send fails instead. +func TestSelectTemplateRefusesToCrossVariants(t *testing.T) { + t.Parallel() + + acme := template("acme", "en") + + _, err := selectTemplate(management.Templates{acme}, "globex", nil, &management.Project{Locale: "en"}) + require.Error(t, err) +} + func TestResolveVariant(t *testing.T) { t.Parallel() diff --git a/internal/store/management/campaigns.go b/internal/store/management/campaigns.go index c49677c55..3d5d6ca25 100644 --- a/internal/store/management/campaigns.go +++ b/internal/store/management/campaigns.go @@ -103,12 +103,17 @@ type CampaignsStore struct { func (s *CampaignsStore) CreateCampaign(ctx context.Context, campaign Campaign) (uuid.UUID, error) { stmt := ` - INSERT INTO campaigns (project_id, name, channel, subscription_id, transactional) - VALUES ($1, $2, $3, $4, $5) + INSERT INTO campaigns (project_id, name, channel, subscription_id, transactional, variants) + VALUES ($1, $2, $3, $4, $5, $6) RETURNING id` + variants, err := campaign.Variants.Value() + if err != nil { + return uuid.Nil, err + } + var id uuid.UUID - err := s.db.GetContext(ctx, &id, stmt, campaign.ProjectID, campaign.Name, campaign.Channel, campaign.SubscriptionID, campaign.Transactional) + err = s.db.GetContext(ctx, &id, stmt, campaign.ProjectID, campaign.Name, campaign.Channel, campaign.SubscriptionID, campaign.Transactional, variants) if err != nil { return uuid.Nil, err } diff --git a/internal/store/management/variants.go b/internal/store/management/variants.go index 2bca433db..f2d98927a 100644 --- a/internal/store/management/variants.go +++ b/internal/store/management/variants.go @@ -2,6 +2,7 @@ package management import ( "fmt" + "regexp" "strings" "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" @@ -78,6 +79,12 @@ func (selector VariantSelector) Validate(variants CampaignVariants) error { } } +// variantKeyPattern constrains a declared variant key. Keys are written into +// template rows and into console dropdown values, so they are kept to a plain +// slug: leading punctuation is refused, which keeps the sentinel values the +// console reserves for "default" and "none" out of the declared set. +var variantKeyPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]*$`) + // CampaignVariant declares one white-labelled edition of a campaign. Key is // what a send resolves against to pick a set of templates; the empty key is the // default variant every campaign starts with and is never declared here. @@ -164,6 +171,9 @@ func CampaignVariantsFromOAPI(variants oapi.CampaignVariants) (CampaignVariants, if key == "" { return result, fmt.Errorf("variant key cannot be empty") } + if !variantKeyPattern.MatchString(key) { + return result, fmt.Errorf("variant key %s must start with a lowercase letter or digit and contain only lowercase letters, digits, dashes and underscores", key) + } if seen[key] { return result, fmt.Errorf("duplicate variant key %s", key) }