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

8.5.x
Nathaniel Catchpole 2017-09-18 12:08:42 +01:00
parent a00387129f
commit 76fd2bdbde
5 changed files with 135 additions and 4 deletions

View File

@ -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;

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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());
}
}
}

View File

@ -117,6 +117,27 @@ class WorkflowTest extends UnitTestCase {
$this->assertArrayEquals([], array_keys($workflow->getTypePlugin()->getStates([])));
}
/**
* 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
*/