From 39c9b4e0c4bcea976cfdf5ac400266eecc3f924c Mon Sep 17 00:00:00 2001 From: Jason Skomorowski Date: Thu, 13 Aug 2026 13:29:39 -0400 Subject: [PATCH 1/2] Drop workarounds now that backend respects PUT semantics --- ui-rs/src/settings/templates/TemplateForm.js | 7 +----- .../settings/templates/TemplateForm.test.js | 22 ------------------- ui-rs/src/settings/templates/mapping.js | 6 ++--- ui-rs/src/settings/templates/mapping.test.js | 11 ++++------ 4 files changed, 8 insertions(+), 38 deletions(-) diff --git a/ui-rs/src/settings/templates/TemplateForm.js b/ui-rs/src/settings/templates/TemplateForm.js index 92779e0..527ddbe 100644 --- a/ui-rs/src/settings/templates/TemplateForm.js +++ b/ui-rs/src/settings/templates/TemplateForm.js @@ -234,11 +234,6 @@ const TemplateForm = ({ initialValues, onSubmit, onClose, title, submitLabelId, return errors; }; - // Swapping patron for staff is an ordinary update; only clearing one is impossible, - // since PUT can set an audience but never restore the "matches both" null. So the - // choice is withheld rather than offered and then refused. - const audiences = editing && initialValues?.audience ? AUDIENCES.filter(Boolean) : AUDIENCES; - // Whether the body currently holds markup, which is not the same question as // which content type is selected: switching to text leaves the markup alone. const bodyIsMarkup = useRef(initialValues?.contentType === 'html'); @@ -335,7 +330,7 @@ const TemplateForm = ({ initialValues, onSubmit, onClose, title, submitLabelId, id="template-audience" name="audience" component={Select} - dataOptions={opts('audience', audiences)} + dataOptions={opts('audience', AUDIENCES)} label={} /> diff --git a/ui-rs/src/settings/templates/TemplateForm.test.js b/ui-rs/src/settings/templates/TemplateForm.test.js index fecc090..ef719ea 100644 --- a/ui-rs/src/settings/templates/TemplateForm.test.js +++ b/ui-rs/src/settings/templates/TemplateForm.test.js @@ -254,28 +254,6 @@ describe('TemplateForm', () => { expect(byId('template-purpose')).toBeDisabled(); }); - describe('audience', () => { - const optionValues = () => [...byId('template-audience').options].map(opt => opt.value); - - it('cannot be cleared once set, which the broker cannot restore to "both"', () => { - renderForm(jest.fn(), { - editing: true, - initialValues: { ...baseInitial, title: 'T', body: 'B', subject: 'S', labels: ['l'], audience: 'patron' }, - }); - - expect(optionValues()).toEqual(['patron', 'staff']); - }); - - it('can still be narrowed from "both", which needs no restoring', () => { - renderForm(jest.fn(), { - editing: true, - initialValues: { ...baseInitial, title: 'T', body: 'B', subject: 'S', labels: ['l'] }, - }); - - expect(optionValues()).toEqual(['', 'patron', 'staff']); - }); - }); - describe('labels', () => { it('offers built-in and existing labels before a preset is selected', async () => { renderForm(jest.fn(), { initialValues: { ...baseInitial, purpose: '' } }); diff --git a/ui-rs/src/settings/templates/mapping.js b/ui-rs/src/settings/templates/mapping.js index c0598d4..d02c3bd 100644 --- a/ui-rs/src/settings/templates/mapping.js +++ b/ui-rs/src/settings/templates/mapping.js @@ -39,10 +39,10 @@ export function buildUpdateTemplateBody(values = {}) { body: values.body, contentType: values.contentType, labels: cleanLabels(values.labels), - subject: (values.subject ?? '').trim(), }; - // Omitted rather than sent empty: "" matches neither an audience nor the IS NULL - // "both" case, making the template unreachable. The form rejects clearing it. + // Omitted optional fields are cleared by PUT. + const subject = (values.subject ?? '').trim(); + if (subject) updated.subject = subject; if (values.audience) updated.audience = values.audience; return updated; } diff --git a/ui-rs/src/settings/templates/mapping.test.js b/ui-rs/src/settings/templates/mapping.test.js index df69f2e..b93cb3f 100644 --- a/ui-rs/src/settings/templates/mapping.test.js +++ b/ui-rs/src/settings/templates/mapping.test.js @@ -83,13 +83,10 @@ describe('buildUpdateTemplateBody', () => { expect(buildUpdateTemplateBody(values)).not.toHaveProperty('purpose'); }); - it('sends an empty subject so clearing one takes effect', () => { - // PUT leaves omitted fields untouched, so omission would silently keep the old value. - expect(buildUpdateTemplateBody({ ...values, subject: '' }).subject).toBe(''); - }); - - it('omits a cleared audience, which would otherwise be stored as an unmatchable empty string', () => { - expect(buildUpdateTemplateBody({ ...values, audience: '' })).not.toHaveProperty('audience'); + it('omits blank optional fields', () => { + const body = buildUpdateTemplateBody({ ...values, subject: ' ', audience: '' }); + expect(body).not.toHaveProperty('subject'); + expect(body).not.toHaveProperty('audience'); }); }); From abc508d5b7ba04b95683e67d37c0121d304bef6c Mon Sep 17 00:00:00 2001 From: Jason Skomorowski Date: Tue, 18 Aug 2026 19:47:56 -0400 Subject: [PATCH 2/2] Copilot made good points --- ui-rs/src/settings/templates/TemplateForm.test.js | 9 +++++++++ ui-rs/src/settings/templates/mapping.js | 2 +- ui-rs/src/settings/templates/mapping.test.js | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ui-rs/src/settings/templates/TemplateForm.test.js b/ui-rs/src/settings/templates/TemplateForm.test.js index ef719ea..5b0a4f0 100644 --- a/ui-rs/src/settings/templates/TemplateForm.test.js +++ b/ui-rs/src/settings/templates/TemplateForm.test.js @@ -254,6 +254,15 @@ describe('TemplateForm', () => { expect(byId('template-purpose')).toBeDisabled(); }); + it('offers "both" when editing a template that has an audience, which PUT can now restore', () => { + renderForm(jest.fn(), { + editing: true, + initialValues: { ...baseInitial, title: 'T', body: 'B', subject: 'S', labels: ['l'], audience: 'patron' }, + }); + + expect([...byId('template-audience').options].map(opt => opt.value)).toEqual(['', 'patron', 'staff']); + }); + describe('labels', () => { it('offers built-in and existing labels before a preset is selected', async () => { renderForm(jest.fn(), { initialValues: { ...baseInitial, purpose: '' } }); diff --git a/ui-rs/src/settings/templates/mapping.js b/ui-rs/src/settings/templates/mapping.js index d02c3bd..43fad2a 100644 --- a/ui-rs/src/settings/templates/mapping.js +++ b/ui-rs/src/settings/templates/mapping.js @@ -42,7 +42,7 @@ export function buildUpdateTemplateBody(values = {}) { }; // Omitted optional fields are cleared by PUT. const subject = (values.subject ?? '').trim(); - if (subject) updated.subject = subject; + if (subject && values.purpose !== 'pullslip') updated.subject = subject; if (values.audience) updated.audience = values.audience; return updated; } diff --git a/ui-rs/src/settings/templates/mapping.test.js b/ui-rs/src/settings/templates/mapping.test.js index b93cb3f..f6f31dd 100644 --- a/ui-rs/src/settings/templates/mapping.test.js +++ b/ui-rs/src/settings/templates/mapping.test.js @@ -88,6 +88,10 @@ describe('buildUpdateTemplateBody', () => { expect(body).not.toHaveProperty('subject'); expect(body).not.toHaveProperty('audience'); }); + + it('omits the subject for pull slips, clearing one that was stored', () => { + expect(buildUpdateTemplateBody({ ...values, purpose: 'pullslip' })).not.toHaveProperty('subject'); + }); }); describe('escapeHtml', () => {