Breadcrumbs/Mongo integration: Handle 'any_of' cases

[Cawllec]

Goal

Enables mongo breadcrumb filters to include nested keys, such as in any_of cases.

[tobyhs]

This variable could be slightly misleading because an Array value doesn't necessarily mean it was an $or (it could have been $and or something else).

[tobyhs]

This method fails on queries such as SomeModel.where(tags: ['a', 'b']):

irb(main):001:0> def sanitize_filter(filter_hash)
irb(main):002:1>   filter_hash.map do |key, value|
irb(main):003:2*     if value && value.is_a?(Array)
irb(main):004:3>       filtered_or = value.map { |filter| sanitize_filter(filter) }
irb(main):005:3>       [key, filtered_or]
irb(main):006:3>     else
irb(main):007:3*       [key, '?']
irb(main):008:3>     end
irb(main):009:2>   end.to_h
irb(main):010:1> end
=> :sanitize_filter
irb(main):011:0> sanitize_filter({'tags' => [1, 2]})
NoMethodError: undefined method `map' for 1:Integer
Did you mean?  tap
	from (irb):2:in `sanitize_filter'
	from (irb):4:in `block (2 levels) in sanitize_filter'
	from (irb):4:in `map'
	from (irb):4:in `block in sanitize_filter'
	from (irb):2:in `each'
	from (irb):2:in `map'
	from (irb):2:in `sanitize_filter'
	from (irb):11

I would change the method/algorithm to something like:

def sanitize_filter_element(element)
  if element.is_a?(Array)
    element.map { |e| sanitize_filter_element(e) }
  elsif element.is_a?(Hash)
    element.map { |k, v| [k, sanitize_filter_element(v)] }.to_h
  else
    '?'
  end
end

The only concern I have about the above is that there could be giant arrays for $in, but I'll pull the old college textbook approach of writing "This is left as an exercise for the reader".

[tobyhs]

A Numeric type isn't in the valid types of https://docs.mongodb.com/mongoid/current/tutorials/mongoid-documents/#fields so I would just use Integer here instead.

[tobyhs]

[Integer] (as the depth isn't going to be something that isn't a whole number)

[tobyhs]

I would change the block params to |(key, value), output| so you don't need the next line

[tobyhs]

This should be module instead of class to match the mongo gem

[tobyhs]

This should probably be load (and you might need to end the argument with ".rb") instead of require. The reason is that if the file was required before from some other test, then the require won't load the file.

[tobyhs]

leave_mongo_breadcrumb (spelling)

[tobyhs]

collection (spelling)

[tobyhs]

I would use single quotes for the outer quotes here so that the blackslashes aren't needed and it is easier to read this string literal

[tobyhs]

I would omit all your sanitize_filter_value method/message expectations since your equality expectations should be good enough. Method/message expectations tend to be for things that have side effects that you don't want run (e.g. remote/network calls).

[tobyhs]

I would change this to not_to have_key(request_id) as the key shouldn't be in at all and the value is not relevant to the test.

[tobyhs]

a few more comments, otherwise lgtm

[tobyhs]

'Command' (capital C) to match the mongo gem

[tobyhs]

The tests on lines 185-192 no longer provide much value, so I would remove them.

[tobyhs]

leave_mongo_breadcrumb (spelling)

[tobyhs]

This doc string is no longer accurate; I would replace line 203 with line 212 as the input on line 212 recurses and remove this block (so the "is recursive and iterative for array values" test uses [1, [2, [3]]])

[tobyhs]

lgtm