Skip to content

Commit c582e3b

Browse files
Make the mock Plone API enforce the form contract the way collective.volto.formsupport does (#307)
* Allow mock plone api fixture to handle both uppercase and lowercase "Subject" fields * mock: make @submit-form a real test double The mock accepted any POST to @submit-form and answered 204, so a form test could only ever prove that something was sent — not what, and not that it was acceptable. A frontend that posted the wrong block id, an empty body or a malformed address passed just as happily as a correct one. It now records each submission (keyed by content path + block_id, the way formsupport keys its stored records) and serves them back through @form-data, with an optional block_id filter and a DELETE to reset — so a test can assert on exactly what the frontend sent, including two forms on one page keeping separate result sets. It also reproduces the checks collective.volto.formsupport really makes, so a broken submission fails loudly instead of silently succeeding: empty form data (no entries and no attachments) is a 400, as its post adapter does; the honeypot captcha is a 400 unless `captcha.value` is the empty string, matching HoneypotSupport.verify; and a `from` field whose value is not an address is a 400, matching validate_email_fields. Block resolution walks the flattened block tree and refuses non-form blocks, like get_block_data. One divergence, recorded rather than hidden: formsupport treats an unresolvable block_id as an empty block and quietly does nothing, which in a test double would mask the one bug multi-form pages can have. Here the submission is still accepted, but the record carries `block_found` so a test can assert the id pointed at a real form. Branched from the commit the NSW frontend pins (4472bf8) rather than hydra main, so the pin bump that picks this up is exactly this change and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S41xYe9z2XDyH1ju435RmE * block-sanity: credit a typed object_list item to its own block type An object_list may be TYPED: each item names its own registered block type in a `typeField`, so one item is a `text` block and the next a `select`. Discovery ignored that. It read a block's type from `blockData['@type']`, which an object_list item does not have, and folded every item into one virtual `<parent>:<field>` type. The consequence only shows up once a frontend registers those per-item types, which the NSW form block is the first to do: all twelve form field types looked like blocks that were registered but that no content example ever exercised, and the coverage check failed for every one of them — while the fixtures did in fact contain an instance of each. buildBlockPathMap already resolves this correctly (`blockType: itemBlockType, // Real type (from typeField) or virtual type (from parent:field)`), so discovery now falls back to the path-map entry's type. `buildObjectListFieldsMap` carries `typeField` alongside `idField`, and the legacy `extractBlocks` walk uses it too. Also stops flagging an object_list item's identity fields as undeclared schema fields. `field_id` and `field_type` are in the stored data of every item and in no item schema — they say which block this is, not what the author wrote — so asking for them to be declared asks for a sidebar field that must never exist. The exempt names are derived from the schemas (every declared idField/typeField) rather than hardcoded, so a list keyed on something else is covered too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S41xYe9z2XDyH1ju435RmE * mock: reject a submission the way formsupport actually does Two of formsupport's own rejections were missing from the double, and one "divergence" I had documented turned out not to be one. `validate_form` raises on an unresolvable `block_id` (`block_form_not_found_label`) — I had read `get_block_data` returning an empty dict and concluded the real service quietly did nothing. It does not: it 400s, and the double now does too. It also raises `missing_action` for a form with neither `send` nor `store` — "You need to set at least one form action between send and store". That is easy to author by accident and invisible until a visitor submits, so a test double that accepts it hides a trap rather than catching it. The invalid-address message now names the field, as `wrong_email` does, since the frontend passes the backend's message through to the reader. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S41xYe9z2XDyH1ju435RmE * fieldRules: an empty array counts as unset The widgets that store arrays — multiselect, object_browser — leave `[]` behind when the author removes the last entry rather than dropping the key, so `isSet` read a field the author had just cleared as still answered, and any rule driven by "has anything been picked?" went on hiding what it was hiding. The `contains` tests already documented this as the behaviour ("isSet only tells you the array is non-empty"); now it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S41xYe9z2XDyH1ju435RmE * mock: enforce the answer rules the way formsupport does The @submit-form double checked required-ness and email shape but knew nothing about a field's `validations`, so a form could author a rule and the tests would prove only that we stored it. It now runs the validators formsupport registers — the four settable ones plus the regex ones a form is realistically authored with — and rejects with the backend's own per-field shape, `{"error": {"type": "Invalid", "errors": {field: {validator: message}}}}`, instead of the whole-form BadRequest it uses for everything else. Two things fall out of that. Skip logic is resolved first, so a rule on a field the conditions hid is not applied; and because the backend looks the trigger field up by `id` with no fallback to `field_id`, a field stored without one now fails here with that explanation rather than raising an IndexError in production. Reads also carry `validationSettings`, the catalogue of settable validators the serializer injects on every GET and the sidebar builds its rule-settings widget from — a fixture holding its own copy would be testing the copy. * mock: an absent captcha token is not a bot The @submit-form double rejected any submission to a honeypot-protected form that did not carry a `captcha` object. That is stricter than HoneypotSupport.verify, which only checks a token when one was sent and otherwise looks through the submitted data for a FILLED honeypot field — so a frontend that does not implement the token still submits. The Nuxt example is such a frontend, and its form tests went red: the submission was refused and the success message never rendered. The real fallback also rejects a submission missing the field entirely (`found_honeypot(form, required=True)`), but that rule is conditional on collective.honeypot's HONEYPOT_FIELD being configured — unset, the whole check short-circuits to "pass" — and there is no such environment here, so only the "present and filled" half is modelled. Noted at the call site. * Try to fix flakey test * Revert "Allow mock plone api fixture to handle both uppercase and lowercase "Subject" fields" This reverts commit 8a0e150. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f45794d commit c582e3b

5 files changed

Lines changed: 397 additions & 24 deletions

File tree

packages/volto-hydra/src/utils/blockSync.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,6 +2204,10 @@ function evaluateOperators(surface, operators) {
22042204
notRegex,
22052205
} = operators;
22062206

2207+
// Presence (isSet/isNotSet) goes through `isPresent`, which treats an empty
2208+
// array as unset — the array widgets (multiselect, object_browser) leave `[]`
2209+
// behind when the last entry is removed rather than dropping the key, so a
2210+
// field the author has just cleared must not still read as answered.
22072211
if (isSet !== undefined && (isSet ? !isPresent(surface) : isPresent(surface)))
22082212
return false;
22092213
if (

packages/volto-hydra/src/utils/blockSync.test.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,9 +885,14 @@ describe('fieldRules — multiselect surface (array of values)', () => {
885885
expect(run(undefined, { f: { gt: 0 } })).toBe(false);
886886
});
887887

888-
test('isSet — empty selection is unset', () => {
888+
test('isSet / isNotSet — empty selection is unset', () => {
889+
// Array widgets (multiselect, object_browser) leave `[]` behind when the
890+
// last entry is removed rather than dropping the key, so a field the author
891+
// has just cleared must read as unset — not still answered.
889892
expect(run(['image'], { f: { isSet: true } })).toBe(true);
890893
expect(run([], { f: { isSet: true } })).toBe(false);
894+
expect(run([], { f: { isNotSet: true } })).toBe(true);
895+
expect(run(['image'], { f: { isNotSet: true } })).toBe(false);
891896
});
892897

893898
test('oneOf and regex throw on an array field', () => {

0 commit comments

Comments
 (0)