[ADD] product_uom_merge #1809

JasminSForgeFlow · 2023-08-03T05:16:24Z

Module for merge duplicate uoms

cc @ForgeFlow

AaronHForgeFlow

Tested with done stock moves. Works good. Thanks!

AaronHForgeFlow

Approved as green now and functional tests done

AaronHForgeFlow · 2023-08-04T12:41:36Z

base_uom_merge/__manifest__.py

+    "name": "Base Product UOMs Merge",
+    "summary": "Merge duplicate product UOMs",
+    "version": "15.0.1.0.0",
+    "license": "LGPL-3",


Suggested change

"license": "LGPL-3",

"license": "AGPL-3",

based on https://github.com/OCA/openupgradelib/blob/master/openupgradelib/openupgrade_merge_records.py

Updated, Thanks

OriolMForgeFlow

Some small comments

Functional review: it works properly

OriolMForgeFlow · 2023-08-08T08:04:55Z

base_uom_merge/readme/DESCRIPTION.rst

@@ -0,0 +1 @@
+We can merge duplicates products into single product


Suggested change

We can merge duplicates products into single product

We can merge duplicates UOMs into single UOM.

Updated, Thanks

OriolMForgeFlow · 2023-08-08T08:05:42Z

base_uom_merge/readme/USAGE.rst

@@ -0,0 +1,3 @@
+Select uoms from same cateogry then click on Merge UOMs in Action menu.


Suggested change

Select uoms from same cateogry then click on Merge UOMs in Action menu.

Select uoms from same category then click on Merge UOMs in Action menu.

Updated, Thanks

OriolMForgeFlow · 2023-08-08T08:33:24Z

base_uom_merge/__manifest__.py

+    "license": "AGPL-3",
+    "website": "https://github.com/OCA/stock-logistics-warehouse",
+    "author": "ForgeFlow, Odoo Community Association (OCA)",
+    "category": "Warehouse Management",


I would change the category field, as this module is not exclusively for Warehouse Management. It can be used from the Sales application without requiring the use of a warehouse, as it no depends to "stock"

Maybe change to category "Product".

Update category as per dependent module Product's category.

Thanks for suggestion

OriolMForgeFlow · 2023-08-16T10:37:37Z

base_uom_merge/README.rst

@@ -0,0 +1,3 @@
+===================
+Base Products Merge


Maybe change to Base Product UOMs Merge

Updated, Thanks

OriolMForgeFlow · 2023-08-16T10:38:44Z

base_uom_merge/tests/test_base_uom_merge.py

+
+
+class TestMergeUOM(TransactionCase):
+    def setUp(self):


Change to setUpClass according to the migration rules to v15

Updated, Thanks

OriolMForgeFlow

LGTM!

OCA-git-bot · 2023-08-16T11:15:42Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

rousseldenis · 2023-08-16T12:36:55Z

@JasminSForgeFlow My 10k€ question: is this should be here or in product-attribute repository as it does not depends on stock ?

hbrunn

you should either forbid merging UoMs with different factors too, or add code that converts the quantity of quants. Otherwise you'll get wrong stock levels, as quants always refer to the product's standard UoM. Maybe a big red warning also suffices for cases where stock is wrong anyways and the merging is the first step to get it right?

Imagine you have product A with UoM km and product B with UoM m. If you now go and merge km into m, your stock levels of product A will be off by factor 1000.

I'm working on a generic merge addon where my customer has been bitten by a very similar issue when merging products, there also quants suddenly refer to a different UoM and are potentially off by the factor difference of the merged products' UoMs.

My fix is to convert quantities before merging: https://github.com/OCA/server-ux/pull/625/files#diff-b166ff54d6536b547b2dbad5be3664f2b7926c93fda8cbb3ddf38e312037aa78R23

@rousseldenis that would then also answer the 10k question, if you want to implement the conversion here, you'd need a dependency on stock.

rousseldenis · 2023-08-17T09:28:29Z

@rousseldenis that would then also answer the 10k question, if you want to implement the conversion here, you'd need a dependency on stock.

@hbrunn I haven't look at the implementation yet 😅 but indeed this needs to be more coercive. In fact, your module depends on stock but could be split (supplierinfo in product / quant in stock ones).

hbrunn · 2023-08-17T14:29:33Z

@JasminSForgeFlow I had to implement UoM support for the PR above anyways, so it might be just a little more work to simply lift this to v15 (I expect you just have to change the version number and some field names), then you get all the checks and conversions I talked about for free. And the possibility to merge countries, states and whatever else users tend to duplicate by misspelling things in many2one fields with inline create activated.
Note this is not meant to bully you into doing this, I'll just as happily review an update here where you add the conversion/check/warning. I just think you/your customers would get much more options this way for free.

@rousseldenis let's discuss stuff related to my PR there such as not to hijack the discussion here :-)

JasminSForgeFlow · 2023-08-23T10:59:11Z

Closing PR

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch 2 times, most recently from c217748 to eb79e99 Compare August 3, 2023 12:32

AaronHForgeFlow reviewed Aug 3, 2023

View reviewed changes

JasminSForgeFlow marked this pull request as ready for review August 4, 2023 04:40

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch from eb79e99 to 9e5e3b6 Compare August 4, 2023 04:56

AaronHForgeFlow approved these changes Aug 4, 2023

View reviewed changes

AaronHForgeFlow reviewed Aug 4, 2023

View reviewed changes

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch from 9e5e3b6 to d137d5e Compare August 8, 2023 06:50

OriolMForgeFlow reviewed Aug 8, 2023

View reviewed changes

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch from d137d5e to 9cb0948 Compare August 16, 2023 10:34

OriolMForgeFlow reviewed Aug 16, 2023

View reviewed changes

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch from 9cb0948 to 52463c5 Compare August 16, 2023 10:47

[ADD] product_uom_merge

aeb539a

JasminSForgeFlow force-pushed the 15.0-add-product_uom_merge branch from 52463c5 to aeb539a Compare August 16, 2023 10:54

OriolMForgeFlow approved these changes Aug 16, 2023

View reviewed changes

OCA-git-bot added approved ready to merge labels Aug 16, 2023

hbrunn requested changes Aug 17, 2023

View reviewed changes

OCA-git-bot removed approved ready to merge labels Aug 17, 2023

JasminSForgeFlow closed this Aug 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] product_uom_merge #1809

[ADD] product_uom_merge #1809

JasminSForgeFlow commented Aug 3, 2023

AaronHForgeFlow left a comment

AaronHForgeFlow left a comment

AaronHForgeFlow Aug 4, 2023

JasminSForgeFlow Aug 8, 2023

OriolMForgeFlow left a comment

OriolMForgeFlow Aug 8, 2023

JasminSForgeFlow Aug 16, 2023

OriolMForgeFlow Aug 8, 2023

JasminSForgeFlow Aug 16, 2023

OriolMForgeFlow Aug 8, 2023

JasminSForgeFlow Aug 16, 2023

OriolMForgeFlow Aug 16, 2023

JasminSForgeFlow Aug 16, 2023

OriolMForgeFlow Aug 16, 2023

JasminSForgeFlow Aug 16, 2023

OriolMForgeFlow left a comment

OCA-git-bot commented Aug 16, 2023

rousseldenis commented Aug 16, 2023

hbrunn left a comment

rousseldenis commented Aug 17, 2023

hbrunn commented Aug 17, 2023

JasminSForgeFlow commented Aug 23, 2023

		@@ -0,0 +1 @@
		We can merge duplicates products into single product

	We can merge duplicates products into single product
	We can merge duplicates UOMs into single UOM.

		@@ -0,0 +1,3 @@
		Select uoms from same cateogry then click on Merge UOMs in Action menu.

	Select uoms from same cateogry then click on Merge UOMs in Action menu.
	Select uoms from same category then click on Merge UOMs in Action menu.

[ADD] product_uom_merge #1809

[ADD] product_uom_merge #1809

Conversation

JasminSForgeFlow commented Aug 3, 2023

AaronHForgeFlow left a comment

Choose a reason for hiding this comment

AaronHForgeFlow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OriolMForgeFlow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OriolMForgeFlow left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Aug 16, 2023

rousseldenis commented Aug 16, 2023

hbrunn left a comment

Choose a reason for hiding this comment

rousseldenis commented Aug 17, 2023

hbrunn commented Aug 17, 2023

JasminSForgeFlow commented Aug 23, 2023