Repository navigation
Reject groups inside groups and group rules on their own questions in the API - #220
Conversation
The sequence API only accepts blocks of the form and a top-level group as scope, and a group keeps no scope. A rule on a group can't use a question inside that group, by API and by template import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecks # Conflicts: # CHANGELOG.md
PhilReinking
left a comment
PhilReinking
left a comment
There was a problem hiding this comment.
OK at 6bebabd, CI green.
Checked beyond the PR text: the editor's real payload (every block, groups at the top level) saves, a mixed valid and invalid sequence saves nothing, and group rules on questions in another group still save. An old nested group blocks the editor, and one API call with that group's scope set to null fixes it. Two older gaps of the same kind are inline, fine as a follow-up too.
| 'sequence' => 'required|array', | ||
| 'sequence.*.id' => ['required', Rule::in($form->formBlocks->pluck('id'))], | ||
| 'sequence.*.scope' => [ | ||
| 'nullable', |
There was a problem hiding this comment.
A sequence item without a scope key still gives a 500, after the items before it are saved. With present it's a 422 before anything saves:
| 'nullable', | |
| 'present', | |
| 'nullable', |
| 'blocks.*.id' => 'nullable|string', | ||
| 'blocks.*.type' => ['required', Rule::enum(FormBlockType::class)], | ||
| // same as the sequence API, a group stays at the top level | ||
| 'blocks.*.parent_block' => 'prohibited_if:blocks.*.type,'.FormBlockType::group->value, |
There was a problem hiding this comment.
A child whose parent_block points at a question or at a missing id is never created, and the import still answers 200. A group with parent_block: "" passes the new check, but the group and its questions are never created, after the form's old questions are deleted. Allowing only top-level groups of the template as parents catches all three:
| 'blocks.*.parent_block' => 'prohibited_if:blocks.*.type,'.FormBlockType::group->value, | |
| 'blocks.*.parent_block' => [ | |
| 'nullable', | |
| 'prohibited_if:blocks.*.type,'.FormBlockType::group->value, | |
| Rule::in(collect($template['blocks'] ?? [])->where('type', FormBlockType::group->value)->whereNull('parent_block')->pluck('id')->filter()), | |
| ], |
I tried both suggestions together: the sequence, logic and template tests pass (69).
…level group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PhilReinking
left a comment
PhilReinking
left a comment
There was a problem hiding this comment.
OK at 063a9ae, CI green.
The strict check is better than my Rule::in suggestion, since it also catches "" on a group without children. A child listed before its group still imports, and a numeric parent_block now gets a 422 instead of being dropped. The 5 new test cases fail without the change.
1 participant
The API and template import now check what the editor already prevents: no group inside a group, and no group rule on a question inside it. Before, such questions disappeared from the public form, or the rule sent respondents back to the start.
Fixes BITS-179
Rule::in, becauseRule::inskips"", which the import also drops.Heads-up: a form that already has a group inside a group can't be reordered in the editor until that group is moved to the top level by API.
Tested: full PHP suite. The 15 new test cases fail without the fix. The existing sequence, logic and template tests pass unchanged.
Not tested: dragging blocks in the editor by hand.
🤖 Generated with Claude Code