Variant incorrectly displaying unit name not display as in shopfront and backoffice

[chezaorchard]

Description Product variants, where unit size=item, are displaying in all open shopfronts with only the unit name, not the full 'display as' which usually includes a number as well as descriptor.

Product variants, where unit size=item, are displaying in customer totals report with only the unit name, not the full 'display as'

Investigation:

• Affecting the backoffice (reports), and the customer shopfront experience
• Only where unit size=item
• Happening for accounts where sells = all OR sells = any
• Affecting customer totals reports, but pack by reports have the correct 'display as' in variant column
• Displaying incorrectly in backoffice orders, but displaying correctly on order confirmation email
• Issue appeared at some point after approx 10pm AEST December 1 (determined by download time of user where the report did include 'display as' but when run today it is only displaying 'unit name')

Tested with user IDs: 2840 1927 153 2246 639

Expected Behavior All product variants should display in shopfronts and reports with complete 'display as' value

Actual Behaviour Product variants are incorrectly displaying in shopfronts and some reports as 'unit name' only

Steps to Reproduce Navigate to open shop Scroll down to product variant where size=item Observe variant is incorrectly displaying with 'unit name' only

Log in as superadmin (User ID 2840) Run customer totals report for current order cycle Observe column G incorrectly displays 'unit name' only

Long in as customer (User ID 2840) Run customer totals report for current order cycle Observe column G incorrectly displays 'unit name' only

Check with User ID 2840 via phone - they downloaded the report yesterday evening (approx Australian time 10pm, Wed Dec) and the report did contact the correct 'display as' at that time.

Screenshot

Severity

bug-s2: a non-critical feature is broken, no workaround

Your Environment

• Browser name and version: Chrome Version 96.0.4664.55 (Official Build) (x86_64)
• Operating System and version (desktop or mobile): Desktop & Mobile

[lin-d-hop]

Is this related to #8500?

[RachL]

@lin-d-hop Judging by Chez comment on slack it seems #8500 introduced this problem :(

[lin-d-hop]

@apricot12 Can we assign to you in this case? @openfoodfoundation/testers Let's also make sure we take both cases into account during testing.

[apricot12]

@lin-d-hop Yes, I'll look at it immediately.

[jibees]

@openfoodfoundation/testers Let's also make sure we take both cases into account during testing.

Could we also create an automatic test for this?

[RonellaG]

@apricot12 @jibees @lin-d-hop @RachL We've started to have more hubs enquire about this issue. This is impacting their shopfront listings as well as their ability to generate packing reports. What is the status of this bug and how soon can it be resolved? Given it impacts our hubs sales and operations quite a lot can it please be labelled S1?

[chezaorchard]

User ID 286, a bulk food hub, are experiencing this issue with their current order cycle: "The pricing for e.g. coconut cream is $3.25/can if bought by the can or in 6's but $2.55/can if ordered in 12's. However the shopper will not realise how many cans they are buying as the number has now disappeared."

[apricot12]

@apricot12 @jibees @lin-d-hop @RachL We've started to have more hubs enquire about this issue. This is impacting their shopfront listings as well as their ability to generate packing reports. What is the status of this bug and how soon can it be resolved? Given it impacts our hubs sales and operations quite a lot can it please be labelled S1?

Hey, It should be up for code review by today. I'll try to speed it up as much as possible :)

[apricot12]

@chezaorchard @RonellaG I've just put up a PR that fixes this issue. Now it's just a matter of going through the review and testing process and getting it into this weeks release.

[Matt-Yorkley]

Hey :wave:, I've just been looking through this and the other related issues (#8495) and recent changes (#8500).

It's a tricky one! The way these different values are saved is really complex in terms of the datamodel, and the code that handles it is quite arcane (the VariantUnits::VariantAndLineItemNaming service). The tricky part is that even small changes in that service can potentially have lots of different effects all throughout the codebase, and it's almost impossible to predict what they'll be.

I think I would suggest maybe reverting the changes in #8500, and then trying to re-assess how to solve the original issue #8495 by making some changes in the view files that handle displaying products in the shopfront, eg:

• https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/views/shop/products/_summary.html.haml
• https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/assets/javascripts/templates/shop_variant.html.haml

[apricot12]

@Matt-Yorkley I was wondering the same thing. I wanted to try implementing some view logic to fix this, but I wasnt sure if it was the right thing to do. It seemed urgent too so I just went with a hacky fix.

[Matt-Yorkley]

I wasnt sure if it was the right thing to do.

Fair enough! I'm personally terrified of touching that service :sweat_smile: It's really difficult to work with.

[mkllnk]

It sounds like a good idea to replace its use bit by bit then. For another PR I went through the code just to find out which database fields it was using and it which order and that was an essay already.

I just reverted #8500 on au-prod because it's causing huge problems for our enterprises here. We should probably do that in master and then any further change needs to first add a spec for this issue (shopfront and reports were affected) and then find a better way of solving the original issue.

[filipefurtad0]

In principle this bug won't be occurring anymore after this weeks release, as it was introduced by #8500 - I'll leave it as a production test, as a double check.

[filipefurtad0]

Customer totals report displays the correct #display_as name, after reverting the aforementioned PR, closing: