Problem/Motivation

EntityqueueDragtableWidget::addItemSubmit() writes the new row into user input:

$rows = NestedArray::getValue($user_input, $items_path) ?: [];
$rows[$delta] = ['target_id' => $row_value, '_weight' => $delta];

$rows already contains the add_more key, so it puts the new numeric key after it. key order becomes [0, 1, ... 38, 'add_more', 39].

core WidgetBase::deleteSubmit() then runs array_values($input) on that same array. So on the next Remove click:

  • add_more becomes a numeric row
  • the row that was just added is pushed past items_count

On rebuild:

  • the pushed row is dropped by unset($elements[$field_state['items_count']]), so the item the editor added is gone, no message
  • the promoted add_more row has no target_id, so formElement() falls back to $items[$delta] and renders an item that is already in the list. This duplicate is then saved.

Steps to reproduce

  1. Queue with some items
  2. Add item
  3. Remove any row, before saving
  4. One item is now duplicated, and the added item is gone

Add alone is fine. Remove alone is fine. Only add + remove in the same form session.

Probably also the reason for #3618249: EntitySubqueue::getItemPosition() triggers "Undefined array key target_id" PHP warning. A row without target_id is exactly what getItemPosition() warns about.

Proposed resolution

Keep add_more last at the end of addItemSubmit():

$add_more = $rows['add_more'];
unset($rows['add_more']);
ksort($rows, SORT_NUMERIC);
$rows['add_more'] = $add_more;

Issue fork entityqueue-3624745

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:

About issue forks

Comments

doxigo created an issue. See original summary.

amateescu made their first commit to this issue’s fork.

amateescu opened merge request !68

amateescu’s picture

amateescu commented
Status: Active » Needs review

Thanks for the detailed write-up! The part about WidgetBase::deleteSubmit() is spot on. When add_more sits in the middle of the submitted rows, array_values() turns it into a row, every row after it shifts by one, and the added item ends up past the item count.

I couldn't reproduce it with the steps from the summary on a stock theme, though. addItemSubmit() only changes the input of the Add item request itself. The next request (the Remove click) builds its input from the POST, in page order, and every core theme renders the add control after the table (Gin inherits Claro's template). So with a stock theme add_more always comes last, and EntityqueueDragtableWidgetTest::testAddInvalidAndRemove() already does add, then remove, in a browser.

It does happen as soon as the add control is posted before the rows. I moved it above the table in testAddInvalidAndRemove() with one executeScript() line, and without a fix, removing a row loses the added item. My guess is that your admin theme or a template override renders the add control above the table. Could you check if that's the case?

The MR makes the widget override deleteSubmit(): it sets the add_more input aside while core removes the row, then puts it back, so the order of the posted fields no longer matters. Moving add_more to the end in addItemSubmit(), as proposed in the summary, wouldn't help here, because that input is thrown away once the Add item response is sent.

Since the fix is safe either way, I'm going to commit it now. If you still see the problem after updating, please reopen with details about your theme setup.

amateescu changed the visibility of the branch 3624745-adding-an-item to hidden.

  • amateescu committed dbf34fcf on 8.x-1.x 
    fix: #3624745 Adding an item and then removing one creates a duplicate...
amateescu’s picture

amateescu commented
Status: Needs review » Fixed

Merged and marking as fixed, but please reopen if needed :)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.