human-essentials icon indicating copy to clipboard operation
human-essentials copied to clipboard

[BUG] A bank users are unable to make a edit/correction to a purchase, donations, or distributions locally

Open edwinthinks opened this issue 3 years ago • 8 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Current Behavior

Attempting to make a correction on a distribution, donation, or purchase will result in a 500 error that looks like this:

undefined local variable or method `id' for {"item_id"=>4828, "name"=>"Wipes (Baby)", "quantity"=>1000, "active"=>true}:ActiveSupport::HashWithIndifferentAccess

Image

Expected Behavior

I expect that the editing works without the error.

Steps To Reproduce

In local (development), go to a purchase, donation, or distribution and attempt to make a edit or correction. It would result in the error mentioned.

Environment

- OS:  macOS 12.4

Anything else?


[email protected] reported that they are unable to edit a purchase. I tried it out and got this error:

undefined local variable or method `id' for {"item_id"=>4828, "name"=>"Wipes (Baby)", "quantity"=>1000, "active"=>true}:ActiveSupport::HashWithIndifferentAccess

[email protected] also reported the same issue with donations:

undefined local variable or method `id' for {"item_id"=>406, "name"=>"Kids (Newborn)", "quantity"=>175, "active"=>true}:ActiveSupport::HashWithIndifferentAccess

edwinthinks avatar Jul 13 '22 00:07 edwinthinks

I don't see this in production bug monitoring tools so I'd like to see if someone can verify if anyone else is experiencing this issue. It might be therefore a bug that's local and not yet shipped to production (which is still at https://github.com/rubyforgood/human-essentials/commit/93fac01b9fb2cb1ba7f7492368f9a1a6e4818a6f since the discovery this)

edwinthinks avatar Jul 13 '22 00:07 edwinthinks

It looks like its not happening in production or staging. I wonder what is it about my local environment that is causing this, looking forward to see if anyone else incur this same issue. @dorner you get the same thing?

edwinthinks avatar Jul 13 '22 01:07 edwinthinks

Looks like it's being caused by Bullet. :) image

dorner avatar Jul 14 '22 13:07 dorner

@dorner thank goodness bullet isn't on in deployed environments! Wanna take a swing at fixing this?

edwinthinks avatar Jul 14 '22 14:07 edwinthinks

I'm taking it 🚀

italomatos avatar Jul 14 '22 21:07 italomatos

I'm getting the same thing when I'm trying to create a kit on my local. FWIW.

cielf avatar Jul 18 '22 18:07 cielf

This issue has been inactive for 246 hours (10.25 days) and will be automatically unassigned after 114 more hours (4.75 days).

github-actions[bot] avatar Jul 29 '22 00:07 github-actions[bot]

This issue has been inactive for 366 hours (15.25 days) and is past the limit of 360 hours (15.00 days) so is being unassigned.

github-actions[bot] avatar Aug 03 '22 00:08 github-actions[bot]

So it looks like the issue is here: https://github.com/flyerhzm/bullet/blob/master/lib/bullet/detector/n_plus_one_query.rb#L36

combined with this: https://github.com/rubyforgood/human-essentials/blob/main/app/models/concerns/itemizable.rb#L92

Itemizable defines a to_a method which returns a list of items. I don't know why it behaves this way since to_a is a "special" method used by Ruby to allow a class to define a method that returns an array. For example, the following:

{foo: "bar"}.to_a

returns [[:foo, "bar"]].

Apparently @seanmarcia loves them for it though 😛 https://github.com/rubyforgood/human-essentials/pull/718#discussion_r284282261

Long story short we have a few options here:

  1. Figure out all the places where to_a is used in our code and rip them out. Unfortunately this is nigh-on impossible, since to_a is used implicitly, such as in exactly this case.
  2. Add a PR to bullet changing the Array call to Array.wrap, which only uses to_ary and does not drop down to to_a. I think that's really what's meant to happen here in any case - I don't think Bullet really wants us to call a transformation function.

For now I'll add in the PR and see what the response is.

dorner avatar Aug 12 '22 20:08 dorner

PR has been created! https://github.com/flyerhzm/bullet/pull/621

dorner avatar Aug 12 '22 20:08 dorner