Open SpikedCola opened 1 year ago
That's a great report! I agree, it looks like a bug. Do you think you can write a test for your fix and create a pull request for that?
Hi @mringler,
I have almost no experience writing tests, but I suppose this is a good opportunity to learn! Let me see what I can come up with.
That's the spirit! If you haven't found it already, there is documentation how to run the tests locally.
To get you started, it looks like there are a bunch of tests for ObjectCombinationCollection, but they are not at the expected place, but in the files:
tests/Propel/Tests/Generator/Builder/Om/GeneratedObjectM2MRelationThreePKsTest.php
tests/Propel/Tests/Generator/Builder/Om/GeneratedObjectM2MRelationThreePKs2Test.php
and they look pretty messy, too.
You can still put the tests into those files, but the right place would be in
tests/Propel/Tests/Runtime/Collection/ObjectCombinationCollection.php
There are tests for the parent's diff()
at the end of
tests/Propel/Tests/Runtime/Collection/CollectionTest.php
it might help to have a look at those.
So you could create the ObjectCombinationCollection.php
file using CollectionTest.php
as a template (or find another way of course). Let me know if you run into issues. Godspeed!
Apologies in advance if this is hard to follow. This is with the latest version,
dev-master 011aaa7
.My schema is using 3 primary keys on an
isCrossRef
table. In this case, 1 or more performers appearing at an event, with a flag to say who is a headliner. Schema is like this:I am using an
ObjectCombinationCollection
to collect the perfomer & headliner flag, and calling the generatedsetPerformerHeadliner()
function like this:The first pass works correctly, rows are inserted as expected.
On subsequent passes, however, the
event_performers
cross-reference rows are deleted and re-created on every pass.Digging in to the generated
setPerformerIsHeadliners()
function, the existing records are fetched from the db,diff()
is called, and then any records scheduled for deletion are removed:If I print
$combinationCollPerformerIsHeadlinersScheduledForDeletion
, on every subsequent pass it lists all of my cross-references. So it appearsdiff()
is not working as expected and all of my relationships are deleted.Notice that
if (!$currentPerformerIsHeadliners->contains(...$PerformerIsHeadliner)) {
is using the spread operator.$currentPerformerIsHeadliners
returns an object of typeObjectCombinationCollection
, but there is no overload fordiff()
. The version ofdiff()
that is called is in theCollection
class, which doesn't use the spread operator on thecontains()
call:This results in the
search()
function ofObjectCombinationCollection
being passed args that look like:You can see the
foreach (func_get_args() as $pos => $obj) {
loop is expecting to loop over the 2 args, checking if they areActiveRecordInterface
. But the double-array breaks that:If I edit
ObjectCombinationCollection
and overload thediff()
function, such that it uses the spread operator like insetPerformerIsHeadliners()
:Then
search()
is passed args like[ Performer_object, IsHeadliner_bool ]
which works correctly, and my generatedsetPerformerIsHeadliners()
functions work properly. Rows aren't deleted erroneously on every pass.I believe this is a bug.