[Product import] Some products are duplicated instead of updated

[audez]

Description

In the admin dashboard, if you import a file to update products, it can create duplicates for some products. Weirdly it seems to only apply to products which unit type is ml (or mL) and quantity is between 688 and 703. It's the same behavior whether you first create the product from the dashboard or from an import file.

It impacts hubs using csv imports to update their products database: they don't understand why certain products are created again, and they have to manually delete the duplicates and manually update the original products.

The investigation case was about two 700mL cachaça bottles. tchin tchin 🍹

Expected Behavior

If you import a file to update products, the original products must be updated, not created again.

Actual Behaviour

If you import a file to update products, it will create duplicates for some products (or variants): the ones between 688 and 704ml. In the Products tab, the exact same product (or variant) appears twice.

It doesn't seem to apply when using other unit types or other unit values.

Steps to Reproduce

1. Connect to admin > Products tab > Import tab

2. Click on "Browse"

3. Select a file with at least one of the product with : 703 ⩾ units ⩾ 688 unit_type = ml (or mL) Ex. of test file : test-weirdo.csv

4. Just after the import, in the "Import validation overview", you see that all your products have been created. Save.

5. Click "Upload another file". Choose the same file. You will see that the products between 688 and 703mL will be created again, the other ones being updated.

Animated Gif/Screenshot

First import

Second import

Example of the dashboard product list with same products twice

Workaround

Each time you import a file to update your products database, you have to manually delete the duplicates and manually update the product if needed.

Severity

bug-s3: a feature is broken but there is a workaround

Possible Fix

When creating a 700mL product, the POST /admin/products request sends name="product[unit_value]" 0.700000000000001

instead of 0.700

When importing the file to update the product, unit_value is not the same and the product is considered as new.

See difference in number formatting between requests for 687mL and 688mL: whyohwhy.txt

This probably has more consequences, as all products created between 688 and 703mL will have a wrong unit_value in database...For example an export of the product database would have to be manually corrected.

[jibees]

I tried to reproduce it on local (to see if somehow, I can fix it through the PR #9497) but I failed to reproduced it although it is super simple to see it in fr-staging

[audez]

yep the bug is still on staging and prod

[abdellani]

Hi

I was not able to reproduce the problem. The screenshot shows the third attempt to import your file.

Also, I couldn't see any problem with the product table. Except for some values (693 converted to 693.0000000000001)

[RachL]

thanks for your time on this @abdellani perhaps the underlining issue is fixed then. Let's close it and reopen if we manage to reproduce it.

[audez]

@abdellani thank you! I just tested and unfortunately the issue is still here 😢

To reproduce:

• Create two products from the admin interface (/admin/products), for instance: 701ml product, category "ALCOOL FORT", units: 701ml, TVA 20% 704ml product, category "ALCOOL FORT", units: 704ml, TVA 20%

• Then import a csv file to update these 2 products ( test-weirdo4.csv

• The 704ml product is updated, but the other is created again

But you're right that the bug doesn't exist anymore if you created the product the first time by importing a file. But it's not sorted out, it's just because they're imported twice with the wrong formatting, so for the system it's the same product. We can see as you said that the products between 688 and 703ml are still badly formatted:

The fix would be: when importing a product from a csv file, remove the formatting of products between 688 and 703ml.

[RachL]

The fix would be: when importing a product from a csv file, remove the formatting of products between 688 and 703ml.

@audez I don't think that should be the role of the product import? If I understand clearly the problem is at the product catalog level? Otherwise can you detail how the second 704ml product was saved with manual creation and then with product import?

[audez]

Maybe I formulate it poorly. This is what I wanted to say: there is a bug to be corrected: when you import products only from 688 and 703ml with a .csv file, the products unit are recorded with a bad formatting. For instance you import a product, in the request it is "units":"701", but when you see the products in the product list, it displays 701.0000000000001. And in the db it is recorded as 701.0000000000001. There is no problem if you update this product by re-importing a .csv file, because it will compare 701.0000000000001 to 701.0000000000001, and it will be considered as the same product. But still the unit is wrong.

Now this bug doesn't exist when creating products from the interface. So if you create a product from the interface, whether within or outside 688 to 703ml range, the units are good (701 will be displayed as 701).

But then if you import a .csv to update these products in the 688 to 703ml range that you created by hand, it will create a new product, as the imported product will be recorded as X.0000....01 and considered new.

For 704ml products, there is no problem, whether by importing or creating from the interface, because the bug is only from 688 to 703ml.

If it was a problem at the product catalog level, you would probably see X.0000....01 for both products created by hand or imported with .csv

That's why I guess the thing to check is: how the product is recorded in the db after a product import, because there's something wrong with the unit formatting, weirdly only for numbers from 688 to 703 and mL unit

[audez]

When I created the issue, the bug was both when creating from the dashboard or importing. ~~But this summer after @jibees did some fix it was sorted out when creating from the dashboard, but remained when importing csv.~~ Actually it's still there whether from creating from the dashboard or importing. The difference after the fix done this summer is that if you create the product first by importing, and import it again, you won't see the bug for the reason previously explained.

[RachL]

ah ok thanks for the additional info! @abdellani do you have enough info to move forward?

[abdellani]

Yes @RachL . You can assign it to me now : )

[jibees]

@abdellani done! and thanks! 🙏

[abdellani]

Here's what I found so far. When I tried to create an invalid product with the unit_value = 700, the value was converted to 699.9999999999999 at the view level. When I changed the unit from "mL" to "L", the unit value problem doesn't occur.

I think the problem is related to string-to-float conversion in javascript (when it tries to convert 0.7 in product.master.unit_value).

I'm not familiar with AngularJs, but I suspect that the problem is somewhere around app/assets/javascripts/admin/products/controllers/edit_units_controller.js.coffee

I'll continue on it later.

[abdellani]

I found a problem at the controller level: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/assets/javascripts/admin/products/controllers/units_controller.js.coffee#L39

When the controller initializes the visible value field in the form (unit_value_with_description), The code devises the unit_value by the unit_scale.

• the unit_value = 0.7 (I guess values in mL are automatically converted to L)
• if the unit size is ml, the unit_scale will be equal to 0.001 ( reference)

I tried to do this in the console on the developer tools, and here is what I found:

>0.7/0.001
699.9999999999999

I think there is a similar problem in the part of code that list the products.

@jibees Should I create a PR to fix this problem?

[jibees]

Hi @abdellani 👋 Thanks a lot for spotting this one!

Yes, we're facing this kind of issues and seems to using js-big-decimal is probably the good way to go. You can see an example here: https://github.com/openfoodfoundation/openfoodnetwork/pull/9497

Feel free to open a PR, thanks a lot for that!

[abdellani]

Hi It seems that floating number precision problem is not only in the product creation form, it's also in the import function:

I imported test-weirdo4.csv

This is just an update, I'll handle the problem.

[abdellani]

Hi

The second part of the problem is in the Unit Convert used in the product imported.

[2] pry(#<ProductImport::UnitConverter>)> @attrs['units']
=> "701"
[3] pry(#<ProductImport::UnitConverter>)> @attrs['variant_unit_scale']
=> 0.001
[4] pry(#<ProductImport::UnitConverter>)> @attrs['units'].to_f * @attrs['variant_unit_scale']
=> 0.7010000000000001

Solution, use BigDecimal (to_d)

3.0.3 :009 > "701".to_f * 0.001
 => 0.7010000000000001 
3.0.3 :010 > "701".to_d * 0.001
 => 0.701e0
3.0.3 :011 > 0.701e0.to_s
 => "0.701"