Framework: Eloquent\Collection::merge skipping some items

[KKSzymanowski]

Imagine we have something like GitHub issues with just comments and status changes.
Let's say I want to display a combined list of them ordered by date of creation:

$comments = $issue->comments;
$statusChanges = $issue->statusChanges;

$actions = $comments->merge($statusChanges)->sortBy('created_at');

When some comments and statusChanges have the same primary keys, particular comments will not be included in the final collection because they have been overwritten by statusChanges.

I suppose this isn't expected behaviour, when merged collections contain instances of different classes.

I propose a change to Eloquent\Collection::merge looking something like this:

public function merge($items)
{
    $dictionary = $this->getDictionary();

    $result = [];

    foreach($dictionary as $item) {
        $result[$item->getKey() . get_class($item)] = $item;
    }

    foreach ($items as $item) {
        $result[$item->getKey() . get_class($item)] = $item;
    }

    return new static(array_values($result));
}

That way we will not overwrite any items which have the same primary key unless they are instances of the same class.

Other option to solve this(to be absolutely certain we preserve backward compatibility) would be to introduce a concatentate method which will simply put those two collections together and wouldn't care about any possible duplicates.

[themsaid]

This is an intended behaviour, the Eloquent collection merge is mainly there to allow merging two collection of models of the same type, in such case have a unique key for every item makes sense.

If you want to be able to merge two collections of different types you can convert it to base collection:

$comments->toBase()->merge($statusChanges);

[KKSzymanowski]

I didn't know about the toBase method. Thank you.

[themsaid]

Welcome :)