Problem/Motivation

EntitySubqueueItemsFieldItemList overrides appendItem() so that, when the queue has queue_settings.reverse: TRUE, the new item is unshifted to the front of the list instead of appended:

public function appendItem($value = NULL) {
  $queue = $this->getEntity()->getQueue();
  if ($queue && $queue->isReversed()) {
    $item = $this->createItem(0, $value);
    array_unshift($this->list, $item);
    return $item;
  }
  return parent::appendItem($value);
}

appendItem() is a generic Typed Data list operation, not a queue operation. Its contract is "add an item at the end of the list", and core calls it in contexts that have nothing to do with adding an entity to a queue. Overriding its semantics therefore corrupts the item list in three ways.

1. Building the subqueue form shifts every delta by one

\Drupal\Core\Field\WidgetBase::formMultipleElements() appends an empty item to build the trailing element of an unlimited-cardinality widget:

for ($delta = 0; $delta <= $max; $delta++) {
  // Add a new empty item if it doesn't exist yet at this delta.
  if (!isset($items[$delta])) {
    $items->appendItem();
  }

On a reversed queue that empty item lands at delta 0. A subqueue holding [10, 11, 12, 13] becomes [[], 10, 11, 12, 13] as a side effect of merely rendering its edit form, so $subqueue->get('items')->get($delta) no longer matches widget row $delta. Anything reading the item list during form build (hook_form_alter() implementations that add per-row elements, third-party widgets, #process callbacks) silently addresses the wrong item.

2. Setting an item by delta writes to the wrong position

\Drupal\Core\TypedData\Plugin\DataType\ItemList::set() appends when the requested delta does not exist yet, then writes into the returned item:

$item = $this->list[$index] ?? $this->appendItem();
$item->setValue($value);

So on a reversed queue, starting from a single item and calling set(1, …) then set(2, …) yields [3, 2, 1] rather than [1, 2, 3].

3. Item delta contexts go stale

array_unshift() is applied to $this->list directly without calling rekey(), so each item keeps its old name/delta context. After the unshift, $items->get(2)->getName() still returns 1. Constraint violation property paths and any code relying on FieldItemInterface::getName() then point at the wrong delta.

There is currently no test coverage for reverse: TRUE at all — every test fixture in the module sets 'reverse' => FALSE, which is why this went unnoticed.

Steps to reproduce

  1. Create a queue with "Add new items to the top of the queue" enabled and add 4 items to a subqueue.
  2. Build its edit form and inspect the entity.
  3. Widget row 1 now shows item 11 while $subqueue->get('items')->get(1) returns 10.
$subqueue = EntitySubqueue::load('my_subqueue');
// [{"target_id":"10"},{"target_id":"11"},{"target_id":"12"},{"target_id":"13"}]

\Drupal::service('entity.form_builder')->getForm($subqueue, 'edit');
// [[],{"target_id":"10"},{"target_id":"11"},{"target_id":"12"},{"target_id":"13"}]

Observed downstream in a custom hook_form_entity_subqueue_form_alter() that adds a scheduling date widget to each row: every row after the first was keyed to the previous row's entity, so dates were written against the wrong node and the last row's values were dropped. Items and ordering were unaffected, because those come from submitted form values rather than from the item list — which is what made the bug so hard to spot.

Proposed resolution

Move the reversal into EntitySubqueue::addItem(), which is the operation that actually means "add this entity to the queue", and drop the appendItem() override so the list behaves like any other EntityReferenceFieldItemList:

public function addItem(EntityInterface $entity) {
  $items = $this->get('items');
  $queue = $this->getQueue();

  if ($queue && $queue->isReversed()) {
    $values = $items->getValue();
    array_unshift($values, ['target_id' => $entity->id()]);
    $items->setValue($values);
  }
  else {
    $items->appendItem($entity->id());
  }

  return $this;
}

addItem() is the only caller in the module that needs the reversed behaviour, so the user-visible behaviour of the "Add new items to the top of the queue" setting is unchanged.

The attached patch against 8.x-1.12 also adds a kernel test, EntitySubqueueReverseTest, covering:

  • ::addItem() still prepends on a reversed queue — passes before and after the fix, guarding the existing behaviour;
  • appending an empty item does not shift existing deltas and leaves each item reporting its own delta — fails before the fix;
  • ItemList::set() by delta does not reorder the list — fails before the fix.

Test results with the patch applied, on Drupal 11.3.17 / PHP 8.3 / MariaDB 11.8:

  • tests/src/Kernel/ — 42 tests, 1062 assertions, no failures
  • tests/src/Functional/ — 18 tests, 300 assertions, no failures
  • phpcs --standard=Drupal,DrupalPractice — clean on all changed files

tests/src/FunctionalJavascript/ was not run (no webdriver available in the environment used). The patch touches neither the widget nor any AJAX callback.

API changes

EntitySubqueueItemsFieldItemList::appendItem() no longer reverses. Any code calling appendItem() directly on a subqueue's item list and relying on the item landing at the top must call EntitySubqueue::addItem() instead. No such caller exists in the module itself.

The patch keeps the EntitySubqueueItemsFieldItemList class and its setClass() registration in order to stay minimal; the class simply has no behaviour left.

Remaining tasks

  • Decide whether EntitySubqueueItemsFieldItemList should be removed outright now that it is empty, or kept as an extension point. Removing it would be a follow-up.
  • Consider whether the dragtable widget's "Add item" button should also honour reverse — it currently appends the new row to the bottom regardless, which is arguably inconsistent with the setting, but that is existing behaviour and out of scope here.

User interface changes

None.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

owilliwo created an issue. See original summary.

owilliwo’s picture

Status: Active » Needs review