Action "List: add item" ignores specified key when setting a numeric key

This is a challenging one. Should we probably introduce a flag in \Drupal\eca\Plugin\DataType\DataTransferObject to optionally turn off rekeying which should be called right before calling $list->set($index, $item); in the action plugin?

Should we probably introduce a flag in \Drupal\eca\Plugin\DataType\DataTransferObject to optionally turn off rekeying which should be called right before calling $list->set($index, $item); in the action plugin?

Either this, or initialize a DTO with a rekey option, or do both. Maybe then let the user decide whether rekey should happen, by adding a corresponding option to the "List: add item" action? There is another place where someone can add an item to a list, for example with "Token: set value" using token name [mylist:+]. But I think that one is probably never used by anyone. Still there may be other places where an item is being added to a list (other actions, such as merging).

For now, I've only implemented the explicit disabling of rekeying for a DTO in the context of this action plugin. Reason being that we don't know where the DTO is being initialized, that could be almost everywhere. And using a special token syntax to disable rekeying is something that won't be used, as @mxh already stated.

Thanks for addressing this. Took a brief look and left one note.

Addressed that in another commit and commented in the MR thread.

Another suggestion, since we already check for instanceof DTO: We could add a helper method to the DTO "setWithoutRekey" or "setWithRekeyOption" which takes care of properly setting the rekey state back-and-forth.

public function setWithRekeyOption($property_name, $value, $notify = TRUE, $disableRekey = FALSE) {
  $initial = $this->disableRekey;
  $this->disableRekey = $disableRekey;
  $this->set(...);
  $this->disableRekey = $initial;
}

Another suggestion, since we already check for instanceof DTO: We could add a helper method to the DTO "setWithoutRekey" or "setWithRekeyOption" which takes care of properly setting the rekey state back-and-forth.

I wouldn't do that, because we're not calling writePropertyValue either, we're calling the set method in the interface which may do more things than just calling writePropertyValue. I think what we now have should be safe.

Not sure what you mean in #9 - I think calling a helper method once would be a bit more readable code rather than back-and-forth flipping of the boolean variable from the outside. Nevermind, the current state of the MR now looks also to be a working solution.

Setting to RTBC since tests are green and code looks like a working solution.

Merged and back ported to 2.1.x, 2.0.x, and 1.1.x

Automatically closed - issue fixed for 2 weeks with no activity.