Issue #2919482: Clean code - better naming of $entity_type

src/Controller/WorkflowTransitionListController.php

@@ -128,8 +128,8 @@ class WorkflowTransitionListController extends EntityListController implements C
     if (!is_object($view)) {
       // @deprecated. Use the Views display above.
       // Add the history list from programmed WorkflowTransitionListController.
-      $entity_type = 'workflow_transition';
-      $list_builder = $this->entityTypeManager()->getListBuilder($entity_type);
+      $entity_type_id = 'workflow_transition';
+      $list_builder = $this->entityTypeManager()->getListBuilder($entity_type_id);
       // Add the Node explicitly, since $list_builder expects a Transition.
       $list_builder->setTargetEntity($entity);
       $form += $list_builder->render();
@@ -184,20 +184,20 @@ class WorkflowTransitionListController extends EntityListController implements C
   public static function addRevertOperation(WorkflowTransitionInterface $transition) {
     $operations = [];
 
-    $entity_type = $transition->getTargetEntityTypeId();
+    $entity_type_id = $transition->getTargetEntityTypeId();
     $entity_id = $transition->getTargetEntityId();
     $field_name = $transition->getFieldName();
 
     // Only add 'revert' to the first row. Skip all following records.
     static $first;
-    if (!($first[$entity_type][$entity_id][$field_name] ?? TRUE)) {
+    if (!($first[$entity_type_id][$entity_id][$field_name] ?? TRUE)) {
       return $operations;
     }
 
     if (!$transition->isRevertable()) {
       // Some states are not fit to revert to.
       // In each of these cases, prohibit to revert to an even older state.
-      $first[$entity_type][$entity_id][$field_name] = FALSE;
+      $first[$entity_type_id][$entity_id][$field_name] = FALSE;
       return $operations;
     }
 
@@ -217,7 +217,7 @@ class WorkflowTransitionListController extends EntityListController implements C
       ];
 
       // No need to read the following records.
-      $first[$entity_type][$entity_id][$field_name] = FALSE;
+      $first[$entity_type_id][$entity_id][$field_name] = FALSE;
       return $operations;
     }
 

src/Element/WorkflowTransitionTimestamp.php

@@ -45,8 +45,7 @@ class WorkflowTransitionTimestamp extends FormElement {
     if (!$input) {
       // Massage, normalize value after pressing Form button.
       // $element is also updated via reference.
-      $value = $element['#default_value'];
-      return $value;
+      return $timestamp;
     }
 
     // Fetch $timestamp from widget for scheduled transitions.

src/Entity/WorkflowManager.php

@@ -152,9 +152,9 @@ class WorkflowManager implements WorkflowManagerInterface {
     $user = workflow_current_user();
     $field_names = workflow_get_workflow_field_names($entity);
     foreach ($field_names as $field_name) {
-      // Transition is created in widget or WorkflowTransitionForm.
       /** @var \Drupal\workflow\Entity\WorkflowTransitionInterface $transition */
-      $transition = $entity->{$field_name}->__get('workflow_transition');
+      // @todo Transition is created in widget or WorkflowTransitionForm.
+      $transition = $entity->{$field_name}->__get('_workflow_transition') ?? NULL;  // @todo #default_value.
       if (!$transition) {
         // We come from creating/editing an entity via entity_form, with core widget or hidden Workflow widget.
         // @todo D8: From an Edit form with hidden widget.
@@ -221,22 +221,22 @@ class WorkflowManager implements WorkflowManagerInterface {
 
   }
 
-    /**
+  /**
    * {@inheritdoc}
    */
   public static function deleteTransitionsOfEntity(EntityInterface $entity, $transition_type, $field_name, $langcode = '') {
-    $entity_type = $entity->getEntityTypeId();
+    $entity_type_id = $entity->getEntityTypeId();
     $entity_id = $entity->id();
 
     switch ($transition_type) {
       case 'workflow_transition':
-        foreach (WorkflowTransition::loadMultipleByProperties($entity_type, [$entity_id], [], $field_name, $langcode, NULL, 'ASC', $transition_type) as $transition) {
+        foreach (WorkflowTransition::loadMultipleByProperties($entity_type_id, [$entity_id], [], $field_name, $langcode, NULL, 'ASC', $transition_type) as $transition) {
           $transition->delete();
         }
         break;
 
-        case 'workflow_scheduled_transition':
-        foreach (WorkflowScheduledTransition::loadMultipleByProperties($entity_type, [$entity_id], [], $field_name, $langcode, NULL, 'ASC', $transition_type) as $transition) {
+      case 'workflow_scheduled_transition':
+        foreach (WorkflowScheduledTransition::loadMultipleByProperties($entity_type_id, [$entity_id], [], $field_name, $langcode, NULL, 'ASC', $transition_type) as $transition) {
           $transition->delete();
         }
         break;
@@ -338,8 +338,8 @@ class WorkflowManager implements WorkflowManagerInterface {
     // @todo Read history with an explicit langcode(?).
     $langcode = ''; // $entity->language()->getId();
     // @todo D8: #2373383 Add integration with older revisions via Revisioning module.
-    $entity_type = $entity->getEntityTypeId();
-    $last_transition = WorkflowTransition::loadByProperties($entity_type, $entity->id(), [], $field_name, $langcode, 'DESC');
+    $entity_type_id = $entity->getEntityTypeId();
+    $last_transition = WorkflowTransition::loadByProperties($entity_type_id, $entity->id(), [], $field_name, $langcode, 'DESC');
     if ($last_transition) {
       $sid = $last_transition->getToSid(); // @see #2637092, #2612702
     }
@@ -365,14 +365,14 @@ class WorkflowManager implements WorkflowManagerInterface {
         // @todo Test; use deleteTransitionsOfEntity().
         $field_config = $entity;
         /** @var \Drupal\Core\Entity\ContentEntityBase $field_config */
-        $entity_type = (string) $field_config->get('entity_type');
+        $entity_type_id = (string) $field_config->get('entity_type');
         $field_name = (string) $field_config->get('field_name');
         /** @var \Drupal\workflow\Entity\WorkflowTransitionInterface $transition */
-        foreach (WorkflowScheduledTransition::loadMultipleByProperties($entity_type, [], [], $field_name) as $transition) {
+        foreach (WorkflowScheduledTransition::loadMultipleByProperties($entity_type_id, [], [], $field_name) as $transition) {
           $transition->delete();
         }
         WorkflowManager::deleteTransitionsOfEntity($entity, 'workflow_transition', $field_name);
-        foreach (WorkflowTransition::loadMultipleByProperties($entity_type, [], [], $field_name) as $transition) {
+        foreach (WorkflowTransition::loadMultipleByProperties($entity_type_id, [], [], $field_name) as $transition) {
           $transition->delete();
         }
         break;
@@ -455,11 +455,11 @@ class WorkflowManager implements WorkflowManagerInterface {
    * {@inheritdoc}
    */
   public static function getDefaultTransition(EntityInterface $entity, $field_name): WorkflowTransitionInterface {
-    $entity_type = $entity->getEntityTypeId();
+    $entity_type_id = $entity->getEntityTypeId();
     $entity_id = $entity->id();
 
     // Only 1 scheduled transition can be found, but multiple executed ones.
-    $transition = WorkflowScheduledTransition::loadByProperties($entity_type, $entity_id, [], $field_name);
+    $transition = WorkflowScheduledTransition::loadByProperties($entity_type_id, $entity_id, [], $field_name);
     if (!$transition) {
       // Create a transition, to pass to the form. No need to use setValues().
       $current_sid = workflow_node_current_state($entity, $field_name);

src/Entity/WorkflowScheduledTransition.php

@@ -123,17 +123,17 @@ class WorkflowScheduledTransition extends WorkflowTransition {
   /**
    * {@inheritdoc}
    */
-  public static function loadByProperties($entity_type, $entity_id, array $revision_ids = [], $field_name = '', $langcode = '', $sort = 'ASC', $transition_type = 'workflow_scheduled_transition') {
+  public static function loadByProperties($entity_type_id, $entity_id, array $revision_ids = [], $field_name = '', $langcode = '', $sort = 'ASC', $transition_type = 'workflow_scheduled_transition') {
     // N.B. $transition_type is set as parameter default.
-    return parent::loadByProperties($entity_type, $entity_id, $revision_ids, $field_name, $langcode, $sort, $transition_type);
+    return parent::loadByProperties($entity_type_id, $entity_id, $revision_ids, $field_name, $langcode, $sort, $transition_type);
   }
 
   /**
    * {@inheritdoc}
    */
-  public static function loadMultipleByProperties($entity_type, array $entity_ids, array $revision_ids = [], $field_name = '', $langcode = '', $limit = NULL, $sort = 'ASC', $transition_type = 'workflow_scheduled_transition') {
+  public static function loadMultipleByProperties($entity_type_id, array $entity_ids, array $revision_ids = [], $field_name = '', $langcode = '', $limit = NULL, $sort = 'ASC', $transition_type = 'workflow_scheduled_transition') {
     // N.B. $transition_type is set as parameter default.
-    return parent::loadMultipleByProperties($entity_type, $entity_ids, $revision_ids, $field_name, $langcode, $limit, $sort, $transition_type);
+    return parent::loadMultipleByProperties($entity_type_id, $entity_ids, $revision_ids, $field_name, $langcode, $limit, $sort, $transition_type);
   }
 
   /**

src/Entity/WorkflowTransitionInterface.php

@@ -38,7 +38,7 @@ interface WorkflowTransitionInterface extends WorkflowConfigTransitionInterface,
   /**
    * Load (Scheduled) WorkflowTransitions, most recent first.
    *
-   * @param string $entity_type
+   * @param string $entity_type_id
    *   The entity type ID.
    * @param int $entity_id
    *   An entity ID.
@@ -56,14 +56,14 @@ interface WorkflowTransitionInterface extends WorkflowConfigTransitionInterface,
    * @return \Drupal\workflow\Entity\WorkflowTransitionInterface
    *   Object representing one row from the {workflow_transition_history} table.
    */
-  public static function loadByProperties($entity_type, $entity_id, array $revision_ids = [], $field_name = '', $langcode = '', $sort = 'ASC', $transition_type = '');
+  public static function loadByProperties($entity_type_id, $entity_id, array $revision_ids = [], $field_name = '', $langcode = '', $sort = 'ASC', $transition_type = '');
 
   /**
    * Given an entity, get all transitions for it.
    *
    * Since this may return a lot of data, a limit is included to allow for only one result.
    *
-   * @param string $entity_type
+   * @param string $entity_type_id
    *   The entity type ID.
    * @param int[] $entity_ids
    *   A (possibly empty) list of entity ID's.
@@ -83,7 +83,7 @@ interface WorkflowTransitionInterface extends WorkflowConfigTransitionInterface,
    * @return WorkflowTransitionInterface[]
    *   An array of transitions.
    */
-  public static function loadMultipleByProperties($entity_type, array $entity_ids, array $revision_ids = [], $field_name = '', $langcode = '', $limit = NULL, $sort = 'ASC', $transition_type = '');
+  public static function loadMultipleByProperties($entity_type_id, array $entity_ids, array $revision_ids = [], $field_name = '', $langcode = '', $limit = NULL, $sort = 'ASC', $transition_type = '');
 
   /**
    * Helper function for __construct.

src/Plugin/Block/WorkflowTransitionBlock.php

@@ -10,13 +10,10 @@ use Drupal\workflow\Entity\WorkflowManager;
 /**
  * Provides a 'Workflow Transition form' block.
  *
- * @todo D8: Add cache options in configuration.
- *    'cache' => DRUPAL_NO_CACHE, // DRUPAL_CACHE_PER_ROLE will be assumed.
- *
  * @Block(
  *   id = "workflow_transition_form_block",
  *   admin_label = @Translation("Workflow Transition form"),
- *   category = @Translation("Forms")
+ *   category = @Translation("Forms"),
  * )
  */
 class WorkflowTransitionBlock extends BlockBase {

src/Plugin/Field/FieldFormatter/WorkflowDefaultFormatter.php

@@ -112,7 +112,7 @@ class WorkflowDefaultFormatter extends FormatterBase implements ContainerFactory
 
     $field_name = $this->fieldDefinition->getName();
     $entity = $items->getEntity();
-    $entity_type = $entity->getEntityTypeId();
+    $entity_type_id = $entity->getEntityTypeId();
 
     // @todo Perhaps global user is not always the correct user.
     // E.g., on ScheduledTransition->execute()? But this function is mostly used in UI.
@@ -143,7 +143,7 @@ class WorkflowDefaultFormatter extends FormatterBase implements ContainerFactory
       return $elements;
     }
 
-    if ($entity_type == 'comment') {
+    if ($entity_type_id == 'comment') {
       // No Workflow form allowed on a comment display.
       // (Also, this avoids a lot of error messages.)
       return $elements;

src/Plugin/Validation/Constraint/WorkflowFieldConstraintValidator.php

@@ -76,7 +76,7 @@ class WorkflowFieldConstraintValidator extends ConstraintValidator implements Co
     }
 
     $field_name = $field_storage->getName();
-    // Check if the 'comment' field name exists on the 'commented' entity type.
+    // A 'comment' field name MUST be equal to content field name.
     // @todo Fix field on a non-relevant entity_type.
     $comment_field_name_ok = FALSE;
     foreach (_workflow_info_fields() as $info) {

src/Plugin/views/filter/WorkflowState.php

@@ -24,7 +24,7 @@ class WorkflowState extends ManyToOne {
   public function init(ViewExecutable $view, DisplayPluginBase $display, array &$options = NULL) {
     parent::init($view, $display, $options);
     $wid = $this->definition['wid'] ?? '';
-    $grouped = isset($options['group_info']['widget']) ? $options['group_info']['widget'] == 'select' : FALSE;
+    $grouped = ($options['group_info']['widget'] ?? '') == 'select';
     $this->valueOptions = workflow_get_workflow_state_names($wid, $grouped);
   }
 

src/WorkflowTransitionListBuilder.php

@@ -65,7 +65,7 @@ class WorkflowTransitionListBuilder extends EntityListBuilder {
   /**
    * {@inheritdoc}
    *
-   * Building the header and content lines for the contact list.
+   * Builds the header column definitions.
    *
    * Calling the parent::buildHeader() adds a column for the possible actions
    * and inserts the 'edit' and 'delete' links as defined for the entity type.

workflow.form.inc

@@ -23,7 +23,11 @@ function workflow_form_alter(&$form, FormStateInterface $form_state, $form_id) {
 
   // Keep aligned:workflow_form_alter(), WorkflowTransitionForm::actions().
   $base_form_id = $form_state->getBuildInfo()['base_form_id'] ?? '';
-  if ($base_form_id !== 'workflow_transition_form') {
+  // The WorkflowTransitionForm::actions() has its own handling.
+  if (!in_array($base_form_id, [
+    'workflow_transition_form',
+    'workflow_scheduled_transition_form',
+  ])) {
     // The WorkflowTransitionForm::actions() has its own handling.
     // E.g., Workflow History tab, Block.
     // Add action/drop buttons next to the 'Save'/'Delete' buttons.
@@ -33,7 +37,6 @@ function workflow_form_alter(&$form, FormStateInterface $form_state, $form_id) {
       $actions = _workflow_transition_form_get_action_buttons($form, $form_state, $actions);
     }
   }
-
 }
 
 /**
@@ -71,7 +74,6 @@ function _workflow_use_action_buttons($button_type = '') {
  *   The workflow element, or empty array.
  */
 function _workflow_transition_form_get_first_workflow_element(&$form) {
-  $workflow_element = [];
 
   // Find the first workflow.
   // (So this won't work with multiple workflows per entity.)
@@ -81,6 +83,8 @@ function _workflow_transition_form_get_first_workflow_element(&$form) {
     return $form;
   }
 
+  // We are on node edit page. First fetche the field.
+  $workflow_element = [];
   foreach (Element::children($form) as $key) {
     $transition = $form[$key]['widget'][0]['#default_value'] ?? NULL;
     if ($transition instanceof WorkflowTransitionInterface) {
@@ -120,7 +124,7 @@ function _workflow_transition_form_get_action_buttons(array $form, FormStateInte
     return $actions;
   }
 
-  $to_sid_widget = $workflow_element['to_sid'];
+  $to_sid_widget = $workflow_element['to_sid']['widget'][0]['target_id'];
   $options = $to_sid_widget['#options'];
   $current_sid = $to_sid_widget['#default_value'];
 
@@ -131,7 +135,7 @@ function _workflow_transition_form_get_action_buttons(array $form, FormStateInte
   }
 
   /** @var \Drupal\workflow\Entity\WorkflowTransitionInterface $transition */
-  $transition = $workflow_element['workflow_transition']['#value'];
+  $transition = $workflow_element['#default_value'];
   $field_name = $transition->getFieldName();
 
   // Find the default submit button and replace with our own action buttons.
@@ -147,7 +151,7 @@ function _workflow_transition_form_get_action_buttons(array $form, FormStateInte
 
   // Add the new submit buttons next to/below the default submit buttons.
   foreach ($options as $sid => $option_name) {
-       // Make the workflow button act exactly like the original submit button.
+    // Make the workflow button act exactly like the original submit button.
     $same_state_button = ($sid == $current_sid);
 
     $workflow_submit_action = $default_submit_action;
@@ -230,7 +234,7 @@ function _workflow_transition_form_get_triggering_button(FormStateInterface $for
   else {
     $input = $form_state->getUserInput();
     $result['field_name'] = $input['field_name'] ?? NULL;
-    $result['to_sid'] = $input['to_sid'] ?? NULL;
+    $result['to_sid'] = $input['to_sid'][0]['target_id'] ?? NULL;
   }
 
   return $result;
@@ -267,18 +271,12 @@ function _workflow_transition_form_element_validate_time($element, FormStateInte
  */
 function _workflow_transition_form_validate_buttons($form, FormStateInterface &$form_state) {
   // Retrieve the data from the form.
-  $transition = $form_state->getValue('workflow_transition');
+  // @todo Use #default_value.
+  $transition = $form_state->getValue('_workflow_transition');
   if ($transition) {
     // On WorkflowTransitionForm.
     $action_info = _workflow_transition_form_get_triggering_button($form_state);
-    $values['to_sid'] = $action_info['to_sid'];
-
-    // Update the form_state.
-    $form_state->setValues($values);
-  }
-  else {
-    // On edit form in WorkflowDefaultWidget.
-    // @see $form_state->getTriggeringElement()
+    $form_state->setValue(['to_sid', 0, 'target_id'], $action_info['to_sid']);
   }
 
 }