fix: #3624690 Read a deadline setting by its own name, so the providers refuse what they cannot measure

AbsoluteDeadline and RelativeDeadline read their fields as getValue($form['until']['#parents']). Their only host is the timeout editor, which hands each provider a SubformState, and a subform state's values are already the slice under that subform: asking it for the element's absolute path looked that whole path up inside the slice, found nothing, and read every value as empty. Empty is the one value both refusals accept, so Enter a duration and Enter an absolute date never fired on any real form. An author could save not a duration as a timeout, and the task then parked with no deadline at run time, which the troubleshooting page describes as arriving by configuration import - the one door it was meant to arrive through.

TimeoutJoin::validateConfigurationForm() states the rule two directories over: a subform state's getValue() is relative to the subform, so the key is the field's own name. Both providers now read theirs that way.

The existing NodeFeatureEditorTest::testRelativeDeadlineRejectsInvalidDuration could not see it, because it asked the provider directly with a plain FormState whose #parents equalled the key, a shape no host produces. It is replaced by testTheEditorRefusesAnUnmeasurableDeadline, which saves a workflow through the modeler form the way the editor does and reads the errors off that, covering both providers and the absolute offset. Without the fix it reports no error at all.

Rebased on 1.x after !528 (merged) merged (clean, no overlap).

Audit rounds (5, closed on a clean round).

Round 1 asked why these two plugins invented this read when every other validator in the project uses the field's own name — all fifteen of them, and TimeoutJoin says why in a comment. The answer is that docs/extending.md documents only the setting half of the subform rule under "Refusing a setting", and its one line about #parents being safe to read there invites exactly this. Fixing only the two plugins would leave the doc teaching the bug to the next extender, so the reading half is now stated beside it — including why a test that builds its own FormState cannot see the difference, which is what let this survive. The two plugin comments shrink to a pointer, the way Groups.php already points.

Round 2 mutated the fix. Putting each of the three reads back to #parents turns the test red, one field at a time, so each assertion is load-bearing. Removing the variable-name accept on duration turns it red too, so the accept cases are not dead positive controls. Removing the same accept on until did not — that path had nothing holding it, and until this refusal fired at all nothing downstream of it could be wrong. A date naming a process variable is documented and accepted, so a lost accept is now an author who cannot save the workflow; it is asserted beside the duration case. Also dropped an assertion on the wording of an error message: the field key it lands on already identifies the refusal.

Rounds 3 to 5 found nothing. The sweep confirms no other instance: OrchestraCmForm is the only host that renders node feature editors, and the one other place reading #parents in a settings subform (PayableResolverConfigTrait) uses it for raw user input, an AJAX wrapper id and #limit_validation_errors, which all take the absolute path and are correct.

Note for review: a site that saved an unmeasurable value while this never refused anything will now be asked to correct it the next time that workflow is saved. That is the fix working — the value parked the task with no deadline — and the error lands on the field with the message saying what to enter.

Edited by Frank Mably

Merge request reports

Loading
Loading