diff --git a/README.md b/README.md index f121d21..7aeaeeb 100644 --- a/README.md +++ b/README.md @@ -447,7 +447,7 @@ Helpers and send-flow primitives also exported: ```ts import { - toSlackBlocks, // strips builder-only fields (e.g. header `level`) before sending + toSlackBlocks, // scrubs unsafe URLs + retrieval-only fields before sending encodeBlocksToString, // base64url-encode a blocks array (for URL state) decodeBlocksFromString, defaultPalette, // the built-in palette — spread to customize diff --git a/src/components/editors/block-editor.stories.tsx b/src/components/editors/block-editor.stories.tsx index 74b88a8..7db2539 100644 --- a/src/components/editors/block-editor.stories.tsx +++ b/src/components/editors/block-editor.stories.tsx @@ -332,6 +332,22 @@ export const TypingHeaderTextProducesValidBlock: Story = { } }; +// Regression: header `level` is a real Slack field, so picking a level in +// the editor must survive `toSlackBlocks` and reach the wire payload. +// It used to be stripped on the way out, silently discarding the choice. +export const SelectingHeaderLevelProducesValidBlock: Story = { + args: { block: variant('structure_header') }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByLabelText(/^h2$/i)); + await expect(args.onChange).toHaveBeenCalled(); + const latest = expectLastOnChangeIsValid(args.onChange as ReturnType); + expect(latest.type).toBe('header'); + expect(latest).toHaveProperty('level', 2); + expect(toSlackBlocks([latest])[0]).toHaveProperty('level', 2); + } +}; + export const TypingMarkdownTextProducesValidBlock: Story = { args: { block: variant('markdown_list') }, play: async ({ canvasElement, args }) => { diff --git a/src/components/editors/header-editor.tsx b/src/components/editors/header-editor.tsx index a0c7a36..0961525 100644 --- a/src/components/editors/header-editor.tsx +++ b/src/components/editors/header-editor.tsx @@ -13,7 +13,7 @@ const HEADER_LEVELS: HeaderLevel[] = [1, 2, 3, 4]; /** * Editor form for header blocks. Slack enforces plain text only. * Shows a character count against the 150-char Slack limit, plus an - * optional 1-4 level selector (builder-only extension; Slack ignores it). + * optional 1-4 heading level selector. * @param props - editor props * @param props.block - the header block to edit * @param props.onChange - called with the updated block payload @@ -44,7 +44,7 @@ export function HeaderEditor({ block, onChange }: BlockEditorProps - + { diff --git a/src/lib/to-slack-blocks.ts b/src/lib/to-slack-blocks.ts index 55364d0..fc83f34 100644 --- a/src/lib/to-slack-blocks.ts +++ b/src/lib/to-slack-blocks.ts @@ -2,24 +2,23 @@ import type { SupportedBlock } from '../types'; import { sanitizeBlock } from './sanitize-blocks'; /** - * Strip builder-only fields from blocks so they conform to Slack's - * Block Kit schema, and scrub dangerous URI schemes from any - * `url`/`image_url` fields. Currently removes the cosmetic `level` - * field from header blocks (a builder-only extension Slack would - * reject) and routes every URL/image-url through the allowlist in - * `lib/url-safety.ts` so a payload that round-trips through the - * builder cannot carry `javascript:`/`data:text/html` URIs to a - * downstream consumer or to the Slack API. + * Prepare blocks for the Slack API: scrub dangerous URI schemes from any + * `url`/`image_url` fields and drop the read-only metadata Slack attaches + * on retrieval but rejects on send. Every URL/image-url is routed through + * the allowlist in `lib/url-safety.ts` so a payload that round-trips + * through the builder cannot carry `javascript:`/`data:text/html` URIs to + * a downstream consumer or to the Slack API. + * + * Header `level` is passed through: it is a real Slack field (an integer + * 1-4, see https://docs.slack.dev/reference/block-kit/blocks/header-block), + * not a builder-only extension. Earlier versions stripped it here, which + * silently dropped the heading level from every sent message. Out-of-range + * values are left intact so they surface as validation errors rather than + * being quietly discarded. + * * @param blocks - the working draft blocks - * @returns blocks with builder-only fields removed and URLs scrubbed + * @returns blocks with retrieval-only fields removed and URLs scrubbed */ export function toSlackBlocks(blocks: SupportedBlock[]): SupportedBlock[] { - return blocks.map((block) => { - const safe = sanitizeBlock(block); - if (safe.type === 'header' && 'level' in safe) { - const { level: _omit, ...rest } = safe; - return rest as SupportedBlock; - } - return safe; - }); + return blocks.map((block) => sanitizeBlock(block)); } diff --git a/src/types.ts b/src/types.ts index 0a1285d..ee50877 100644 --- a/src/types.ts +++ b/src/types.ts @@ -407,15 +407,17 @@ export interface ContainerBlock { } /** - * Header heading level shown in the preview. Slack's API has no - * `level` field on header blocks, so this is a builder-only extension - * that round-trips on the block payload but is otherwise cosmetic. + * Header heading level. A real Slack field on header blocks — an optional + * integer 1-4 that Slack renders at the corresponding heading size — so it + * round-trips through {@link SupportedBlock} and is sent to the API. + * @see https://docs.slack.dev/reference/block-kit/blocks/header-block */ export type HeaderLevel = 1 | 2 | 3 | 4; /** * Header block as edited by the builder. Extends Slack's HeaderBlock - * with an optional {@link HeaderLevel}. + * with an optional {@link HeaderLevel}, which the bundled + * `slack-web-api-client` types predate. */ export type SupportedHeaderBlock = HeaderBlock & { level?: HeaderLevel }; diff --git a/test/public-api.test.ts b/test/public-api.test.ts index 6f49320..dcbb0ea 100644 --- a/test/public-api.test.ts +++ b/test/public-api.test.ts @@ -6,7 +6,10 @@ import { decodeBlocksFromString, encodeBlocksToString } from '../src/lib/url-sta import type { SupportedBlock } from '../src/types'; describe('toSlackBlocks', () => { - it('strips the builder-only `level` field from header blocks', () => { + // Regression: `level` is a real Slack header field (integer 1-4), not a + // builder-only extension. It used to be stripped here, which silently + // dropped the heading level from every message the builder sent. + it('preserves the `level` field on header blocks', () => { const input: SupportedBlock[] = [ { type: 'header', @@ -17,7 +20,40 @@ describe('toSlackBlocks', () => { const [out] = toSlackBlocks(input); expect(out.type).toBe('header'); + expect(out).toEqual(input[0]); + }); + + it('emits header `level` in a payload the validator accepts', () => { + for (const level of [1, 2, 3, 4]) { + const payload = toSlackBlocks([ + { + type: 'header', + level, + text: { type: 'plain_text', text: 'Heading', emoji: true } + } as SupportedBlock + ]); + expect(payload[0]).toHaveProperty('level', level); + expect(validateBlockKit(payload, { target: 'blocks' }).valid).toBe(true); + } + }); + + it('leaves an out-of-range header `level` intact so it surfaces as a validation error', () => { + const payload = toSlackBlocks([ + { + type: 'header', + level: 6, + text: { type: 'plain_text', text: 'Heading', emoji: true } + } as unknown as SupportedBlock + ]); + expect(payload[0]).toHaveProperty('level', 6); + expect(validateBlockKit(payload, { target: 'blocks' }).valid).toBe(false); + }); + + it('leaves a header block without a `level` untouched', () => { + const input: SupportedBlock[] = [{ type: 'header', text: { type: 'plain_text', text: 'Heading', emoji: true } }]; + const [out] = toSlackBlocks(input); expect('level' in out).toBe(false); + expect(out).toBe(input[0]); }); it('passes non-header blocks through unchanged', () => {