blueprinter icon indicating copy to clipboard operation
blueprinter copied to clipboard

[bug] :view key being deleted from options hash by #render

Open ryanmccarthypdx opened this issue 1 year ago • 2 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is this a regression?

  • [ ] Yes, this used to work before

Current Behavior

It seems that the process of rendering is deleting the :view key from the options hash. This breaks batching as only the first batch can be rendered correctly with subsequent ones falling back to the default view.

Expected Behavior

Options k-v pairs should persist through to subsequent batch renders.

Steps To Reproduce

Given this blueprint:

class FooBlueprint < ApplicationBlueprint
  identifier :id

  view :with_bar_id do
    field :bar_id
  end
end

This code:

options = { a_key: 'a value', view: :with_bar_id }
prerendered_foos = []
Foo.limit(5).in_batches(of: 2) do |batch|
  prerendered_foos += FooBlueprint.render_as_hash(batch, options)
end

generates this as prerendered_foos:

[
  {:id=>1, :bar_id=>101},
  {:id=>2, :bar_id=>102},
  {:id=>3},
  {:id=>4},
  {:id=>5}
]

The rendering of the first batch deletes the view option and the subsequent batches fall back to the default identifier-only view.

The workaround I am currently using is to re-add the view to the options hash after every batch, but this is unsatisfactory.

Environment

- OS: OSX 14.4.1
- Browser Name and version: N/A
- Ruby Version: 3.2.4

Anything else?

No response

ryanmccarthypdx avatar Aug 08 '24 20:08 ryanmccarthypdx

FWIW this looks like a good candidate to me, although it's five years old. But if your options hash is newly being shared across multiple renders, it seems a likely cause.

jhollinger avatar Aug 08 '24 20:08 jhollinger

Ah, apologies, I was mistaken that this was working before; I discovered this during a refactor and I got my wires crossed. The code that @jhollinger references above is indeed the issue, so this is perhaps more of a 'feature request'.

I have this draft PR (currently without tests) to start the conversation about what a possible 'fix' for this could look like if we agree this is a good idea. I'm not really sure why the existing uses delete but perhaps someone has a good reason.

I have also edited the above to better illustrate this issue.

ryanmccarthypdx avatar Aug 09 '24 14:08 ryanmccarthypdx