Hashie#deep_find_all not working

[derekyau]

I'm having an issue being able with the example in the documentation.
Ruby version: ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]

user = {
  name: { first: 'Bob', last: 'Boberts' },
  groups: [
    { name: 'Rubyists' },
    { name: 'Open source enthusiasts' }
  ]
}

user.extend Hashie::Extensions::DeepFind
user.deep_find_all(:name) #=> [{:first=>"Bob", :last=>"Boberts"}] (!missing the other values!?)

When I try to run user.deep_find_all(:name) I only get the result as [{:first=>"Bob", :last=>"Boberts"}] which is wrong because it doesn't include the nested values as it should.

when I remove the top level 'name' key from the user hash and run the code again, I do get the two values Rubyists and Open source enthusiasts.

Am I missing something? Or is this not working?

[dblock]

Check against HEAD please, maybe #319 fixed it? If not write a spec that reproduces the problem? Thx.

[derekyau]

hi @dblock thanks for getting back so quick. I've forked the project/created a branch that has a spec that demonstrates the issue https://github.com/derekyau/hashie/tree/deep_find_all_bug

[derekyau]

@dblock I've done a first go at attempting to fix this bug. I'm not sure whether there's a better way to do it, so for now I've just committed it in my branch here: derekyau@628dd1c

I've added a few more specs to catch more cases. Let me know what you think, cheers.

[derekyau]

Found an issue with what my current implementation does - if the object that matches is an Array, the elements in the array get added directly to the matches even though its the Array itself that should be added (this is in fact true, but since I call .flatten at the end we lose the visibility).

Otherwise, seems to work - I'll circle back sometime and see if there's something that can be done about this, its also not in the spec, so I'll write that up sometime as well to have it failing

Let me know if there are further suggestions

[dblock]

Thanks @derekyau. Make pull requests please!

[derekyau]

hey @dblock sorry haven't had time to look at this in awhile but it came up in another project recently. I was tracing the commit history on deep_find.rb and reverted it to commit 36ece58 since this seems to actually work. I've updated specs to reflect the failing cases and it was indeed working with this code before. Should we merge it in with a PR? I believe there is already one existing here: #329

[dblock]

You mean should we merge #329? If it passed specs then possibly ;)

[derekyau]

yes :) maybe go through it on a high level to see if you agree? Specs do pass. What I meant to say was that I realized deep_find changed from commit 36ece58 to b3c123c and actually the code from the earlier commit I believe actually works to spec. I pit that code against my updated specs to see if it would pass and it does.

[derekyau]

@dblock any further thoughts? Would love to get this fixed on master so I don't have to reference my branch :)

[dblock]

See my comment in #329 @derekyau, there's some action items for you there. You should reopen a PR from your fork with this or push an update to it.

[derekyau]

@dblock got it thanks, sorry didn't notice it. i'll get to it asap

[michaelherold]

This was resolved by #378. Here's the verification:

$ bin/console
[1] pry(main)> user = { name: { first: 'Bob', last: 'Boberts' }, groups: [ { name: 'Rubyists' }, { name: 'Open source enthusiasts' } ] }
=> {:name=>{:first=>"Bob", :last=>"Boberts"}, :groups=>[{:name=>"Rubyists"}, {:name=>"Open source enthusiasts"}]}
[2] pry(main)> user.extend Hashie::Extensions::DeepFind
=> {:name=>{:first=>"Bob", :last=>"Boberts"}, :groups=>[{:name=>"Rubyists"}, {:name=>"Open source enthusiasts"}]}
[3] pry(main)> user.deep_find_all(:name)
=> [{:first=>"Bob", :last=>"Boberts"}, "Rubyists", "Open source enthusiasts"]