openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.12k stars 724 forks source link

Price and Stock Override in Inventory not working #6386

Closed lbwright22 closed 4 years ago

lbwright22 commented 4 years ago

Description

Enterprise (Deeside Food Hub) uses the inventory to edit the price and stock levels of items supplied to them. Updating price and stock levels via the inventory for this enterprise is no longer working.

Expected Behavior

  1. Enterprise adds product from a supplier to their inventory.
  2. Enterprise manager changes the price and selects update.
  3. Price is changed.
  4. Enterprise manager changes stock from 'Use Producer Settings' to 'No'. They edit the stock level column.
  5. stock level is updated.

Actual Behaviour

  1. Enterprise adds product from a supplier to their inventory.
  2. Enterprise manager changes the price and selects update.
  3. Error message encountered. Price is not updated.
  4. Enterprise manager changes stock from 'Use Producer Settings' to 'No'. They edit the stock level column.
  5. Error message encountered. Stock level is not updated.

Steps to Reproduce

For Deeside Food Hub:

  1. Add item to an inventory
  2. Change the price of the product.
  3. Select update
  4. Observe the change is not saved and an error message comes up.
  5. Change the stock settings in inventory from 'Use Producer Settings' to 'No'. Update stock level.
  6. Observe error message and stock level is not updated. Unable to replicate on OFN UK demo hub.

Animated Gif/Screenshot

inventory

Workaround

Severity

bug S2 one user effected at the moment.

Your Environment

Possible Fix

lbwright22 commented 4 years ago

@lin-d-hop advised this is bug level S1.

Matt-Yorkley commented 4 years ago

Dev notes

I had a quick look at this one but there's not much to go on. It looks like the submitted form did not update the record in this case due to a 401 unauthorized response (see screenshot). So... at the time the form was submitted, the user was not deemed (via cancan) to have the permissions to update the variant override. The result is that there was no error that was actually thrown anywhere, and so no error logs to investigate, either on the server or in Bugsnag.

It seems like this could not be replicated elsewhere (with other user accounts), eg the bug is not universal.

I'm thinking this is maybe an obscure edge case. Possible explanations:

lbwright22 commented 4 years ago

Deeside Food Hub (producer hub) set up:

  1. Products do not need to be added to the inventory to be added to the shop front.
  2. Permissions exist (add to order cycle and add to inventory) between Deeside and their suppliers.
  3. Manager of Deeside is also the owner and/or on the management of many of her supplier's enterprises.
  4. Unable to edit the stock or price of products from any supplier to Deeside in Deeside's inventory: permissions error comes up as shown above.

testing different things:

  1. Can update price and stock levels in other hubs via inventory (OFN UK demo and Stroudco) without the inventory override being enforced in enterprise settings. Change Inventory setting to 'new products must be added to my inventory...' and back again to 'new products can be added to shopfront' to see if this resets things -> no change.

  2. removed permissions between From Bakery Lane and Deeside and then reinstated them. No change: error message still occurs and unable to update inventory.

  3. Removed manager of Deeside from managers list of From Bakery Lane. Made no change: same permissions error occurred and unable to update the inventory.

lbwright22 commented 4 years ago

Manager of Deeside has reported that her producers are having problems creating products today (they are encountering the slug 'We are sorry but something has gone wrong page'). She also reported that a new user encountered the same screen when trying to log in to OFN.
I have logged in as a producer of her hub and tested both scenarios but didn;t get the same screen

lin-d-hop commented 4 years ago

Slug on product creation is always #4959. Unrelated. Similarly new user will be unrelated.

lin-d-hop commented 4 years ago

@Matt-Yorkley Thanks for looking into this. Your notes above did not help resolve this issue. The problem persists and currently this user's business model and processes are broken because they cannot update inventory.

If this is an unsolveable edge case UK support needs to know so we can have a go at create a whole other enterprise and seeing if that resolves the problem. That or we tell the user to change how they run their business.

Would be great to know how to progress. Thanks team.

filipefurtad0 commented 4 years ago

I tried to reproduce this error in staging-UK. I don't think this is really it, but maybe related.

Having these permissions set:

image

And having added the product "Lettuce" to the Inventory of Distributor Melon Farm:

Session 1 - while editing Inventory page (Melon Farm) Session 2 - The Hacienda removes its "add products to inventory" permission Session 1 - Melon Farm edits Lettuce (price, stock, etc.). Clicking in Update will display the same error as the one shown on the issue above:

image

I've tried to combine this error with inventory import but found nothing else other than this edge case.

This 401 error did not show up on Bugsnag... Maybe an approach would be to tag this type of error on Bugsnag, so we could have more context?

andrewpbrett commented 4 years ago

This should now be resolved. I was able to reproduce the error and figured out that it was because there was a VariantOverride that was tied to the hub, but the hub did not have permissions to manage inventory for the Enterprise that the VariantOverride belonged to.

I set the permission_revoked_at field on the VariantOverride to Time.now and now the inventory is editable again.

My theory is that at some point the Enterprise granted permissions to the hub to manage its inventory and later revoked them. We have a before_destroy callback in EnterpriseRelationship that should revoke permissions for the VariantOverrides when this happens, but it looks like it didn't run here.

I looked at the logs on the UK server to see if I could find a request to delete the EnterpriseRelationship but unfortunately there aren't any params that are sent with a delete request besides the ID. I might have some time later to download an old copy of the database and try to confirm the theory.

lbwright22 commented 4 years ago

I tested on the OFN UK demo hub:

  1. Add permission between enterprise 'Demonstration' and 'OFN UK demo hub' for items to be added to OFN UK Demo hub inventory
  2. Change the price of the eggs from Demonstration in OFN UK Demo inventory. Press save.
  3. Remove the permissions between Demonstration and OFN UK Demo hub.
  4. View Demonstration's product = eggs. No override noted.
  5. Change the price of any other item in OFN UK demo hub- change is saved and no block.

It appears that the issue which resulted when permissions between Deeside Food Hub and The Deesidedly Tasty Company Ltd were revoked is not a wide spread bug, which is good :-)

Thanks Andy for your work on this