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

Error 422 when placing backoffice orders with more items than those available on hand #5784

Open
filipefurtad0 opened this issue Jul 17, 2020 · 9 comments
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround.

Comments

@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Jul 17, 2020

Description

One of the changes introduced by PR #5406 is to prevent stock levels from reaching negative values through backoffice orders. After these changes, creating an order in the backoffice with more items than those in stock is not possible; attempting generates the error on Bugsnag:
https://app.bugsnag.com/open-food-france/open-food-france-prod/errors/5f11cd5d9f0dd90017a69e3d?event_id=5f11cd5d005b6e2ca49f0000&i=sk&m=nw

See also pic below.

Expected Behavior

Placing the order should succeed, but with reduced quantity to the max. available stock.

Actual Behaviour

Attempting to complete a backoffice order with more items than those available in stock fails and displays the message "Error saving payment".

Steps to Reproduce

Opened two browser sessions, both logged in as admin.

Session A: Set the stock level of a product to limited stock with e.g. 4.
Session B: Create an order, Add less than total stock left to cart, e.g. 3.
Session A: Update the stock level of the product to be less than the quantity in cart, e.g. 2.
Session B: Attempt to complete the order: this will fail and display the message "Error saving payment".

Animated Gif/Screenshot

image.png

Workaround

After seeing the error, refresh the page and insert the a nr. of items smaller or equal to that available in stock.

Severity

bug-s4

Your Environment

  • Version used: v.3.1.0
  • Browser name and version: Chromium 83
  • Operating System and version (desktop or mobile): Desktop/GalliumOS

Possible Fix

@filipefurtad0 filipefurtad0 added the bug-s4 The bug is annoying, but doesn't prevent from using the platform. Not so many users are impacted. label Jul 17, 2020
@filipefurtad0 filipefurtad0 added bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. and removed bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. bug-s4 The bug is annoying, but doesn't prevent from using the platform. Not so many users are impacted. labels Jul 17, 2020
@RachL
Copy link
Contributor

RachL commented Jul 17, 2020

@filipefurtad0 @luisramos0 I'm not sure I understand the expected behavior. Why would a manager be allowed to go higher than stock? Most of the users I talk to, don't know their stock info. So the actual behavior described here is something they are asking for.
Also, another workaround for the admin is to simply raise the stock level. So I really don't think this is an s2?

@filipefurtad0
Copy link
Contributor Author

Hi @RachL ,

After PR #5406 it is not possible to exceed stock with backoffice orders, this is one of the changes introduced. After this PR the actual behavior when attempting to exceed available stock in the BO is displaying a payment error message - not informative about stock limitation - and generating an error which turns out in bugsnag as well.

I agree it would be useful for users to know about stock limitations in the backoffice, wouldn't this be a feature request?

The error/bugnsag alert is introduced by the PR, so I would agree it should retain the original bug severity.

@RachL
Copy link
Contributor

RachL commented Jul 20, 2020

@filipefurtad0 ok so the error message is missleading, but there is still a workaround. I would leave it as s3. We can see how many request we have at support to see if this is something we need to upgrade in terms of severity. Yet when this happens, I'm still not sure we want the backoffice order to be successful no matter the stock. This will need some product decision.

@filipefurtad0
Copy link
Contributor Author

Sure, that's indeed a product decision and the message can also be improved, as well. It's the severity of introduced error I am unsure of. But I'm fine with downgrade for now and upgrade later if necessary 👍
Thank you!

@filipefurtad0 filipefurtad0 added bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. and removed bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. labels Jul 20, 2020
@luisramos0
Copy link
Contributor

ok, I just commented on #5300
I thought this was frontoffice, now I get it 👍
This is an S3 👍

I am not sure about the "Expected Behavior", maybe an acceptable solution would be to show a meaningful error message that would enable the user to go back and fix the order quantity in the "Order details" page.
This would probably be easier to do than fixing quantity: replicate it locally and find a good place to catch the exception and replace it with a user friendly stock level error message.

@filipefurtad0
Copy link
Contributor Author

Hi @luisramos0 @RachL ,

a short FYI note: When opening this issue I took the "Expected Behavior" as is, from 5406, please see Scenario 2 in the description of the PR. I thought the code was being designed in this way, so I kept it's original intention.

In any case, the solution you propose sounds good to me as well 👍

@RachL
Copy link
Contributor

RachL commented Aug 3, 2020

@filipefurtad0 shouldn't you be away chilling in the sun? :)

@luisramos0 meaningful error message sounds good 👍 Do you think we could display it easily when adding quantities on the order detail page rather than on the payment page?

@luisramos0
Copy link
Contributor

i am not sure @RachL a question for when a dev picks this up 👍

@filipefurtad0
Copy link
Contributor Author

Seems we're getting a 422 as well, if we resume orders which exceed the available stock, as reported here.

Orders---OFN-Administration.1.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround.
Projects
Status: All the things 💤
Development

No branches or pull requests

3 participants