Stillat / relationships

Provides automated bi-directional @statamic entry relationships
MIT License
18 stars 1 forks source link

Sync issues with `oneToMany` #35

Closed marcorieser closed 6 months ago

marcorieser commented 6 months ago

Issue

I have some weird sync problems when using oneToMany().

Personally I do not need events to be triggering but since there are issues (https://github.com/Stillat/relationships/issues/32) when they are enabled, I tested my cases with them enabled too.

Reproduction

Test Cases (withEvents(false))

[!CAUTION] Make sure to reset the changes in content between the cases.

Case Expected Actual Passes
In a book entry, remove the author and save
  • Author gets removed from book entry
  • Book gets removed from author entry

- Author gets removed from book entry - Book gets removed from author entry
In an author entry, remove a book and save - Book gets removed from author entry - Author gets removed from book entry - Book gets removed from author entry - Author gets removed from book entry
In an author entry, assign a book already assigned to another author - Book gets added to the saving author entry - Book gets removed from the previous author entry - Author gets changed in the book entry - Book gets added to the saving author entry - Book does **NOT** get removed from the previous author entry - Author gets changed in the book entry
In a book entry, assign another author - Book gets added to the new author entry - Book gets removed from the previous author entry - Author gets changed in the book entry - Book gets added to the new author entry - Book gets removed from the previous author entry - Author gets changed in the book entry

Test Cases (withEvents(true))

[!CAUTION] Make sure to reset the changes in content between the cases.

Case Expected Actual Passes
In a book entry, remove the author and save
  • Author gets removed from book entry
  • Book gets removed from author entry

- Author gets removed from book entry - Book gets removed from author entry
In an author entry, remove a book and save - Book gets removed from author entry - Author gets removed from book entry - **All** related Books get removed from author entry - Author gets removed from **some** (but not all) previous related book entries
In an author entry, assign a book already assigned to another author - Book gets added to the saving author entry - Book gets removed from the previous author entry - Author gets changed in the book entry - Book gets added to the saving author entry - Book gets removed from the previous author entry - Author gets changed in the book entry
In a book entry, assign another author - Book gets added to the new author entry - Book gets removed from the previous author entry - Author gets changed in the book entry - Book gets added to the new author entry - Book does **NOT** get removed from the previous author entry - Author gets changed in the book entry

JohnathonKoster commented 6 months ago

Thank you very much for this!

JohnathonKoster commented 6 months ago

I may end up forcing withEvents(true) for oneToMany to ensure the down-stream entries get updated. I've identified the issue with the failing cases when withEvents(true) is enabled. I'll be looking to do a release either this evening or sometime tomorrow after taking a look at a few other open issues 🙂

marcorieser commented 6 months ago

Glad you found the issue. Thank you so much.

JohnathonKoster commented 6 months ago

Another edge case cropped up that is proving to be challenging. Will keep you updated 👍

JohnathonKoster commented 6 months ago

Alright, I've got all but this one working now "In an author entry, assign a book already assigned to another author" (withEvents = false). I'll do some thinking on if that can be reasonably supported without cascading events.

I'm going to sit on this one overnight and work on some test coverage before pushing it out, though

JohnathonKoster commented 6 months ago

As soon as I walked away I thought of a solution. All of your scenarios will now work, regardless of the withEvents setting, with expanded test coverage. Should be good to go as of 2.1.5

Thanks again for putting together such a great sample repo!

marcorieser commented 6 months ago

Just gave it a shot and everything works like a charm. Tried to be a bit mean by mix and match cases from above. Everything perfect. Thank you!