Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should be able to select items for audit that have 0 inventory. #4594

Closed
2 tasks
cielf opened this issue Aug 11, 2024 · 22 comments · Fixed by #4833
Closed
2 tasks

Should be able to select items for audit that have 0 inventory. #4594

cielf opened this issue Aug 11, 2024 · 22 comments · Fixed by #4833

Comments

@cielf
Copy link
Collaborator

cielf commented Aug 11, 2024

Summary

You can only select items for auditing that already have inventory in the chosen storage location. You should be able to select any active item.

Why?

If someone is doing an audit, and has stock of items that have not been entered into system for that storage location, they should be able to add them in at audit time. Also, this bug effectively precludes someone using the audit feature to set up their stock levels.

Details

Sign in as org_admin1@example.com. Choose Inventory, Items & Inventory. Enter a new item. Then, Choose Audit, pick a storage location, then try to pick the item you just entered. You can't. You should be able to.

Criteria for completion

  • Audit item drop downs include all active items, even if the storage location has no inventory of them
  • Tests to confirm that.
@auliafaizahr
Copy link
Contributor

I'd like to give this issue a try :)

@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Aug 29, 2024
@cielf
Copy link
Collaborator Author

cielf commented Aug 29, 2024

Please do -- fair warning, I think this is going to be quite a bit more challenging than the first one.

@auliafaizahr
Copy link
Contributor

thankyou for reminding, i'll back to you once found something challenging :)

@auliafaizahr
Copy link
Contributor

hi @cielf I’m following the instructions to add a new item and then audit it, but I’m a bit confused. There are several tabs on the "Items & Inventory" page (screenshot). Could you let me know which tab I should use to add the item? Thanks!

image

@cielf
Copy link
Collaborator Author

cielf commented Sep 2, 2024

hi @cielf I’m following the instructions to add a new item and then audit it, but I’m a bit confused. There are several tabs on the "Items & Inventory" page (screenshot). Could you let me know which tab I should use to add the item? Thanks!

image

Click the "New Item" button on the Item list to bring up the New Item form.

@cielf
Copy link
Collaborator Author

cielf commented Sep 3, 2024

Ok -- so we found out that a new item in the above case actually works. But this sequence doesn't
log in as org_Admin2@example.com -- this is a brand new organization
add a storage location
Then Audit, new Audit, choose a storage location and try to add an item.

@cielf
Copy link
Collaborator Author

cielf commented Sep 3, 2024

The desired result is that when we do an audit, we can add entries for any active items, whenever they were added.
We want to enable using the audit as a valid way to set up the initial inventory levels — which they can’t yet.
When the bank is set up as a brand new bank, we initialize a list of items, which are standard sizes of diapers, etc. They might use that list of items as is, or add to it before setting up their initial inventory...
I think (without delving into the code) that what may be happening now is that the list of items available in the audit is the list of items that have some association with the storage location. We don’t add all the items to each storage location initially because not everyone actually uses the entire list — but if they are adding a new item, they are probably going to be using it!
However, for audit purposes, we want them to be able to select any active item.
Hope that helps!

Copy link
Contributor

github-actions bot commented Oct 4, 2024

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

@cielf cielf added Help Wanted Groomed + open to all! and removed stale labels Oct 11, 2024
@danielabar danielabar self-assigned this Nov 30, 2024
@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Nov 30, 2024
@danielabar
Copy link
Collaborator

I'm reading up on Audits to understand the workflow. The docs say:

When you do an inventory count, you’ll want to set the inventory in each Storage Location to the physical count. That’s what the Audits function is for.

Does this mean that the person doing the audit is physically present at the storage location, and going over each Item in the location, counting up how many there are, and then entering that number into the system?

But based on the description of this issue: Is it possible that the person conducting the audit, has possession of some items, but they're not yet in a storage location? If so, where are these items? Is the person planning to place them in the storage location soon after audit/counting them?

I'm trying to match up my intuition of what an audit is with what the actual process might be. I'm picturing someone in a warehouse with their mobile browser at the Audit screen, going over all the items on the shelves, counting them, then entering this number into the system.

But if its possible for items to be counted but not yet in a storage location, could this cause confusion with someone looking at the audit numbers and assuming this means those items are available in storage?

@coalest
Copy link
Collaborator

coalest commented Nov 30, 2024

@danielabar I could be wrong, but I think of the audit as a way to get the actual current quantity of items in a storage location, not necessarily what is supposed to be there based on previous donations/distributions/etc. Because who knows if things have come or gone either due to an accounting mistakes or something.

So I think finalizing an audit will then correct the quantities of items in that storage location, so the database is once again in sync with reality.

@coalest
Copy link
Collaborator

coalest commented Nov 30, 2024

Btw I have been working on a PR related to these item dropdown options so I'm familiar with this part of the codebase. I think the fix for this could be as simple as this patch:

diff --git a/app/views/audits/_form.html.erb b/app/views/audits/_form.html.erb
index d9014cae..8cb798a5 100644
--- a/app/views/audits/_form.html.erb
+++ b/app/views/audits/_form.html.erb
@@ -14,7 +14,7 @@
               </div>
               <div class="box-body">
                 <%= simple_form_for @audit, data: { controller: "form-input" }, html: {class: "storage-location-required"} do |f| %>
-                  <%= render partial: "storage_locations/source", object: f, locals: { label: "Storage location", error: "What storage location are you auditing?" } %>
+                  <%= render partial: "storage_locations/source", object: f, locals: { label: "Storage location", error: "What storage location are you auditing?", include_omitted_items: true } %>
                   <fieldset style="margin-bottom: 2rem;">
                     <legend>Items in this audit</legend>
                     <div id="audit_line_items" class="line-item-fields" data-capture-barcode="true">

@cielf
Copy link
Collaborator Author

cielf commented Nov 30, 2024

@danielabar -- @coalest is right -- the purpose of entering an audit, in this system, is to record the actual current physical count of items in a storage location. It sort of corresponds to "doing inventory" in a retail setting.

In an ideal world, it would match up to the sum of the donations and purchases minus the distributions. But that's not how reality works g.

The current version of the audit only allows you to pick items that have already been added to the storage location (i.e. that have had purchases, donations, transfers to, or inventory adjustments).

We want the users to be able to pick any of the bank's active items. (If they find inventory of inactive items, then they'd need to make them active again to enter them in the audit.) This will let new banks use the audit to record an initial physical count of what is in a storage item to set things up --- rather than using the workaround of recording a donation of their current inventory. Which will make for better reporting.

@cielf
Copy link
Collaborator Author

cielf commented Nov 30, 2024

"Is it possible that the person conducting the audit, has possession of some items, but they're not yet in a storage location? If so, where are these items? Is the person planning to place them in the storage location soon after audit/counting them?"

To answer this question specifically -- nope, it's a record of the physical count of what is in the location. Which, due to human factors may be different than the sum of all the inventory-based transactions. For instance, someone may neglect to enter a distribution, or a donation. The audit brings the numbers back in line.

@danielabar
Copy link
Collaborator

danielabar commented Dec 2, 2024

@coalest Thanks for the tip on include_omitted_items: true in the audit form partial.

That does allow the user to select items for audit that have no current inventory, but they still don't show up in the Save Progress/View screen or in any view after audit is finalized.

I think the issue is not so much that inventory is 0 (although that's how it shows in UI) but rather that inventory_item records (and association to a storage location) don't exist at all.

Might be fixable by adding include_omitted: true in audits controller:

# app/controllers/audits_controller.rb
  def show
    @items = View::Inventory.items_for_location(@audit.storage_location, include_omitted: true)
  end

Will continue investigating...

@cielf
Copy link
Collaborator Author

cielf commented Dec 2, 2024

I think that if they are still zero, then not showing up in the save progress screen would be ok, but if you have entered a value, then they need to.

It might be ok, and probably simpler, to just have all the organization's active items show up in that screen, the same as the drop down. We can change it if we get complaints.

I'm not sure (haven't looked) if the "include_omitted" includes inactive -- I expect it does, so we might need something else for that selection.

@dorner
Copy link
Collaborator

dorner commented Dec 2, 2024

@danielabar just to confirm - inventory_items themselves no longer exist at all. We've replaced them with inventory calculated via events.

@danielabar
Copy link
Collaborator

@dorner Good to know, thanks! In the code there's still InventoryItem model, table, and Item model has_many :inventory_items, but maybe that's for a future task to cleanup if unused.

@danielabar
Copy link
Collaborator

@cielf I have something that appears to be working as per description, although not sure about active vs inactive items: https://github.com/danielabar/human-essentials/tree/4594-audit-items-zero-inventory

Could you try it out if you have a chance?

@cielf
Copy link
Collaborator Author

cielf commented Dec 3, 2024

Thanks, Daniela

I took a partial audit of a brand new item through to the end of the audit process.

It is showing inactive items in the dropdown (and shouldn't).
It isn't showing them in the views after you save progress, confirm audit, or finalize audit, which is right.

For convenience, In the seed, Active Briefs (Large/XL) is an inactive item. Conveniently, it's also the first item alphabetically.

@danielabar
Copy link
Collaborator

danielabar commented Dec 6, 2024

@cielf Confirmed that the inactive product is Adult Briefs (Large/X-Large):

items = Item.select(:id, :name, :active).where(active: false)
# two items returned, its the same named item but for different organizations
[
  #<Item:0x000000011ff958d8 id: 48, name: "Adult Briefs (Large/X-Large)", active: false>, 
  #<Item:0x000000011ff95798 id: 1, name: "Adult Briefs (Large/X-Large)", active: false>
]

Here's the interaction I observed when trying to select an inactive item for audit:

The inactive item(s) only shows in the dropdown when no Storage location has been selected:
image

But then it won't allow adding any more items:
image

Trying to save progress at that point results in an error "Storage location must exist":
image

Then when user selects a storage location, the inactive item(s) disappear from the dropdown:
image

So user won't be able to actually audit an inactive item, and if they fill out the form in sequence, first selecting a storage location, they'll never see inactive items in the dropdown.

Is that ok?

OR

Should it be modified such that the dropdown doesn't populate until after a storage location is selected? (maybe there are other forms that work that way).

Also: It is possible to filter out the inactive items from the initial rendering of that dropdown. In this way, the user never sees ineligible items, even if they do start using the item dropdown before selecting a storage location. I added a commit to my branch to do this.

@cielf
Copy link
Collaborator Author

cielf commented Dec 6, 2024

I think that filtering out the inactive items before the initial rendering is the right way to go -- principle of least surprise.

danielabar added a commit to danielabar/human-essentials that referenced this issue Dec 8, 2024
danielabar added a commit to danielabar/human-essentials that referenced this issue Dec 8, 2024
danielabar added a commit to danielabar/human-essentials that referenced this issue Dec 11, 2024
dorner pushed a commit that referenced this issue Dec 13, 2024
* [#4594] allow zero inventory items to be selected for audit and filter out inactive items

* [#4594] replace audit system test with request test to verify item list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants