fix(admin): a text setting that is meant to be empty cannot be cleared #280

New Issue

2026-09-03T09:16:00-05:00

bermudalamb commented

2026-09-03 09:16:00 -05:00

PUT /api/admin/settings refuses any empty text value:

if (typeof raw !== 'string' || raw.trim() === '') {
  return res.status(400).json({ error: `${name} cannot be empty` });
}

That is right for two of the four text settings and wrong for the other two.

Setting	Fallback	Empty is…
`greetingFormat`	`Hi {{firstName}},`	wrong — an empty format renders every greeting as nothing, which reads as a bug in the email
`greetingFallback`	`Hi,`	wrong, same reason
`intakeNotifyEmail`	`''`	the documented default, meaning "do not notify"
`intakeCeilingResetAt`	`''`	the documented default, meaning "no reset recorded"

So intakeNotifyEmail can be set but never cleared. Once an address is in there, turning intake notifications back off through the admin is impossible — the only way is a direct DELETE against admin_settings. Same for the ceiling reset timestamp.

My bug, introduced in #224: I added a text setting whose whole design says empty is a working configuration, into a validator that had been written when every text setting had to be non-empty, and did not notice the two disagreed.

Fix

Whether empty is allowed belongs with the setting, not with the type. Add a flag to the DEFINITIONS row — the same place the type and fallback already live, so a new setting declares it once and nothing else needs to know.

The blanket check stays the default, because for a setting with a non-empty fallback, refusing empty is correct and is the case the message was written for.

Verification

Setting intakeNotifyEmail to '' succeeds and getSettings() then reports ''. Setting greetingFormat to '' is still refused. Found while clearing the code smells for #181.

`PUT /api/admin/settings` refuses any empty text value: ```ts if (typeof raw !== 'string' || raw.trim() === '') { return res.status(400).json({ error: `${name} cannot be empty` }); } ``` That is right for two of the four text settings and wrong for the other two. | Setting | Fallback | Empty is… | |---|---|---| | `greetingFormat` | `Hi {{firstName}},` | wrong — an empty format renders every greeting as nothing, which reads as a bug in the email | | `greetingFallback` | `Hi,` | wrong, same reason | | `intakeNotifyEmail` | `''` | **the documented default**, meaning "do not notify" | | `intakeCeilingResetAt` | `''` | **the documented default**, meaning "no reset recorded" | So `intakeNotifyEmail` can be set but never cleared. Once an address is in there, turning intake notifications back off through the admin is impossible — the only way is a direct `DELETE` against `admin_settings`. Same for the ceiling reset timestamp. My bug, introduced in #224: I added a text setting whose whole design says empty is a working configuration, into a validator that had been written when every text setting had to be non-empty, and did not notice the two disagreed. ## Fix Whether empty is allowed belongs with the setting, not with the type. Add a flag to the `DEFINITIONS` row — the same place the type and fallback already live, so a new setting declares it once and nothing else needs to know. The blanket check stays the default, because for a setting with a non-empty fallback, refusing empty is correct and is the case the message was written for. ## Verification Setting `intakeNotifyEmail` to `''` succeeds and `getSettings()` then reports `''`. Setting `greetingFormat` to `''` is still refused. Found while clearing the code smells for #181.

bermudalamb referenced this issue from a commit

2026-09-03 10:49:51 -05:00

chore: clear the six code smells SonarQube reported (#181)

bermudalamb referenced this issue

2026-09-03 10:50:42 -05:00

chore: clear the six code smells SonarQube reported (#181) #282

bermudalamb referenced this issue from a commit

2026-09-03 16:52:48 -05:00

fix(admin): let a setting whose default is empty actually be cleared (#280)

bermudalamb referenced a pull request that will close this issue

2026-09-03 16:53:17 -05:00

fix(admin): let a setting whose default is empty actually be cleared (#280) #290