Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ All notable changes to this project will be documented in this file.

- **Fix**: Create Notification is now disabled, and Edit hidden, on the transactional notifications page for members without Templates read: creating offered a template picker they could not load, and saving an edit was refused. Editing a notification managed by an integration stays available with Transactional write alone. Its edit drawer then shows the template's ID, with a note that Templates read is needed to see the template, instead of an empty field, and the page no longer requests templates without Templates read.

- **Fix**: A member without Templates read now sees the template's ID instead of an empty field or "Untitled" in a list's double opt-in template, a broadcast's templates, an automation's email nodes and a segment's email conditions, and these screens no longer request templates such a member cannot read. The template fields carry a lock saying that Templates read is needed.

- **Fix**: `transactional.update`, `transactional.delete` and `templates.delete` now answer 400 with the reason, instead of 500, when they refuse to change the name or channels of a notification managed by an integration, or to delete such a notification or template. `templates.delete` also answers a missing permission with 403 and a revoked API key with 401, as the other endpoints do, instead of 500.

- **Fix**: Liquid in templates now matches Shopify Liquid in more cases: `{% if order.total == 100 %}` matches numbers from your data, accented text is no longer corrupted by `capitalize` or `truncate`, amounts are exact (`0.1 | plus: 0.2` is `0.3`), `date` supports `%-d` and formats timestamps, and conditions on a text containing "and" or "or", tags written over several lines and `{%- raw -%}` work.
Expand Down
30 changes: 27 additions & 3 deletions console/src/components/automations/NodeConfigPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it, expect, vi } from 'vitest'
import { describe, it, expect, vi, afterEach } from 'vitest'
import { render, screen, fireEvent } from '@testing-library/react'
import { i18n } from '@lingui/core'
import { I18nProvider } from '@lingui/react'
Expand All @@ -10,7 +10,9 @@ import type { NodeType } from '../../services/api/automation'
// The per-type forms are exercised by their own tests; stubbing the barrel keeps this one about
// the description field the panel owns, and keeps a single textbox on screen to address.
vi.mock('./config', () => {
const Stub = () => <div data-testid="type-form" />
const Stub = ({ canReadTemplates }: { canReadTemplates?: boolean }) => (
<div data-testid="type-form" data-can-read-templates={String(canReadTemplates)} />
)
return {
TriggerConfigForm: Stub,
DelayConfigForm: Stub,
Expand All @@ -24,8 +26,10 @@ vi.mock('./config', () => {
}
})

const context = vi.hoisted(() => ({ canReadTemplates: true }))

vi.mock('./context', () => ({
useAutomation: () => ({ workspace: { id: 'ws-1' } })
useAutomation: () => ({ workspace: { id: 'ws-1' }, canReadTemplates: context.canReadTemplates })
}))

// The NodeType union has no runtime counterpart; build the list from a map typed against it so a
Expand Down Expand Up @@ -158,3 +162,23 @@ describe('NodeConfigPanel description', () => {
expect(container).toBeEmptyDOMElement()
})
})

// Without Templates read the trigger's template filter and the email node's template are locked:
// the panel hands the editor's permission to both forms.
describe('NodeConfigPanel Templates read', () => {
afterEach(() => {
context.canReadTemplates = true
})

it.each([
{ nodeType: 'trigger' as NodeType, canRead: false },
{ nodeType: 'email' as NodeType, canRead: false },
{ nodeType: 'trigger' as NodeType, canRead: true },
{ nodeType: 'email' as NodeType, canRead: true }
])('passes $canRead to the $nodeType form', ({ nodeType, canRead }) => {
context.canReadTemplates = canRead
renderPanel(makeNode('n1', nodeType, {}))

expect(screen.getByTestId('type-form').getAttribute('data-can-read-templates')).toBe(String(canRead))
})
})
4 changes: 3 additions & 1 deletion console/src/components/automations/NodeConfigPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const NodeConfigPanel: React.FC<NodeConfigPanelProps> = ({
onClose
}) => {
const { t } = useLingui()
const { workspace } = useAutomation()
const { workspace, canReadTemplates } = useAutomation()

if (!selectedNode) {
return null
Expand Down Expand Up @@ -99,6 +99,7 @@ export const NodeConfigPanel: React.FC<NodeConfigPanelProps> = ({
onChange={handleConfigChange}
workspaceId={workspaceId}
workspace={workspace}
canReadTemplates={canReadTemplates}
/>
)
case 'delay':
Expand All @@ -115,6 +116,7 @@ export const NodeConfigPanel: React.FC<NodeConfigPanelProps> = ({
onChange={handleConfigChange}
workspaceId={workspaceId}
workspace={workspace}
canReadTemplates={canReadTemplates}
/>
)
case 'ab_test':
Expand Down
75 changes: 75 additions & 0 deletions console/src/components/automations/UpsertAutomationDrawer.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import type { ReactNode } from 'react'
import { describe, it, expect, vi } from 'vitest'
import { render, screen, fireEvent } from '@testing-library/react'
import { App } from 'antd'
import { i18n } from '@lingui/core'
import { I18nProvider } from '@lingui/react'
import { UpsertAutomationDrawer } from './UpsertAutomationDrawer'
import type { Workspace } from '../../services/api/types'

// jsdom lacks matchMedia, which antd's Drawer touches.
vi.stubGlobal('matchMedia', () => ({
matches: false,
addListener() {},
removeListener() {},
addEventListener() {},
removeEventListener() {}
}))

// The provider exposes only what this file checks: the permission it gets. The editor inside it
// is exercised by its own tests.
vi.mock('./context', () => ({
AutomationProvider: ({
canReadTemplates,
children
}: {
canReadTemplates: boolean
children: ReactNode
}) => (
<div data-testid="automation-provider" data-can-read-templates={String(canReadTemplates)}>
{children}
</div>
),
// What the drawer's header reads, inert.
useAutomation: () => ({
isEditing: false,
name: '',
setName: () => {},
listId: undefined,
setListId: () => {},
exitOnReply: false,
setExitOnReply: () => {},
lists: [],
workspace: { id: 'ws1', integrations: [] },
hasUnsavedChanges: false,
isSaving: false,
save: () => Promise.resolve(),
validate: () => [],
canUndo: false,
canRedo: false,
undo: () => {},
redo: () => {}
})
}))

const workspace = { id: 'ws1', name: 'Workspace' } as unknown as Workspace

vi.mock('./AutomationFlowEditor', () => ({ AutomationFlowEditor: () => null }))

describe('UpsertAutomationDrawer', () => {
it.each([false, true])('gives the editor Templates read %s', (canRead) => {
render(
<I18nProvider i18n={i18n}>
<App>
<UpsertAutomationDrawer workspace={workspace} canReadTemplates={canRead} />
</App>
</I18nProvider>
)

// The provider mounts when the drawer opens.
fireEvent.click(screen.getByRole('button', { name: 'Create Automation' }))
expect(screen.getByTestId('automation-provider').getAttribute('data-can-read-templates')).toBe(
String(canRead)
)
})
})
4 changes: 4 additions & 0 deletions console/src/components/automations/UpsertAutomationDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ interface UpsertAutomationDrawerProps {
lists?: List[]
segments?: Segment[]
templates?: Template[]
// Without Templates read the template fields of the canvas show the stored IDs, locked.
canReadTemplates: boolean
// Controlled mode props
open?: boolean
onOpenChange?: (open: boolean) => void
Expand Down Expand Up @@ -247,6 +249,7 @@ export function UpsertAutomationDrawer({
lists = [],
segments = [],
templates = [],
canReadTemplates,
open: controlledOpen,
onOpenChange
}: UpsertAutomationDrawerProps) {
Expand Down Expand Up @@ -312,6 +315,7 @@ export function UpsertAutomationDrawer({
lists={lists}
segments={segments}
templates={templates}
canReadTemplates={canReadTemplates}
onSaveSuccess={handleSaveSuccess}
onClose={handleClose}
>
Expand Down
35 changes: 31 additions & 4 deletions console/src/components/automations/config/ConditionsField.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useState } from 'react'
import { describe, it, expect, vi } from 'vitest'
import { describe, it, expect, vi, afterEach } from 'vitest'
import { render, screen, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { i18n } from '@lingui/core'
Expand All @@ -14,20 +14,33 @@ class ResizeObserverStub {
}
vi.stubGlobal('ResizeObserver', ResizeObserverStub)

const context = vi.hoisted(() => ({ canReadTemplates: true }))

vi.mock('../context', () => ({
useAutomation: () => ({
lists: [{ id: 'list123', name: 'My List' }],
workspace: { id: 'ws1' }
workspace: { id: 'ws1' },
canReadTemplates: context.canReadTemplates
})
}))

// The real TreeNodeInput commits a leaf the instant a source is picked from the cascader,
// before any filter exists. The mock has to be able to do the same, or the abandonment path
// below is untestable.
vi.mock('../../segment/input', () => ({
TreeNodeInput: ({ onChange }: { onChange: (tree: TreeNode) => void }) => (
TreeNodeInput: ({
onChange,
canReadTemplates
}: {
onChange: (tree: TreeNode) => void
canReadTemplates?: boolean
}) => (
<>
<button data-testid="tree-editor" onClick={() => onChange(twoLeafTree)}>
<button
data-testid="tree-editor"
data-can-read-templates={String(canReadTemplates)}
onClick={() => onChange(twoLeafTree)}
>
tree editor
</button>
<button data-testid="pick-source" onClick={() => onChange(incompleteTree)}>
Expand Down Expand Up @@ -123,6 +136,10 @@ const confirmRemove = async () => {
}

describe('ConditionsField', () => {
afterEach(() => {
context.canReadTemplates = true
})

it('shows the add affordance and no editor when there is nothing configured', () => {
renderField(undefined)

Expand Down Expand Up @@ -150,6 +167,16 @@ describe('ConditionsField', () => {
expect(screen.getByText('1 condition')).toBeInTheDocument()
})

// Without Templates read an email condition names its template by ID, locked: the editor gets
// the automation's permission.
it.each([false, true])('gives the editor Templates read %s', async (canRead) => {
context.canReadTemplates = canRead
renderField(oneLeafTree)

await userEvent.click(screen.getByText('Edit'))
expect(screen.getByTestId('tree-editor').getAttribute('data-can-read-templates')).toBe(String(canRead))
})

it('keeps the editor out of the panel until it is asked for', async () => {
renderField(oneLeafTree)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const ConditionsField: React.FC<ConditionsFieldProps> = ({
notice
}) => {
const { t } = useLingui()
const { lists, workspace } = useAutomation()
const { lists, workspace, canReadTemplates } = useAutomation()
const [open, setOpen] = useState(false)

const configured = HasLeaf(value)
Expand Down Expand Up @@ -143,6 +143,7 @@ export const ConditionsField: React.FC<ConditionsFieldProps> = ({
schemas={TableSchemas}
lists={lists}
workspaceId={workspace.id}
canReadTemplates={canReadTemplates}
/>
</Drawer>
</>
Expand Down
45 changes: 45 additions & 0 deletions console/src/components/automations/config/EmailConfigForm.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { describe, it, expect, vi } from 'vitest'
import { render, screen } from '@testing-library/react'
import { i18n } from '@lingui/core'
import { I18nProvider } from '@lingui/react'
import { EmailConfigForm } from './EmailConfigForm'
import type { Workspace } from '../../../services/api/types'

// The selector queries the API; here it only needs to show what it was given.
vi.mock('../../templates/TemplateSelectorInput', () => ({
default: ({ value, canReadTemplates }: { value?: string | null; canReadTemplates?: boolean }) => (
<span data-testid="template-selector" data-can-read-templates={String(canReadTemplates)}>
template:{value ?? 'none'}
</span>
)
}))

const workspace = { id: 'ws1', integrations: [] } as unknown as Workspace

const renderForm = (canReadTemplates?: boolean) =>
render(
<I18nProvider i18n={i18n}>
<EmailConfigForm
config={{ template_id: 'tpl-welcome' }}
onChange={vi.fn()}
workspaceId="ws1"
workspace={workspace}
canReadTemplates={canReadTemplates}
/>
</I18nProvider>
)

// Without Templates read the email node's template shows the stored ID, locked.
describe('EmailConfigForm template', () => {
it.each([
{ given: false, passed: 'false' },
{ given: true, passed: 'true' },
{ given: undefined, passed: 'true' }
])('gives the selector Templates read $given', ({ given, passed }) => {
renderForm(given)

const selector = screen.getByTestId('template-selector')
expect(selector).toHaveTextContent('template:tpl-welcome')
expect(selector.getAttribute('data-can-read-templates')).toBe(passed)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ interface EmailConfigFormProps {
onChange: (config: EmailNodeConfig) => void
workspaceId: string
workspace: Workspace
// Without Templates read the template field shows the stored ID, locked. Defaults to true.
canReadTemplates?: boolean
}

export const EmailConfigForm: React.FC<EmailConfigFormProps> = ({
config,
onChange,
workspaceId,
workspace
workspace,
canReadTemplates = true
}) => {
const { t } = useLingui()

Expand Down Expand Up @@ -72,6 +75,7 @@ export const EmailConfigForm: React.FC<EmailConfigFormProps> = ({
onChange={handleTemplateChange}
workspaceId={workspaceId}
placeholder={t`Select email template...`}
canReadTemplates={canReadTemplates}
/>
</Form.Item>

Expand Down
Loading
Loading