summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathaniel Catchpole2017-09-18 11:08:42 (GMT)
committerNathaniel Catchpole2017-09-18 11:08:42 (GMT)
commit76fd2bdbdebe2f709d81c24eda1d82a1d1f5d4b4 (patch)
treed96409dbeaeae929d37015521f5dcc36a4435a03
parenta00387129ffb3b8a8b7e62b4d315be0e8253a643 (diff)
Issue #2886567 by timmillwood, Sam152, Manuel Garcia, Vj, xjm, tstoeckler: Adding a workflow state or transition with an integer ID results in unrecoverable fatal errors
-rw-r--r--core/modules/workflows/src/Plugin/WorkflowTypeBase.php16
-rw-r--r--core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeStateForm.php2
-rw-r--r--core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeTransitionForm.php2
-rw-r--r--core/modules/workflows/tests/src/Functional/WorkflowUiTest.php98
-rw-r--r--core/modules/workflows/tests/src/Unit/WorkflowTest.php21
5 files changed, 135 insertions, 4 deletions
diff --git a/core/modules/workflows/src/Plugin/WorkflowTypeBase.php b/core/modules/workflows/src/Plugin/WorkflowTypeBase.php
index 5810938..9cd0fef 100644
--- a/core/modules/workflows/src/Plugin/WorkflowTypeBase.php
+++ b/core/modules/workflows/src/Plugin/WorkflowTypeBase.php
@@ -263,7 +263,7 @@ abstract class WorkflowTypeBase extends PluginBase implements WorkflowTypeInterf
}
/**
- * Sort states or transitions by weight and label.
+ * Sort states or transitions by weight, label, and key.
*
* @param \Drupal\workflows\StateInterface[]|\Drupal\workflows\TransitionInterface[] $objects
* Objects to multi-sort.
@@ -273,15 +273,27 @@ abstract class WorkflowTypeBase extends PluginBase implements WorkflowTypeInterf
*/
protected static function labelWeightMultisort($objects) {
if (count($objects) > 1) {
+ // Separate weights, labels, and keys into arrays.
$weights = $labels = [];
+ $keys = array_keys($objects);
foreach ($objects as $id => $object) {
$weights[$id] = $object->weight();
$labels[$id] = $object->label();
}
+ // Sort weights, labels, and keys in the same order as each other.
array_multisort(
+ // Use the numerical weight as the primary sort.
$weights, SORT_NUMERIC, SORT_ASC,
- $labels, SORT_NATURAL, SORT_ASC
+ // When objects have the same weight, sort them alphabetically by label.
+ $labels, SORT_NATURAL, SORT_ASC,
+ // Ensure that the keys (the object IDs) are sorted in the same order as
+ // the weights.
+ $keys
);
+ // Combine keys and weights to make sure the weights are keyed with the
+ // correct keys.
+ $weights = array_combine($keys, $weights);
+ // Return the objects sorted by weight.
return array_replace($weights, $objects);
}
return $objects;
diff --git a/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeStateForm.php b/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeStateForm.php
index 679b314..a466a7d 100644
--- a/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeStateForm.php
+++ b/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeStateForm.php
@@ -22,7 +22,7 @@ class ComplexTestTypeStateForm extends WorkflowTypeStateFormBase {
'#type' => 'textfield',
'#title' => $this->t('Extra'),
'#description' => $this->t('Extra information added to state'),
- '#default_value' => isset($configuration['states'][$state->id()]['extra']) ? $configuration['states'][$state->id()]['extra'] : '',
+ '#default_value' => $state && isset($configuration['states'][$state->id()]['extra']) ? $configuration['states'][$state->id()]['extra'] : '',
];
return $form;
}
diff --git a/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeTransitionForm.php b/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeTransitionForm.php
index 45e1513..a88306c 100644
--- a/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeTransitionForm.php
+++ b/core/modules/workflows/tests/modules/workflow_type_test/src/Form/ComplexTestTypeTransitionForm.php
@@ -22,7 +22,7 @@ class ComplexTestTypeTransitionForm extends WorkflowTypeTransitionFormBase {
'#type' => 'textfield',
'#title' => $this->t('Extra'),
'#description' => $this->t('Extra information added to transition'),
- '#default_value' => isset($configuration['transitions'][$transition->id()]['extra']) ? $configuration['transitions'][$transition->id()]['extra'] : '',
+ '#default_value' => $transition && isset($configuration['transitions'][$transition->id()]['extra']) ? $configuration['transitions'][$transition->id()]['extra'] : '',
];
return $form;
}
diff --git a/core/modules/workflows/tests/src/Functional/WorkflowUiTest.php b/core/modules/workflows/tests/src/Functional/WorkflowUiTest.php
index fcbcbd0..b15b240 100644
--- a/core/modules/workflows/tests/src/Functional/WorkflowUiTest.php
+++ b/core/modules/workflows/tests/src/Functional/WorkflowUiTest.php
@@ -293,4 +293,102 @@ class WorkflowUiTest extends BrowserTestBase {
$this->assertEquals('Extra global settings', $workflow->getTypePlugin()->getConfiguration()['example_setting']);
}
+ /**
+ * Test a workflow, state, and transition can have a numeric ID and label.
+ */
+ public function testNumericIds() {
+ $this->drupalLogin($this->createUser(['administer workflows']));
+ $this->drupalGet('admin/config/workflow/workflows');
+ $this->clickLink('Add workflow');
+ $this->submitForm(['label' => 123, 'id' => 123, 'workflow_type' => 'workflow_type_complex_test'], 'Save');
+
+ $this->assertSession()->addressEquals('admin/config/workflow/workflows/manage/123/add_state');
+
+ $this->submitForm(['label' => 456, 'id' => 456], 'Save');
+ $this->assertSession()->pageTextContains('Created 456 state.');
+
+ $this->clickLink('Add a new state');
+ $this->submitForm(['label' => 789, 'id' => 789], 'Save');
+ $this->assertSession()->pageTextContains('Created 789 state.');
+
+ $this->clickLink('Add a new transition');
+ $this->submitForm(['id' => 101112, 'label' => 101112, 'from[456]' => 456, 'to' => 789], 'Save');
+ $this->assertSession()->pageTextContains('Created 101112 transition.');
+
+ $workflow = $this->container->get('entity_type.manager')->getStorage('workflow')->loadUnchanged(123);
+ $this->assertEquals(123, $workflow->id());
+ $this->assertEquals(456, $workflow->getTypePlugin()->getState(456)->id());
+ $this->assertEquals(101112, $workflow->getTypePlugin()->getTransition(101112)->id());
+ $this->assertEquals(789, $workflow->getTypePlugin()->getTransition(101112)->to()->id());
+ }
+
+ /**
+ * Test the sorting of states and transitions by weight and label.
+ */
+ public function testSorting() {
+ $workflow = Workflow::create(['id' => 'test', 'type' => 'workflow_type_complex_test', 'label' => 'Test']);
+ $workflow
+ ->getTypePlugin()
+ ->setConfiguration([
+ 'states' => [
+ 'twoa' => [
+ 'label' => 'twoa',
+ 'weight' => 2,
+ ],
+ 'three' => [
+ 'label' => 'three',
+ 'weight' => 3,
+ ],
+ 'twob' => [
+ 'label' => 'twob',
+ 'weight' => 2,
+ ],
+ 'one' => [
+ 'label' => 'one',
+ 'weight' => 1,
+ ],
+ ],
+ 'transitions' => [
+ 'three' => [
+ 'label' => 'three',
+ 'from' => ['three'],
+ 'to' => 'three',
+ 'weight' => 3,
+ ],
+ 'twoa' => [
+ 'label' => 'twoa',
+ 'from' => ['twoa'],
+ 'to' => 'twoa',
+ 'weight' => 2,
+ ],
+ 'one' => [
+ 'label' => 'one',
+ 'from' => ['one'],
+ 'to' => 'one',
+ 'weight' => 1,
+ ],
+ 'twob' => [
+ 'label' => 'twob',
+ 'from' => ['twob'],
+ 'to' => 'twob',
+ 'weight' => 2,
+ ],
+ ],
+ ]);
+ $workflow->save();
+
+ $this->drupalLogin($this->createUser(['administer workflows']));
+ $this->drupalGet('admin/config/workflow/workflows/manage/test');
+ $expected_states = ['one', 'twoa', 'twob', 'three'];
+ $elements = $this->xpath('//details[@id="edit-states-container"]//table/tbody/tr');
+ foreach ($elements as $key => $element) {
+ $this->assertEquals($expected_states[$key], $element->find('xpath', 'td')->getText());
+ }
+ $expected_transitions = ['one', 'twoa', 'twob', 'three'];
+ $elements = $this->xpath('//details[@id="edit-transitions-container"]//table/tbody/tr');
+ foreach ($elements as $key => $element) {
+ $this->assertEquals($expected_transitions[$key], $element->find('xpath', 'td')->getText());
+ }
+ }
+
}
diff --git a/core/modules/workflows/tests/src/Unit/WorkflowTest.php b/core/modules/workflows/tests/src/Unit/WorkflowTest.php
index 4d28ab6..8cf4493 100644
--- a/core/modules/workflows/tests/src/Unit/WorkflowTest.php
+++ b/core/modules/workflows/tests/src/Unit/WorkflowTest.php
@@ -118,6 +118,27 @@ class WorkflowTest extends UnitTestCase {
}
/**
+ * Test numeric IDs when added to a workflow.
+ */
+ public function testNumericIdSorting() {
+ $workflow = new Workflow(['id' => 'test', 'type' => 'test_type'], 'workflow');
+ $workflow_type = $workflow->getTypePlugin();
+
+ $workflow_type->addState('1', 'One');
+ $workflow_type->addState('2', 'Two');
+ $workflow_type->addState('3', 'ZZZ');
+ $workflow_type->addState('4', 'AAA');
+
+ $workflow_type->setStateWeight('1', 1);
+ $workflow_type->setStateWeight('2', 2);
+ $workflow_type->setStateWeight('3', 3);
+ $workflow_type->setStateWeight('4', 3);
+
+ // Ensure numeric states are correctly sorted by weight first, label second.
+ $this->assertEquals([1, 2, 4, 3], array_keys($workflow_type->getStates()));
+ }
+
+ /**
* @covers ::getStates
*/
public function testGetStatesException() {