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

Units showing as 'none' on all pending or ready orders; showing as 'shipped' on all shipped orders #4582

Closed
tschumilas opened this issue Dec 14, 2019 · 8 comments · Fixed by #8111
Assignees
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. FR Selected to be done by the French active instance

Comments

@tschumilas
Copy link

tschumilas commented Dec 14, 2019

Description

The units on the order editing page are showing as 'none' on all new and all previous orders for 'pending' and 'ready' orders. The units are showing as 'shipped' on all 'shipped' orders (versus showing the units. This is happening for products entered as item or products entered by weight.
It is happening for new orders and ALL previous orders. I've tested, and it is occuring on OFN-CAN and on OFN-US. Might this be related to !#4581 as they started at the same time (first noticed Dec 14)

Expected Behavior

the units should show on the orders to facilitate editing

Actual Behaviour

units are showing as 'none'. This is happening for all products and all new and old orders. The item quantity CAN be edited and re-calcuation works fine. The units are showing correctly in all reports and on the invoice.

Steps to Reproduce

  1. go into OFN-CAN as super admin
  2. go into the orders editing tab
  3. see that order units are all 'none'

Animated Gif/Screenshot

edit order screen does not show units

Workaround

look to the variant description to know what the units are

Severity

s2
(orders can still be edited)

bug-s1: a critical feature is broken: checkout, payments, signup, login
bug-s2: a non-critical feature is broken, no workaround
bug-s3: a feature is broken but there is a workaround
bug-s4: it's annoying, but you can use it
bug-s5: we can live with it, only a few users impacted

https://github.com/openfoodfoundation/openfoodnetwork/wiki/Bug-severity
-->

Your Environment

  • Version used:
  • Browser name and version:
  • Operating System and version (desktop or mobile):

Possible Fix

@tschumilas tschumilas changed the title units showing as 'none' on all order edit page for all orders units showing as 'none' on all pending or ready orders; showing as 'shipped' on all shipped orders Dec 14, 2019
@tschumilas tschumilas added the bug-s2 The bug is affecting any of the non-critical features described in S1 and there is no workaround. label Dec 14, 2019
@luisramos0
Copy link
Contributor

hello Therese, thanks for raising the issue!
I think this is a bug introduced here: #4385

Although this is ugly, users can preform all actions in the app. I think this is an S3.

@tschumilas
Copy link
Author

Hmmm - wonder why I never saw it before on the orders page - the bug was reported back in October, I and other users here have edited many orders without seeing this. Oh - maybe its to do with just picking up translations in transifex. Damn. so I"ve made it s3 - but its going in my paperclip priorities.

@tschumilas tschumilas 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 Dec 14, 2019
@sigmundpetersen sigmundpetersen changed the title units showing as 'none' on all pending or ready orders; showing as 'shipped' on all shipped orders Units showing as 'none' on all pending or ready orders; showing as 'shipped' on all shipped orders Feb 10, 2020
@kirstenalarsen
Copy link
Contributor

wow i never noticed this, annoying!! putting on my list to consider for votes too

@RachL RachL added the FR Selected to be done by the French active instance label Aug 4, 2021
@RachL
Copy link
Contributor

RachL commented Aug 4, 2021

Ok I'm selecting this as FR's vote (it's more annoying than #7627 because visible during demos).

@openfoodfoundation/train-drivers-product-owners do you agree that "none" should not only be replaced by unit, but a combo of unit value and unit type?

If too complex I'm wondering if we shouldn't consider just removing the "x none/shipped".

@jibees jibees self-assigned this Aug 26, 2021
@jibees
Copy link
Contributor

jibees commented Aug 27, 2021

Reading the code:

- item.states.each do |state,count|
  = "#{count} x #{t(state.humanize.downcase, scope: [:spree, :shipment_states], default: [:missing, "none"])}"

From what I saw, sometimes the item.state could be (in most of cases actually) on_hand, but there is no translated text for spree.shipment_states.on_hand:

shipment_states:
      backorder: backorder
      partial: partial
      pending: pending
      ready: ready
      shipped: shipped
      canceled: canceled

That's why we display none (the default value, if any i18nized working could be found)

There are, I think, two ways to fix the issue:

  • I can add a i18n key for on_hand value state (which translation?).
  • I can remove the whole 'x none/shipped' string.

@jaycmb
Copy link
Collaborator

jaycmb commented Aug 27, 2021

@RachL

do you agree that "none" should not only be replaced by unit, but a combo of unit value and unit type
You mean from 2 x none to 2 x 0.5 kg with 0.5= unit value and kg=unit type,
instead of only to 2 x kg , right?

If so, agree :) Adding only unit would not help.

@jibees If I understand your suggestion correctly, we would end up with 2 x on_hand by simply adding a translation, correct?
As mentioned above, that would not solve the problem.
Since the column label is quantity, the user would expect to see an actual quantity.

If getting to
a) the optimal 2 x 0.5 kg with 0.5= unit value and kg=unit type is too complicated,
b) looking at the code we could have a simplification and just provide the count, 2?

  = "#{count}"

@jibees
Copy link
Contributor

jibees commented Aug 27, 2021

@jaycmb

If I understand your suggestion correctly, we would end up with 2 x on_hand by simply adding a translation, correct?

Yes, that's one of my two initials proposals.

As mentioned above, that would not solve the problem. Since the column label is quantity, the user would expect to see an actual quantity.

Ok.

a) the optimal 2 x 0.5 kg with 0.5= unit value and kg=unit type is too complicated,

the column 'quantity' is intended to be separated according to its "state" (see shipment_states child for the list of expected states ; something like 1 x shipped, 2 x on hand). I guess this solution is harder than the b) ...

b) looking at the code we could have a simplification and just provide the count, 2?
= "#{count}"

Would you like to create the PR? ;)

@RachL
Copy link
Contributor

RachL commented Aug 27, 2021

Yeah let's just display the count, no one will miss info here ,and new users will be less stressed about this confusing "none".

Go @jibees 🎉

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. FR Selected to be done by the French active instance
Projects
None yet
6 participants