From 3e44550e75600e82172acf3548d852643cf71330 Mon Sep 17 00:00:00 2001 From: Marco Sadowski Date: Wed, 3 Nov 2021 11:56:12 +0100 Subject: [PATCH 1/8] Improved MultiStep Validation Process Previously, an already validated step was not validated again when resending. This happens now. Co-Authored-By: arsors <22658305+arsors@users.noreply.github.com> --- Classes/Runtime/Domain/FormState.php | 9 ++++++++ .../MultiStepProcessImplementation.php | 23 +++++++++++++++---- .../Runtime/MultiStepProcess.fusion | 1 + 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Classes/Runtime/Domain/FormState.php b/Classes/Runtime/Domain/FormState.php index 6b5b7e0..f61e4b7 100644 --- a/Classes/Runtime/Domain/FormState.php +++ b/Classes/Runtime/Domain/FormState.php @@ -61,6 +61,15 @@ public function getPart(string $partName): ?array return $this->parts[$partName] ?? null; } + /** + * @param string $partName + * @return void + */ + public function removePart(string $partName): void + { + unset($this->parts[$partName]); + } + /** * @return string[] */ diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 2c5296c..0970ca7 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -51,6 +51,11 @@ class MultiStepProcessImplementation extends AbstractFusionObject implements Pro */ protected $targetSubProcessKey; + /** + * @var string + */ + protected $prevSubProcessKey; + /** * Return reference to self during fusion evaluation * @return $this @@ -91,6 +96,13 @@ public function handle(ActionRequest $request, array $data = []): void $this->currentSubProcessKey = (string)reset($subProcessKeys); } + // select previous subprocess key + if (array_key_exists('__prev', $internalArguments) + && $internalArguments['__prev'] + ) { + $this->prevSubProcessKey = (string)$internalArguments['__prev']; + } + // store target subprocess, but only if it already was submitted if (array_key_exists('__target', $internalArguments) && $internalArguments['__target'] @@ -113,11 +125,12 @@ public function handle(ActionRequest $request, array $data = []): void $this->currentSubProcessKey, $currentSubProcess->getData() ); - } else { - if ($this->targetSubProcessKey) { - $request->setArgument('__submittedArguments', []); - $request->setArgument('__submittedArgumentValidationResults', new Result()); - } + } elseif ( + $this->state + && $this->targetSubProcessKey !== $this->prevSubProcessKey + ) { + $this->state->removePart($this->currentSubProcessKey); + $this->targetSubProcessKey = (string)$internalArguments['__current']; } } diff --git a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion index 9d85dc0..3913cc8 100644 --- a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion +++ b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion @@ -14,6 +14,7 @@ prototype(Neos.Fusion.Form:Runtime.MultiStepProcess) { hiddenFields = afx` + ` prototype(Neos.Fusion.Form:Runtime.SingleStepProcess) { From 6f489bf6b8acb8bbb1ea6417084723d8aa440261 Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Fri, 10 Dec 2021 18:07:26 +0100 Subject: [PATCH 2/8] BUGFIX: Prevent committing values while navigating back in multi step forms. The __target argument that was used for the navigation of multistep forms did make no distinction between back and forth navigating which. This change alters the behavior and prevents committing when __target is prefixed with an exclamation mark. --- .../MultiStepProcessImplementation.php | 17 ++++++++++++++--- .../Prototypes/Runtime/MultiStepProcess.fusion | 2 +- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 97ff690..6c132bb 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -84,6 +84,7 @@ public function handle(ActionRequest $request, array $data = []): void // evaluate the subprocesses this has to be done after the state was restored // as the current data may affect @if conditions $subProcesses = $this->getSubProcesses(); + $resultCommittingIsAllowed = true; // select current subprocess if (array_key_exists('__current', $internalArguments) @@ -99,16 +100,26 @@ public function handle(ActionRequest $request, array $data = []): void if (array_key_exists('__target', $internalArguments) && $internalArguments['__target'] && $this->state - && $this->state->hasPart($internalArguments['__target']) ) { - $this->targetSubProcessKey = (string)$internalArguments['__target']; + $target = (string)$internalArguments['__target']; + if (substr($target, 0, 1) === '!') { + $target = substr($target, 1); + if ($this->state->hasPart($target)) { + $this->targetSubProcessKey = $target; + $resultCommittingIsAllowed = false; + } + } else { + if ($this->state->hasPart($target)) { + $this->targetSubProcessKey = $target; + } + } } // find current and handle $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; $currentSubProcess->handle($request, $this->data); - if ($currentSubProcess->isFinished()) { + if ($currentSubProcess->isFinished() && $resultCommittingIsAllowed) { if (!$this->state) { $this->state = new FormState(); } diff --git a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion index 9d85dc0..9634553 100644 --- a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion +++ b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion @@ -6,7 +6,7 @@ prototype(Neos.Fusion.Form:Runtime.MultiStepProcess) { header = null footer = afx` - {Translation.id('forms.navigation.previousPage').package('Neos.Fusion.Form').translate()} + {Translation.id('forms.navigation.previousPage').package('Neos.Fusion.Form').translate()} {Translation.id('forms.navigation.nextPage').package('Neos.Fusion.Form').translate()} {Translation.id('forms.navigation.submit').package('Neos.Fusion.Form').translate()} ` From e4ee2dc91359a76bbd68faa6ac1487f7592ba15d Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Fri, 10 Dec 2021 18:18:33 +0100 Subject: [PATCH 3/8] BUGFIX: Unset already committed steps when submitted a second time with invalid values Previously already committed steps were only replaced when a new valid state was pushed. This was unexpected as the user had changed the values and expects the new state to be submitted. With this change a step is overwritten when submitted again with valid data or removed otherwise. --- .../Runtime/FusionObjects/MultiStepProcessImplementation.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 6c132bb..c7e81a1 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -129,6 +129,9 @@ public function handle(ActionRequest $request, array $data = []): void $currentSubProcess->getData() ); } else { + if ($this->state && $resultCommittingIsAllowed && $this->state->hasPart($this->currentSubProcessKey)) { + $this->state->removePart($this->currentSubProcessKey); + } if ($this->targetSubProcessKey) { $request->setArgument('__submittedArguments', []); $request->setArgument('__submittedArgumentValidationResults', new Result()); From c0408aa8868ac0b19adab3c4475888dfeadba607 Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Fri, 21 Jan 2022 23:33:02 +0100 Subject: [PATCH 4/8] WIP: Refactor form state handling for more consitency --- Classes/Runtime/Domain/FormState.php | 81 ++++++---- Classes/Runtime/Domain/FormStatePart.php | 55 +++++++ Classes/Runtime/Domain/FormStateService.php | 2 +- .../MultiStepProcessImplementation.php | 148 ++++++++++-------- .../SingleStepProcessImplementation.php | 11 +- .../Runtime/MultiStepProcess.fusion | 4 +- 6 files changed, 202 insertions(+), 99 deletions(-) create mode 100644 Classes/Runtime/Domain/FormStatePart.php diff --git a/Classes/Runtime/Domain/FormState.php b/Classes/Runtime/Domain/FormState.php index f61e4b7..b7ad514 100644 --- a/Classes/Runtime/Domain/FormState.php +++ b/Classes/Runtime/Domain/FormState.php @@ -13,43 +13,38 @@ * source code. */ -class FormState +use Neos\Utility\Arrays; + +class FormState implements \JsonSerializable { /** - * @var mixed[][] + * @var FormStatePart[] */ protected $parts = []; - /** - * FormState constructor. - * @param mixed[][] $parts - */ - public function __construct(array $parts = []) - { - foreach ($parts as $key => $value) { - // ensure only strings are used as partNames - if (is_string($key)) { - $this->parts[$key] = $value; - } - } - } - /** * @param string $partName + * @param null|bool $finished * @return bool */ - public function hasPart(string $partName): bool + public function hasPart(string $partName, ?bool $finished = null): bool { - return array_key_exists($partName, $this->parts); + if (is_null($finished)) { + return array_key_exists($partName, $this->parts); + } else { + return array_key_exists($partName, $this->parts) && ($this->parts[$partName]->isFinished() === $finished); + } } /** * @param string $partName * @param mixed[] $partData + * @param bool $finished */ - public function commitPart(string $partName, array $partData = []): void + public function commitPart(string $partName, array $partData = [], bool $finished = false): void { - $this->parts[$partName] = $partData; + $part = new FormStatePart($partData, $finished); + $this->parts[$partName] = $part; } /** @@ -58,16 +53,10 @@ public function commitPart(string $partName, array $partData = []): void */ public function getPart(string $partName): ?array { - return $this->parts[$partName] ?? null; - } - - /** - * @param string $partName - * @return void - */ - public function removePart(string $partName): void - { - unset($this->parts[$partName]); + if (array_key_exists($partName, $this->parts)) { + return $this->parts[$partName]->getData(); + } + return null; } /** @@ -86,10 +75,38 @@ function ($item) { } /** - * @return mixed[][] + * @param boolean $finished + * @return FormStatePart[] */ - public function getAllParts(): array + public function getParts(): array { return $this->parts; } + + /** + * @return mixed[] + */ + public function getData(): array + { + $data = []; + foreach ($this->parts as $part) { + $data = Arrays::arrayMergeRecursiveOverrule( + $data, + $part->getData() + ); + } + return $data; + } + + public function jsonSerialize() + { + $json = []; + foreach ($this->parts as $name => $part) { + $json[$name] = [ + 'data' => $part->getData(), + 'finished' => $part->isFinished() + ]; + } + return $json; + } } diff --git a/Classes/Runtime/Domain/FormStatePart.php b/Classes/Runtime/Domain/FormStatePart.php new file mode 100644 index 0000000..fe740a6 --- /dev/null +++ b/Classes/Runtime/Domain/FormStatePart.php @@ -0,0 +1,55 @@ +data = $data; + $this->finished = $finished; + } + + /** + * @return mixed[] + */ + public function getData(): array + { + return $this->data; + } + + /** + * @return bool + */ + public function isFinished(): bool + { + return $this->finished; + } +} diff --git a/Classes/Runtime/Domain/FormStateService.php b/Classes/Runtime/Domain/FormStateService.php index fbce7af..482142d 100644 --- a/Classes/Runtime/Domain/FormStateService.php +++ b/Classes/Runtime/Domain/FormStateService.php @@ -33,7 +33,7 @@ class FormStateService public function unserializeState(string $string): FormState { $validatedState = $this->hashService->validateAndStripHmac($string); - return unserialize(base64_decode($validatedState), ['allowed_classes' => [FormState::class]]); + return unserialize(base64_decode($validatedState), ['allowed_classes' => [FormState::class, FormStatePart::class]]); } /** diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index c7e81a1..120d717 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -22,6 +22,7 @@ use Neos\Fusion\Form\Runtime\Domain\ProcessCollectionInterface; use Neos\Fusion\FusionObjects\AbstractFusionObject; use Neos\Utility\Arrays; +use Psr\Log\LoggerInterface; class MultiStepProcessImplementation extends AbstractFusionObject implements ProcessInterface { @@ -51,6 +52,18 @@ class MultiStepProcessImplementation extends AbstractFusionObject implements Pro */ protected $targetSubProcessKey; + /** + * @var bool + */ + protected $attemptFinishing = false; + + /** + * @Flow\Inject + * @var LoggerInterface + */ + protected $logger; + + /** * Return reference to self during fusion evaluation * @return $this @@ -69,12 +82,14 @@ public function handle(ActionRequest $request, array $data = []): void $this->data = $data; $internalArguments = $request->getInternalArguments(); + $stateArgument = $internalArguments['__state'] ?? null; + $currentStepArgument = $internalArguments['__current'] ?? null; + $targetStepArgument = $internalArguments['__target'] ?? null; + $finishProcessArgument = $internalArguments['__finish'] ?? null; // restore state - if (array_key_exists('__state', $internalArguments) - && $internalArguments['__state'] - ) { - $this->state = $this->formStateService->unserializeState($internalArguments['__state']); + if ($stateArgument) { + $this->state = $this->formStateService->unserializeState($stateArgument); } // make the current `data` available to the context before sub processes are evaluated @@ -84,30 +99,44 @@ public function handle(ActionRequest $request, array $data = []): void // evaluate the subprocesses this has to be done after the state was restored // as the current data may affect @if conditions $subProcesses = $this->getSubProcesses(); - $resultCommittingIsAllowed = true; + $subProcessKeys = array_keys($subProcesses); + $firstSubProcessKey = (string)reset($subProcessKeys); - // select current subprocess - if (array_key_exists('__current', $internalArguments) - && $internalArguments['__current'] - ) { - $this->currentSubProcessKey = (string)$internalArguments['__current']; + // do not commit when the target was prepended with exclamation mark + if ($targetStepArgument && (substr($targetStepArgument, 0, 1) === '!')) { + $resultCommittingIsAllowed = false; } else { - $subProcessKeys = array_keys($subProcesses); - $this->currentSubProcessKey = (string)reset($subProcessKeys); + $resultCommittingIsAllowed = true; } - // store target subprocess, but only if it already was submitted - if (array_key_exists('__target', $internalArguments) - && $internalArguments['__target'] - && $this->state - ) { - $target = (string)$internalArguments['__target']; - if (substr($target, 0, 1) === '!') { - $target = substr($target, 1); - if ($this->state->hasPart($target)) { - $this->targetSubProcessKey = $target; - $resultCommittingIsAllowed = false; + // find current subprocess and handle the request + if ($currentStepArgument) { + $this->currentSubProcessKey = $currentStepArgument; + $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; + $currentSubProcess->handle($request, $this->data); + if ($currentStepArgument && $resultCommittingIsAllowed) { + if (!$this->state) { + $this->state = new FormState(); } + $this->state->commitPart( + $this->currentSubProcessKey, + $currentSubProcess->getData(), + $currentSubProcess->isFinished() + ); + } + } else { + $this->currentSubProcessKey = $firstSubProcessKey; + } + + // find target subprocess, but only if it already was submitted + if ($targetStepArgument && $this->state) { + if (substr($targetStepArgument, 0, 1) === '!') { + $target = substr($targetStepArgument, 1); + } else { + $target = $targetStepArgument; + } + if ($currentSubProcess->isFinished()) { + $this->targetSubProcessKey = $target; } else { if ($this->state->hasPart($target)) { $this->targetSubProcessKey = $target; @@ -115,27 +144,38 @@ public function handle(ActionRequest $request, array $data = []): void } } - // find current and handle - $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; - $currentSubProcess->handle($request, $this->data); + // ensure no unfinished subprocesses are before the target + if ($this->state && $this->targetSubProcessKey) { + foreach ($subProcesses as $subProcessKey => $subProcesses) { + if ($subProcessKey === $this->targetSubProcessKey) { + break; + } + if ($this->state->hasPart($subProcessKey, false)) { + $this->targetSubProcessKey = $subProcessKey; + } + if ($subProcessKey === $this->currentSubProcessKey) { + break; + } + } + } - if ($currentSubProcess->isFinished() && $resultCommittingIsAllowed) { + // determine target if none was defined yet + if (!$this->targetSubProcessKey) { if (!$this->state) { - $this->state = new FormState(); + $this->targetSubProcessKey = $firstSubProcessKey; + } else { + foreach ($subProcesses as $subProcessKey => $subProcess) { + if ($this->state->hasPart($subProcessKey, false)) { + $this->targetSubProcessKey = $subProcessKey; + break; + } + } } + } - $this->state->commitPart( - $this->currentSubProcessKey, - $currentSubProcess->getData() - ); - } else { - if ($this->state && $resultCommittingIsAllowed && $this->state->hasPart($this->currentSubProcessKey)) { - $this->state->removePart($this->currentSubProcessKey); - } - if ($this->targetSubProcessKey) { - $request->setArgument('__submittedArguments', []); - $request->setArgument('__submittedArgumentValidationResults', new Result()); - } + // is this an attempt to finish the process + if ($finishProcessArgument) { + $this->attemptFinishing = true; } // restore fusion context to the state before data was pushed @@ -151,14 +191,13 @@ public function isFinished(): bool return false; } - if ($this->targetSubProcessKey) { + if (!$this->attemptFinishing) { return false; } $subProcesses = $this->getSubProcesses(); - foreach ($subProcesses as $subProcessKey => $subProcess) { - if ($this->state->hasPart($subProcessKey) == false) { + if ($this->state->hasPart($subProcessKey, true) == false) { return false; } } @@ -171,22 +210,12 @@ public function isFinished(): bool public function render(): string { $subProcesses = $this->getSubProcesses(); - if ($this->targetSubProcessKey) { - $renderSubProcessKey = $this->targetSubProcessKey; - } else { - foreach ($subProcesses as $subProcessKey => $subProcess) { - if (!$this->state || !$this->state->hasPart($subProcessKey)) { - $renderSubProcessKey = $subProcessKey; - break; - } - } - } - if (isset($renderSubProcessKey) && $renderSubProcessKey && array_key_exists($renderSubProcessKey, $subProcesses)) { - $this->getRuntime()->pushContext('process', $this->prepareProcessInformation($renderSubProcessKey, $subProcesses)); + if (isset($this->targetSubProcessKey) && $this->targetSubProcessKey && array_key_exists($this->targetSubProcessKey, $subProcesses)) { + $this->getRuntime()->pushContext('process', $this->prepareProcessInformation($this->targetSubProcessKey, $subProcesses)); $hiddenFields = $this->runtime->evaluate($this->path . '/hiddenFields') ?? ''; $header = $this->runtime->evaluate($this->path . '/header') ?? ''; - $body = $subProcesses[$renderSubProcessKey]->render(); + $body = $subProcesses[$this->targetSubProcessKey]->render(); $footer = $this->runtime->evaluate($this->path . '/footer') ?? ''; $this->getRuntime()->popContext(); return $hiddenFields . $header . $body . $footer; @@ -205,12 +234,7 @@ public function getData(): array // add subprocess data from state if ($this->state) { - foreach ($this->state->getAllParts() as $subProcessKey => $subProcessData) { - $data = Arrays::arrayMergeRecursiveOverrule( - $data, - $subProcessData - ); - } + $data = Arrays::arrayMergeRecursiveOverrule($data, $this->state->getData()); } return $data; diff --git a/Classes/Runtime/FusionObjects/SingleStepProcessImplementation.php b/Classes/Runtime/FusionObjects/SingleStepProcessImplementation.php index 1ef7448..9c063ee 100644 --- a/Classes/Runtime/FusionObjects/SingleStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/SingleStepProcessImplementation.php @@ -21,6 +21,7 @@ use Neos\Fusion\Form\Runtime\Domain\ProcessInterface; use Neos\Fusion\Form\Runtime\Domain\SchemaInterface; use Neos\Fusion\FusionObjects\AbstractFusionObject; +use Neos\Utility\Arrays; class SingleStepProcessImplementation extends AbstractFusionObject implements ProcessInterface { @@ -62,9 +63,15 @@ public function handle(ActionRequest $request, array $data = []): void if ($arguments || $internalArguments) { $data = $schema->convert($arguments); $result = $schema->validate($data); - $request->setArgument('__submittedArguments', $data); + $request->setArgument('__submittedArguments', $arguments); $request->setArgument('__submittedArgumentValidationResults', $result); - if (!$result->hasErrors()) { + if ($result->hasErrors()) { + foreach ($result->getFlattenedErrors() as $path => $error) { + $data = Arrays::unsetValueByPath($data, $path); + } + $this->data = $data; + $this->isFinished = false; + } else { $this->data = $data; $this->isFinished = true; } diff --git a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion index 9634553..8200813 100644 --- a/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion +++ b/Resources/Private/Fusion/Prototypes/Runtime/MultiStepProcess.fusion @@ -6,9 +6,9 @@ prototype(Neos.Fusion.Form:Runtime.MultiStepProcess) { header = null footer = afx` - {Translation.id('forms.navigation.previousPage').package('Neos.Fusion.Form').translate()} + {Translation.id('forms.navigation.previousPage').package('Neos.Fusion.Form').translate()} {Translation.id('forms.navigation.nextPage').package('Neos.Fusion.Form').translate()} - {Translation.id('forms.navigation.submit').package('Neos.Fusion.Form').translate()} + {Translation.id('forms.navigation.submit').package('Neos.Fusion.Form').translate()} ` hiddenFields = afx` From 4b61977da105ba8d679409a30d1958c28f32e34e Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Sun, 23 Jan 2022 11:56:36 +0100 Subject: [PATCH 5/8] Fix tests --- Classes/Runtime/Domain/FormState.php | 33 ++++++---- .../MultiStepProcessImplementation.php | 9 +-- Tests/Unit/FormStateServiceTest.php | 14 ++-- Tests/Unit/FormStateTest.php | 66 +++++++++++-------- 4 files changed, 72 insertions(+), 50 deletions(-) diff --git a/Classes/Runtime/Domain/FormState.php b/Classes/Runtime/Domain/FormState.php index b7ad514..559ddef 100644 --- a/Classes/Runtime/Domain/FormState.php +++ b/Classes/Runtime/Domain/FormState.php @@ -29,11 +29,16 @@ class FormState implements \JsonSerializable */ public function hasPart(string $partName, ?bool $finished = null): bool { - if (is_null($finished)) { - return array_key_exists($partName, $this->parts); - } else { - return array_key_exists($partName, $this->parts) && ($this->parts[$partName]->isFinished() === $finished); - } + return array_key_exists($partName, $this->parts); + } + + /** + * @param string $partName + * @return bool + */ + public function isPartFinished(string $partName): bool + { + return array_key_exists($partName, $this->parts) && ($this->parts[$partName]->isFinished()); } /** @@ -52,6 +57,15 @@ public function commitPart(string $partName, array $partData = [], bool $finishe * @return mixed[]|null */ public function getPart(string $partName): ?array + { + return $this->getPartData($partName); + } + + /** + * @param string $partName + * @return mixed[]|null + */ + public function getPartData(string $partName): ?array { if (array_key_exists($partName, $this->parts)) { return $this->parts[$partName]->getData(); @@ -74,15 +88,6 @@ function ($item) { ); } - /** - * @param boolean $finished - * @return FormStatePart[] - */ - public function getParts(): array - { - return $this->parts; - } - /** * @return mixed[] */ diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 120d717..60b5607 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -114,7 +114,7 @@ public function handle(ActionRequest $request, array $data = []): void $this->currentSubProcessKey = $currentStepArgument; $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; $currentSubProcess->handle($request, $this->data); - if ($currentStepArgument && $resultCommittingIsAllowed) { + if ($resultCommittingIsAllowed) { if (!$this->state) { $this->state = new FormState(); } @@ -126,6 +126,7 @@ public function handle(ActionRequest $request, array $data = []): void } } else { $this->currentSubProcessKey = $firstSubProcessKey; + $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; } // find target subprocess, but only if it already was submitted @@ -150,7 +151,7 @@ public function handle(ActionRequest $request, array $data = []): void if ($subProcessKey === $this->targetSubProcessKey) { break; } - if ($this->state->hasPart($subProcessKey, false)) { + if (!$this->state->isPartFinished($subProcessKey)) { $this->targetSubProcessKey = $subProcessKey; } if ($subProcessKey === $this->currentSubProcessKey) { @@ -165,7 +166,7 @@ public function handle(ActionRequest $request, array $data = []): void $this->targetSubProcessKey = $firstSubProcessKey; } else { foreach ($subProcesses as $subProcessKey => $subProcess) { - if ($this->state->hasPart($subProcessKey, false)) { + if (!$this->state->isPartFinished($subProcessKey)) { $this->targetSubProcessKey = $subProcessKey; break; } @@ -197,7 +198,7 @@ public function isFinished(): bool $subProcesses = $this->getSubProcesses(); foreach ($subProcesses as $subProcessKey => $subProcess) { - if ($this->state->hasPart($subProcessKey, true) == false) { + if ($this->state->isPartFinished($subProcessKey) == false) { return false; } } diff --git a/Tests/Unit/FormStateServiceTest.php b/Tests/Unit/FormStateServiceTest.php index ae83048..644e2c0 100644 --- a/Tests/Unit/FormStateServiceTest.php +++ b/Tests/Unit/FormStateServiceTest.php @@ -47,11 +47,9 @@ public function setUp(): void */ public function formStateCanBeSerializedAndUnserialized() { - $stateParts = [ - 'first' => ['value1' => 'foo', 'value2' => 'bar'], - 'second' => ['value2' => 'foo', 'value4' => 'bar'] - ]; - $state = new FormState($stateParts); + $state = new FormState(); + $state->commitPart('first', ['value1' => 'foo', 'value2' => 'bar'], true); + $state->commitPart('second', ['value2' => 'foo', 'value4' => 'bar'], false); $statePartsSerialized = base64_encode(serialize($state)); @@ -68,7 +66,11 @@ public function formStateCanBeSerializedAndUnserialized() $serializedState = $this->formStateService->serializeState($state); $unserializedState = $this->formStateService->unserializeState($serializedState); - $this->assertEquals($stateParts, $unserializedState->getAllParts()); + $this->assertEquals(['first', 'second'], $unserializedState->getCommittedPartNames()); + $this->assertTrue($unserializedState->isPartFinished('first')); + $this->assertFalse($unserializedState->isPartFinished('second')); + $this->assertEquals(['value1' => 'foo', 'value2' => 'bar'], $unserializedState->getPartData('first')); + $this->assertEquals(['value2' => 'foo', 'value4' => 'bar'], $unserializedState->getPartData('second')); } /** diff --git a/Tests/Unit/FormStateTest.php b/Tests/Unit/FormStateTest.php index 4a5eb0a..5055185 100644 --- a/Tests/Unit/FormStateTest.php +++ b/Tests/Unit/FormStateTest.php @@ -34,48 +34,57 @@ public function setUp(): void public function emptyStateHasNoParts() { $state = new FormState(); - $this->assertEmpty($state->getAllParts()); $this->assertFalse($state->hasPart('example')); - $this->assertNull($state->getPart('example')); + $this->assertNull($state->getPartData('example')); $this->assertEquals([], $state->getCommittedPartNames()); } + public function partsCanBeAcessedAfterBeingComittedDataProvider(): array + { + return [ + ['example1', ['value1' => 'exampleValue1'], true], + ['example2', ['value2' => 'exampleValue2'], false], + ]; + } + /** * @test + * @dataProvider partsCanBeAcessedAfterBeingComittedDataProvider */ - public function partsCanBeAcessedAfterBeingComitted() + public function partsCanBeAcessedAfterBeingComitted($name, $data, $finished) { $state = new FormState(); - $state->commitPart('example', ['value' => 'exampleValue']); - $this->assertTrue($state->hasPart('example')); - $this->assertEquals(['value' => 'exampleValue'], $state->getPart('example')); - $this->assertEquals(['example' => ['value' => 'exampleValue']], $state->getAllParts()); - $this->assertEquals(['example'], $state->getCommittedPartNames()); + $state->commitPart($name, $data, $finished); + $this->assertTrue($state->hasPart($name)); + $this->assertEquals($data, $state->getPartData($name)); + $this->assertEquals($finished, $state->isPartFinished($name)); + $this->assertEquals([$name], $state->getCommittedPartNames()); } - /** - * @test - */ - public function initialPartsCanBeAcessed() + public function comittedPartsOverwriteExistingPartsWithSameNameDataProvider(): array { - $state = new FormState(['example' => ['value' => 'exampleValue']]); - $this->assertTrue($state->hasPart('example')); - $this->assertEquals(['value' => 'exampleValue'], $state->getPart('example')); - $this->assertEquals(['example' => ['value' => 'exampleValue']], $state->getAllParts()); - $this->assertEquals(['example'], $state->getCommittedPartNames()); + return [ + [['value1' => 'exampleValue1'], true, ['value1' => 'exampleValue1'], false], + [['value2' => 'exampleValue2'], false, ['value2' => 'exampleValue2'], true ], + [['value1' => 'exampleValue2'], true, [], false ], + [[], false, ['value1' => 'exampleValue2'], true ], + ]; } + /** * @test + * @dataProvider comittedPartsOverwriteExistingPartsWithSameNameDataProvider */ - public function comittedPartsOverwriteExistingPartsWithSameName() + public function comittedPartsOverwriteExistingPartsWithSameName($dataOriginal, $finishedOriginal, $dataAfter, $finishedAfter) { - $state = new FormState(['example' => ['value' => 'exampleValue']]); - $state->commitPart('example', ['another' => 'exampleValue']); + $state = new FormState(); + $state->commitPart('example', $dataOriginal, $finishedOriginal); + $state->commitPart('example', $dataAfter, $finishedAfter); + $this->assertTrue($state->hasPart('example')); - $this->assertEquals(['another' => 'exampleValue'], $state->getPart('example')); - $this->assertEquals(['example' => ['another' => 'exampleValue']], $state->getAllParts()); - $this->assertEquals(['example'], $state->getCommittedPartNames()); + $this->assertEquals($dataAfter, $state->getPartData('example')); + $this->assertEquals($finishedAfter, $state->isPartFinished('example')); } /** @@ -83,12 +92,17 @@ public function comittedPartsOverwriteExistingPartsWithSameName() */ public function comittedPartsAreAddedIfNamesAreDifferent() { - $state = new FormState(['example1' => ['value' => 'exampleValue']]); + $state = new FormState(); + $state->commitPart('example1', ['value' => 'exampleValue']); + $this->assertEquals(['example1'], $state->getCommittedPartNames()); + $this->assertTrue($state->hasPart('example1')); + $this->assertFalse($state->hasPart('example2')); + $state->commitPart('example2', ['another' => 'exampleValue']); $this->assertTrue($state->hasPart('example1')); $this->assertTrue($state->hasPart('example2')); - $this->assertEquals(['value' => 'exampleValue'], $state->getPart('example1')); - $this->assertEquals(['another' => 'exampleValue'], $state->getPart('example2')); + $this->assertEquals(['value' => 'exampleValue'], $state->getPartData('example1')); + $this->assertEquals(['another' => 'exampleValue'], $state->getPartData('example2')); $this->assertEquals(['example1', 'example2'], $state->getCommittedPartNames()); } } From cf9afe632d9c39950dcf09bf9d52ac36b91f454a Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Mon, 2 May 2022 12:34:02 +0200 Subject: [PATCH 6/8] TASK: Add example for confirmation checkbox during validation --- Documentation/Examples/MultiStepForm.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/Examples/MultiStepForm.md b/Documentation/Examples/MultiStepForm.md index 6dc8e43..a57b987 100644 --- a/Documentation/Examples/MultiStepForm.md +++ b/Documentation/Examples/MultiStepForm.md @@ -65,7 +65,13 @@ prototype(Vendor.Site:Content.MultiStepFormExample) < prototype(Neos.Fusion.Form confirmation { content = afx`

Confirm to submit {data.firstName} {first.data.lastName} from {data.city}, {data.street}

+ + Agree to data storage + ` + schema { + gdprAgreed = ${Form.Schema.boolean().validator('BooleanValue', {expectedValue:true})} + } } } } From 27fb460265bba3cc00ac58157576666fd307d809 Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Mon, 2 May 2022 13:41:50 +0200 Subject: [PATCH 7/8] TASK: Remove logger that was just added for debugging --- .../FusionObjects/MultiStepProcessImplementation.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 60b5607..1aa6690 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -22,7 +22,6 @@ use Neos\Fusion\Form\Runtime\Domain\ProcessCollectionInterface; use Neos\Fusion\FusionObjects\AbstractFusionObject; use Neos\Utility\Arrays; -use Psr\Log\LoggerInterface; class MultiStepProcessImplementation extends AbstractFusionObject implements ProcessInterface { @@ -57,13 +56,6 @@ class MultiStepProcessImplementation extends AbstractFusionObject implements Pro */ protected $attemptFinishing = false; - /** - * @Flow\Inject - * @var LoggerInterface - */ - protected $logger; - - /** * Return reference to self during fusion evaluation * @return $this From 4a1b56099305a2282533e2f7d038e83dc9d17c06 Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Mon, 2 May 2022 15:29:11 +0200 Subject: [PATCH 8/8] TASK: Remove non committing logic with exclamation mark --- .../MultiStepProcessImplementation.php | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php index 1aa6690..8ca93a7 100644 --- a/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php +++ b/Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php @@ -75,9 +75,9 @@ public function handle(ActionRequest $request, array $data = []): void $internalArguments = $request->getInternalArguments(); $stateArgument = $internalArguments['__state'] ?? null; - $currentStepArgument = $internalArguments['__current'] ?? null; - $targetStepArgument = $internalArguments['__target'] ?? null; - $finishProcessArgument = $internalArguments['__finish'] ?? null; + $currentStep = $internalArguments['__current'] ?? null; + $targetStep = $internalArguments['__target'] ?? null; + $finishProcess = $internalArguments['__finish'] ?? null; // restore state if ($stateArgument) { @@ -94,45 +94,32 @@ public function handle(ActionRequest $request, array $data = []): void $subProcessKeys = array_keys($subProcesses); $firstSubProcessKey = (string)reset($subProcessKeys); - // do not commit when the target was prepended with exclamation mark - if ($targetStepArgument && (substr($targetStepArgument, 0, 1) === '!')) { - $resultCommittingIsAllowed = false; - } else { - $resultCommittingIsAllowed = true; - } // find current subprocess and handle the request - if ($currentStepArgument) { - $this->currentSubProcessKey = $currentStepArgument; + if ($currentStep) { + $this->currentSubProcessKey = $currentStep; $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; $currentSubProcess->handle($request, $this->data); - if ($resultCommittingIsAllowed) { - if (!$this->state) { - $this->state = new FormState(); - } - $this->state->commitPart( - $this->currentSubProcessKey, - $currentSubProcess->getData(), - $currentSubProcess->isFinished() - ); + if (!$this->state) { + $this->state = new FormState(); } + $this->state->commitPart( + $this->currentSubProcessKey, + $currentSubProcess->getData(), + $currentSubProcess->isFinished() + ); } else { $this->currentSubProcessKey = $firstSubProcessKey; $currentSubProcess = $subProcesses[$this->currentSubProcessKey]; } // find target subprocess, but only if it already was submitted - if ($targetStepArgument && $this->state) { - if (substr($targetStepArgument, 0, 1) === '!') { - $target = substr($targetStepArgument, 1); - } else { - $target = $targetStepArgument; - } + if ($targetStep && $this->state) { if ($currentSubProcess->isFinished()) { - $this->targetSubProcessKey = $target; + $this->targetSubProcessKey = $targetStep; } else { - if ($this->state->hasPart($target)) { - $this->targetSubProcessKey = $target; + if ($this->state->hasPart($targetStep)) { + $this->targetSubProcessKey = $targetStep; } } } @@ -167,7 +154,7 @@ public function handle(ActionRequest $request, array $data = []): void } // is this an attempt to finish the process - if ($finishProcessArgument) { + if ($finishProcess) { $this->attemptFinishing = true; }