Improvements to how distribution new works

[cielf]

Summary

New (and edit) distribution works in some ways that are not consistent with the other itemizables. This will bring it into line.

Why

Consistency. Also blocking some of our work with "packs" for the RubyforGood2024 event

Details

When working with items in distribution new or edit: Currently if you don't have a quantity, but you do have an item selected, it just skips those when it saves. It should, instead, give an error. "You must have a quantity if you have selected an item. Either provide a quantity for remove the item selected: [ Item name ]

Note: the quantity can be zero.

Currently We default the added item to be the first item in the list. Instead, we should make them "Choose an Item", like we do in donations.

Criteria for completion

• [ ] Functionality as above
• [ ] Tests modified or added to support the above behaviour rather than the current behaviour

[danielabar]

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.

[danielabar]

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

[cielf]

@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)

[danielabar]

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.

[cielf]

@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"?

[awwaiid]

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.