magento/magento2 — pull request #685 by fooman (2014-10-10 01:08:56+00:00)
Title: test showing inconsistent behaviour with collections null/not null

Collections are one of the most used classes and I think should work in as many ways as possible. The following is a simplified example of how this issue can come up.

Use case:
1.) each order gets assigned a customer representative at some point after the order has been placed
2.) order info and assigned customer representative are shown in an admin grid

Some further assumptions
we do not want to clutter sales_flat_order and have created our own table to hold the info

order_id customer_rep

when displaying the admin grid the original order table is joined to not duplicate data

Scenario
First order is placed - we look at the grid, 1 order is displayed = no problem
We run our code that assigns the customer rep
our table might now look like

order_id customer_rep
1 1

We place two more orders and we want to look at the grid. Now the grid will either display correctly or error out with the exception

Item (' . get_class($item) . ') with the same id "' . $item->getId() . '" already exist

The determining factor is the sort order of the grid. If the first order is shown first it's fine if it is reversed (the more likely scenario showing latest order on top) the error will come up.

The underlying cause is the collision between the automatically assigned array key and the explicit id based array key.

As any changes to addItem() will break existing behaviour I will leave it up to you to decide on the best way forward, but I think this is something that should be addressed in the framework. As it currently stands we can do collections with all items having an id, collections will all items with a null id but when we have a collection with both null and not null there is a potential for a collision.