3988 Omit inactive items from distributions#new dropdown

[rowenwillabus]

Description

• This change resolves #3988 where inactive items were displayed when creating a new distribution.
• An existing method was used to display the active items

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

• Created a system test to verify the changes.
• Changes also verified manually

To reproduce

1. Sign in as org_admin
2. Go into Inventory | Inventory Items and delete Adult Large/XL
3. Go to Distributions
4. New Distribution
5. Fill in the storage location
6. Check list of items: Adult Large/XL will be omitted.

[dorner]

@rowenwillabus this looks good! Ideally we'd also have this work when editing a distribution and adding a new item to it (i.e. it shouldn't remove any existing inactive items, but you shouldn't be able to select new items for the same distribution that are inactive). @cielf does that sound right?

Were you able to verify for the other itemizables (donations, transfers, audits, purchases) that they don't currently show inactive items?

[rowenwillabus]

@dorner The gotcha regarding the edit was mentioned by @cielf: https://github.com/rubyforgood/human-essentials/issues/3988#issuecomment-1869000531 Initially, I shared the same train of thought (editing a distribution, to add a new item, shouldn't allow an inactive item). However, that'll punish users for making a mistake.

Scenario:

1. There is a distribution of two items Adult Briefs (Large/X-Large) and Bed Pads (Cloth)
2. Organization user creates a distribution and mistakenly adds 1 item

• the error is realized two days later, both items are now inactive *

The Organization user will then be prevented from making the correction.

I'm happy to make the change, do let me know.

[cielf]

@dorner -- That does sound right. Shouldn't be able to add inactive items.

We do need to consider the issue of what happens if someone reduces the amount distributed on an inactive item, whether it is on a line-item basis or a distribution reclaim.

One possibility is that we should display an error, and make the user make the item active to do the change (though only if they are changing the value or reclaiming). It is going to be a uncommon case, I think.

[cielf]

And, because it's a pretty uncommon case, I don't really mind making the user work around the situation. (1571 out of 11154 items are inactive, but once an item is inactive, it usually stays that way.)

[rowenwillabus]

@cielf @dorner The issue stated "inactive" with an explanation on how to make an item inactive. Should we expand that to omit inactive (by status change) AND items with no inventory (zero count)?

If you try to distribute an item that has 0 in inventory, it displays Sorry, we weren't able to save the distribution. Validation failed: Inventory 1T Diapers is not available at this storage location. My thought is to remove such from the dropdown.

Do let me know. I'll wrap this up tomorrow.

[cielf]

What would the effect of that be on a case where we are creating a distribution from a request that has an item that has 0 in inventory? Will they still initially see the item (that has 0)?

[rowenwillabus]

They wouldn't see that. They would only see items that can be added to the distribution. If they pick an item without any inventory and attempt to move forward, the validation blocks them. My proposal is to only show paths that can be travelled.

See screencast of what happens now: Screencast from 12-31-2023 06:45:25 AM.webm

edit I haven't fully explored the UX of creating a distribution from a request. Permit me an hour to respond to that.

[rowenwillabus]

What would the effect of that be on a case where we are creating a distribution from a request that has an item that has 0 in inventory? Will they still initially see the item (that has 0)? - @cielf

No. They wouldn't. The item will not be displayed in the list. This is the same effect as a user requesting an item that then becomes inactive. However, I do see the utility in leaving it to be displayed as the user switches between storage locations. Whichever way, I believe it the new and edit experience should be similar, whether the distribution is from a request or not.

You have more context, happy to solve any way you suggest.

[dorner]

@rowenwillabus - it's a great idea, but I'd actually like to put a kibosh on any inspection of current inventory for these pages. Especially for the edit one - you don't want to know if there is inventory now, but if there was inventory at the time of distribution. We don't have a good way of doing this right now. We are in the process of reimplementing inventory entirely using event sourcing, so it will be handled on submit once that's done.

[rowenwillabus]

@dorner Thanks for mentioning that as I went down a rabbit hole and found some ways in which we can improve the process. E.g a request may have multiple distributions as all items requested may not come from a single storage location. Please loop me in on that work, happy to help.

I'll keep this PR focused on the inactive items affecting the new and edit methods. Thank you both for your time. Will submit changes shortly to close this.

[cielf]

"E.g a request may have multiple distributions as all items requested may not come from a single storage location" In practice, that's not a thing I've seen happening. And if it did happen, you would just create another distribution, not linked to the request, for the second storage location. It's a rare enough case that having that as a workaround is good.

[cielf]

@rowenwillabus Hi Rowen - what's the status on this PR? Are you able to move forward with it?

[dorner]

Going to close this for now. Feel free to open a new one if relevant.