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

Fix behaviour with errors on Distribution #4613

Open
3 tasks
cielf opened this issue Aug 25, 2024 · 5 comments
Open
3 tasks

Fix behaviour with errors on Distribution #4613

cielf opened this issue Aug 25, 2024 · 5 comments

Comments

@cielf
Copy link
Collaborator

cielf commented Aug 25, 2024

Summary

If you have an error (too many of an item) on saving a new distribution, the number of items in inventory for each item is disappearing, and only one error is showing even if you have multiple overages. Fix it!

Why?

Speed up process of dealing with errors. Give enough information for banks to fix all the errors in one go.

Details

To recreate this issue:
sign in as [email protected]
Requests
View a request
Click Fulfill
enter a distribution that has two items that have a quantity that is more than is in the inventory for your chosen storage location.
Save.

You should see errors for both items (you'lll only see one)
The number in brackets after the item (which is the inventory level for that item/storage location) has disappeared -- it should still be there.

Criteria for completion

  • errors for all item overages are shown at the same time
  • the inventory level is shown for items after save
  • tests to confirm the above behaviour
@Naraveni
Copy link
Contributor

I would like to take it

@cielf
Copy link
Collaborator Author

cielf commented Aug 29, 2024

Please do!

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

Naraveni commented Sep 9, 2024

Hey, I would like suggestions from a developer on this.

def handle_inventory_event(payload, inventory, validate: true, previous_event: nil) payload.items.each do |line_item| quantity = line_item.quantity if previous_event previous_item = previous_event.data.items.find { |i| i.same_item?(line_item) } quantity -= previous_item.quantity if previous_item end inventory.move_item(item_id: line_item.item_id, quantity: quantity, from_location: line_item.from_storage_location, to_location: line_item.to_storage_location, validate: validate) end # remove the quantity from any items that are now missing previous_event&.data&.items&.each do |previous_item| new_item = payload.items.find { |i| i.same_item?(previous_item) } if new_item.nil? inventory.move_item(item_id: previous_item.item_id, quantity: previous_item.quantity, from_location: previous_item.to_storage_location, to_location: previous_item.from_storage_location, validate: validate) end end

This is the function where we can handle multiple errors of Inventory Error if raised if something regarding quantity goes wrong

on DonationEvent, DistributionEvent, AdjustmentEvent, PurchaseEvent, TransferEvent, DistributionDestroyEvent, DonationDestroyEvent, PurchaseDestroyEvent, TransferDestroyEvent, UpdateExistingEvent do |event, inventory, validate: false, previous_event: nil| handle_inventory_event(event.data, inventory, validate: validate, previous_event: previous_event) rescue InventoryError => e e.event = event raise e end

this is where the raise error is caught.

As per the issues I thought of collecting all the error through a rescue block the combining the messages and throwing standard Error. But doing that so may bring more error as few of the events are utilizing the attributes of Inventory Error to validate and raise the error.

This may take more changes or bring out more issues.

May be I am over thinking can anyone suggest if It can be dont in a more easy way.

Thank You

@cielf @

@cielf
Copy link
Collaborator Author

cielf commented Sep 10, 2024

@dorner or @awwaiid Can you chime in on this?

@dorner
Copy link
Collaborator

dorner commented Sep 12, 2024

I think rescuing those errors per event in handle_inventory_event and collating them together might be fine. Ideally you'd keep track of the first error and just keep appending to its message with any subsequent errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants