processwire / processwire-requests

ProcessWire feature requests.
39 stars 0 forks source link

WireArray: setting for if duplicates should be unset or skipped #447

Open Toutouwai opened 2 years ago

Toutouwai commented 2 years ago

Short description of the enhancement

When adding an item to a WireArray with WireArray::add() (which is used by several other core methods, e.g. in PageArray) and that item already exists in the WireArray, the default behaviour is to unset the existing item and then add the new item at the end.

It would be good if there was a optional setting that preserved the position of the existing item, so that instead of unsetting that existing item it skipped adding the new item. Here's a related forum topic where a user wanted to add groups of items to a PageArray so that the PageArray would be ordered in terms of relevance but the behaviour of WireArray::add() was interfering with that order: https://processwire.com/talk/topic/27305-wirearray-sorts-automatically-according-to-page-id-even-if-add-or-append-methods-are-used/

Sidenotes

I noticed a couple of unexpected things that probably can't be altered now because they'd be breaking changes.

1. There is a WireArray::import() method that already does the skipping of duplicates as I'm requesting here, but that method is overwritten by PageArray::import() which doesn't use the same approach and so duplicates are unset instead of skipped. Ideally these methods would follow the same approach to duplicates and if that were so then no further changes would be needed because the user could select add() or import() depending on how they wanted the duplicates handled.

2. From what I can see, when a single item is supplied to WireArray::add() it gets appended but when a WireArray is supplied the items get prepended. Strange that these would be treated in opposite ways.

ryancramerdesign commented 1 year ago

@Toutouwai The WireArray::add() method is meant to be an alias of the WireArray::append() method. If I understand what you are looking for, it would be something like this?

if(!$a->has($item)) $a->add($item); 

I don't think I can change the meaning of the add() method, but I could add a new setDuplicateChecking() option. The only thing is that would affect any add() method calls that follow it, which may not be ideal if some other code uses the same WireArray and expects the normal behavior... so maybe that's not a good option.

What I think might be better is a new method or syntax for the purpose. For instance, WireArray doesn't currently implement this syntax (which would throw an exception):

$a[] = $item;

I could have it implement that array syntax and have it use the behavior you are looking for (on WireArrays that have duplicate checking enabled, such as PageArray):

$a[] = $page1; // add
$a[] = $page2; // add
$a[] = $page1; // does nothing since $page1 already present, leaves it as-is
Toutouwai commented 1 year ago

@ryancramerdesign, I lean towards your setDuplicateChecking() suggestion because the $a[] syntax implies that existing items can only be retained in their earlier position if new items are added individually, but I think in many cases (including the original forum post that prompted this request) users want to add multiple items at once. If this was handled via setDuplicateChecking() then users could use the existing add()/append()/import() methods but with whichever duplicating handling mode they prefer.

The only thing is that would affect any add() method calls that follow it, which may not be ideal if some other code uses the same WireArray and expects the normal behavior... so maybe that's not a good option.

If users are setting a non-standard mode for setDuplicateChecking() they could set it back to the standard mode after they have added their item(s) to avoid affecting subsequent code. So for instance if the new mode was set by passing the string 'skipExisting' as an argument to setDuplicateChecking() then you could have code like this:

$my_items->setDuplicateChecking('skipExisting');
$my_items->add($my_new_items);
$my_items->setDuplicateChecking(true);

I don't know the best way to signal the setDuplicateChecking() mode - you'll probably have better ideas.