Skip to content
This repository has been archived by the owner on Aug 16, 2019. It is now read-only.

Deletion populator and multiple new records #128

Open
wants to merge 1 commit into
base: f6
Choose a base branch
from

Conversation

koszcz
Copy link

@koszcz koszcz commented Mar 9, 2018

I found out that using the default example for deletion Populator does not allow to have multiple new items added to collection.
In this case all new fragments do not have id. So find matcher will return the wrong item since song.id.to_s == fragment["id"].to_s (nil == nil) will always be true from 2nd run.

I suggest we additonally check for id presence.

I found out that using the default example for deletion Populator does not allow to have multiple new items added to collection.
In this case all new fragments do not have `id`. So find matcher will return the wrong item since `song.id.to_s == fragment["id"].to_s` (nil == nil) will always be true from 2nd run.

I suggest we additonally check for id presence.
@@ -269,7 +269,7 @@ You can implement your own deletion.
collection :songs,
populator: ->(fragment:, **) {
# find out if incoming song is already added.
item = songs.find { |song| song.id.to_s == fragment["id"].to_s }
item = songs.find { |song| song.id.present? && song.id.to_s == fragment["id"].to_s }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #present? method not exist.

1.present?
NoMethodError: undefined method `present?' for 1:Fixnum

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants