Improvements to how distribution new works about human-essentials HOT 6 CLOSED

We are going to roll this into the packs-8 task which is part of our event this weekend! Thanks for taking an initial look at it @danielabar.

from human-essentials.

re: User selects an Item but does not enter a quantity, then the entire line item is skipped:

I believe this behaviour is due to the use of || rather than && in the reject_if option of accepts_nested_attributes_for :line_items in the Itemizable concern, code:

accepts_nested_attributes_for :line_items,
                                  allow_destroy: true,
                                  reject_if: proc { |l| l[:item_id].blank? || l[:quantity].blank? }

This says if either item id (i.e. a selected item) or quantity are blank, then do not associate the line item with this Itemizable (Donation, Distribution, etc).

Changing the || to && will change this behaviour to only skip associating the line item if both of these are blank. But if either one is provided, then it will associate, and run the line items validation. (found this when investigating the Safari quantity issue).

Note that since Itemizable is used in several entities, changing this behaviour in the concern will affect affect all of these models:

app/models/adjustment.rb:
  21:   include Itemizable

app/models/audit.rb:
  22:   include Itemizable

app/models/distribution.rb:
  32:   include Itemizable

app/models/donation.rb:
  36:   include Itemizable

app/models/kit_allocation.rb:
  15:   include Itemizable

app/models/kit.rb:
  16:   include Itemizable

app/models/purchase.rb:
  29:   include Itemizable

app/models/transfer.rb:
  20:   include Itemizable

This will also require a number of system tests to be updated to now expect validation errors (or fill in a quantity). Because there are some tests that do not provide a quantity, but if its one of the models that automatically have the first item selected, currently its allowed to be saved, but with this change, it will return a validation error unless quantity is also selected.

from human-essentials.

I could pick this up if it hasn't been assigned yet.

from human-essentials.

@danielabar It's been identified as a blocker for some of the issues for the event at the end of the month -- just checking your confidence in getting it addressed in that timeframe -- otherwise we'll slot it for the event. (don't know if you're going)

from human-essentials.

Not sure if I'd be able to finish it in the next week or so, (I won't be attending the event). If its needed in a hurry, might be better to assign to someone who has more time this week.

from human-essentials.

@awwaiid, @scooter-dangle Do we want to keep this as a separate issue to be addressed at or before the event, or just roll it in to "PACKS #8"?

from human-essentials.