State transition validation should only happen after a transition is applied.

Created on 23 February 2023, over 1 year ago
Updated 24 February 2023, over 1 year ago

Problem/Motivation

Currently, StateItem::isValid(); (which is the method invoked on field validation), gets the allowed states based on the "previous state", and return FALSE if the current state is not allowed.

The main problem is, this is happening even when the state didn't change.

This is the current code:

    $allowed_states = $this->getAllowedStates($this->originalValue);
    return isset($allowed_states[$this->value]);

And this is the getAllowedStates() method:


  protected function getAllowedStates($value) {
    $workflow = $this->getWorkflow();
    if (!$workflow) {
      // The workflow is not known yet, the field is probably being created.
      return [];
    }
    $allowed_states = [];
    if (!empty($value) && ($current_state = $workflow->getState($value))) {
      $allowed_states[$value] = $current_state;
    }

    $transitions = $workflow->getAllowedTransitions($value, $this->getEntity());
    foreach ($transitions as $transition) {
      $state = $transition->getToState();
      $allowed_states[$state->getId()] = $state;
    }

    return $allowed_states;
  }

So this will loop over all the allowed transitions based on the "previous state", even if the state didn't actually change.
This is wrong for 2 reasons:

  1. We don't check whether the state changed
  2. We're checking ALL allowed transitions, rather than checking that the transition that was applied was actually allowed.

So because of 2), the transition guards are going to be invoked for transitions that were never intended to be applied... which is wrong to me, IMO we should only check that the particular transition that was applied is allowed.

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇮🇱Israel jsacksick

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024