[MIG] product_multi_barcode: Migration to 16.0 #471

StefanRijnhart · 2022-12-02T13:25:49Z

From unmerged #434

This PR uncovers a problem with odoo-test-helper used in barcodes_generator_abstract's tests. This should be fixed in OCA/odoo-test-helper#22.

rousseldenis · 2023-02-08T15:43:49Z

@StefanRijnhart Still draft ?

rousseldenis · 2023-02-08T15:44:55Z

/ocabot migration product_multi_barcode

StefanRijnhart · 2023-02-08T16:05:16Z

@rousseldenis ~~waiting for the odoo-test-helper fix to get this one green. Thanks for checking that one!~~ Now ready for review.

StefanRijnhart · 2023-02-08T20:07:20Z

Pre-commit fixed here: #491

rousseldenis

Code review. Some performance improvements can be done.

rousseldenis · 2023-02-09T09:18:45Z

product_multi_barcode/models/product_barcode.py

+                [("id", "!=", record.id), ("name", "=", record.name)]
+            )
+            if barcodes:
+                raise UserError(


You should use ValidationError in constraints

rousseldenis · 2023-02-09T09:20:19Z

product_multi_barcode/models/product_product.py

+    def _inverse_barcode(self):
+        for product in self:
+            if product.barcode_ids:
+                product.barcode_ids[:1].write({"name": product.barcode})


I think write() is not correct in an inverse method. You should assign value instead.

I changed this, although I think write is fine in principle. The same method also uses create and unlink. But assigning a single attribute is more odoo-esque of course.

rousseldenis · 2023-02-09T09:21:24Z

product_multi_barcode/models/product_product.py

+            elif not product.barcode:
+                product.barcode_ids.unlink()
+            else:
+                self.env["product.barcode"].create(self._prepare_barcode_vals())


Gather barcode vals in a list, then use multi create.

rousseldenis · 2023-02-09T09:22:21Z

product_multi_barcode/models/product_product.py

+            if product.barcode_ids:
+                product.barcode_ids[:1].write({"name": product.barcode})
+            elif not product.barcode:
+                product.barcode_ids.unlink()


Gather barcode ids in a set(), then barcode_model.browse(ids).unlink()

rousseldenis · 2023-02-09T09:23:45Z

product_multi_barcode/tests/test_product_multi_barcode.py

+
+@tagged("post_install", "-at_install")
+class TestProductMultiBarcode(TransactionCase):
+    def setUp(self):


Could you change this to classmethod ?

hieulucky111 · 2023-02-10T09:42:02Z

product_multi_barcode/models/product_barcode.py

+    @api.constrains("name")
+    def _check_duplicates(self):
+        for record in self:
+            barcodes = self.search(


should use search_count?

Actually, the found duplicate is used to present a detailed error message to the user so I've kept search.

hieulucky111 · 2023-02-10T09:43:13Z

product_multi_barcode/models/product_product.py

+            elif not product.barcode:
+                product.barcode_ids.unlink()
+            else:
+                self.env["product.barcode"].create(self._prepare_barcode_vals())


should be product._prepare_barcode_vals()

aliciagaarzo

Review comments and cover all cases in tests.

hugo-cordoba · 2023-02-28T12:26:21Z

Hello @rousseldenis , I have seen that @aliciagaarzo has made the changes you requested, how do you see them? I think the development is correct, can we help to merge the code? Thank you very much, have a nice day.

liebana · 2023-03-28T16:04:33Z

I've made a functional review and LGTM.

Friendly ping to @rousseldenis @StefanRijnhart @hieulucky111

rousseldenis · 2023-03-29T06:42:11Z

@StefanRijnhart Could you attend comments ?

…code [FIX] tests

Co-authored-by: Sébastien Beau <sebastien.beau@akretion.com>

Currently translated at 100.0% (17 of 17 strings) Translation: stock-logistics-barcode-14.0/stock-logistics-barcode-14.0-product_multi_barcode Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-barcode-14-0/stock-logistics-barcode-14-0-product_multi_barcode/it/

…form to see barcodes.

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: stock-logistics-barcode-14.0/stock-logistics-barcode-14.0-product_multi_barcode Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-barcode-14-0/stock-logistics-barcode-14-0-product_multi_barcode/

rousseldenis

Code review

StefanRijnhart · 2023-04-04T11:57:40Z

Thanks for the reviews. Fixes are now applied. I also included #501

rousseldenis · 2023-04-04T12:33:52Z

Thanks for the reviews. Fixes are now applied. I also included #501

@StefanRijnhart See my comment #501 (review)

StefanRijnhart · 2023-04-06T14:10:16Z

@rousseldenis do you still have any remaining concerns, or are we ready for merge?

rousseldenis · 2023-04-06T14:32:00Z

/ocabot merge nobump

OCA-git-bot · 2023-04-06T14:32:32Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-471-by-rousseldenis-bump-nobump, awaiting test results.

OCA-git-bot · 2023-04-06T14:37:05Z

Congratulations, your PR was merged at 0346ada. Thanks a lot for contributing to OCA. ❤️

StefanRijnhart added this to the 16.0 milestone Dec 2, 2022

StefanRijnhart force-pushed the 16.0-mig-product_multi_barcode branch 3 times, most recently from a56fe4a to c4e15ad Compare December 2, 2022 14:03

OCA-git-bot mentioned this pull request Feb 8, 2023

Migration to version 16.0 #440

Open

13 tasks

StefanRijnhart force-pushed the 16.0-mig-product_multi_barcode branch from c4e15ad to 6e67e60 Compare February 8, 2023 20:01

StefanRijnhart marked this pull request as ready for review February 8, 2023 20:07

OCA deleted a comment from OCA-git-bot Feb 8, 2023

StefanRijnhart force-pushed the 16.0-mig-product_multi_barcode branch from 6e67e60 to f619864 Compare February 8, 2023 21:04

rousseldenis requested changes Feb 9, 2023

View reviewed changes

hieulucky111 suggested changes Feb 10, 2023

View reviewed changes

aliciagaarzo suggested changes Feb 21, 2023

View reviewed changes

liebana approved these changes Mar 28, 2023

View reviewed changes

hugo-cordoba approved these changes Mar 29, 2023

View reviewed changes

kevinkhao and others added 9 commits April 4, 2023 13:45

[REF] product_multi_ean: Rename product_mult_ean to product_multi_bar…

2901243

…code [FIX] tests

[IMP] product_multi_barcode: split py files

4a0931f

[IMP] product_multi_barcode: run pre-commit

cedacd6

[FIX] product_multi_barcode: installation hook

fe4d913

[UPD] Update product_multi_barcode.pot

34749ae

[UPD] README.rst

59e5f46

[IMP] product_multi_barcode: black, isort, prettier

2b478d9

[MIG] product_multi_barcode: Migration to 14.0

1837fff

Co-authored-by: Sébastien Beau <sebastien.beau@akretion.com>

[FIX] remove inverse_sudo

bd8a33d

oca-travis and others added 16 commits April 4, 2023 13:45

[UPD] Update product_multi_barcode.pot

77cf3a9

[UPD] README.rst

fa8884c

[FIX] product_multi_barcode: missing else on a condition

de94cc9

product_multi_barcode 14.0.1.0.1

4446a4a

[14.0][UPD] product_multi_barcode: Updated product variant easy edit …

6ea7dca

…form to see barcodes.

[UPD] Update product_multi_barcode.pot

537c396

[UPD] README.rst

bbccefe

product_multi_barcode 14.0.1.1.0

b12ce78

[14.0][FIX] OCA#416

7b32908

product_multi_barcode 14.0.1.1.1

34e6de8

product_multi_barcode: fix muti-company access right issue

9517569

[MIG] product_multi_barcode: Migration to 15.0

c5ca818

[IMP] product_multi_barcode: pre-commit stuff

e824096

[MIG] product_multi_barcode: Migration to 16.0

594d303

StefanRijnhart force-pushed the 16.0-mig-product_multi_barcode branch from f619864 to 594d303 Compare April 4, 2023 11:54

rousseldenis approved these changes Apr 4, 2023

View reviewed changes

aliciagaarzo approved these changes Apr 4, 2023

View reviewed changes

rousseldenis approved these changes Apr 6, 2023

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Apr 6, 2023

OCA-git-bot merged commit d3dfe58 into OCA:16.0 Apr 6, 2023

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Apr 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MIG] product_multi_barcode: Migration to 16.0 #471

[MIG] product_multi_barcode: Migration to 16.0 #471

StefanRijnhart commented Dec 2, 2022 •

edited

Loading

rousseldenis commented Feb 8, 2023

rousseldenis commented Feb 8, 2023

StefanRijnhart commented Feb 8, 2023 •

edited

Loading

StefanRijnhart commented Feb 8, 2023

rousseldenis left a comment

rousseldenis Feb 9, 2023

rousseldenis Feb 9, 2023

StefanRijnhart Apr 4, 2023

rousseldenis Feb 9, 2023

rousseldenis Feb 9, 2023

rousseldenis Feb 9, 2023

hieulucky111 Feb 10, 2023

StefanRijnhart Apr 4, 2023

hieulucky111 Feb 10, 2023

aliciagaarzo left a comment

hugo-cordoba commented Feb 28, 2023

liebana commented Mar 28, 2023

rousseldenis commented Mar 29, 2023

rousseldenis left a comment

StefanRijnhart commented Apr 4, 2023

rousseldenis commented Apr 4, 2023

StefanRijnhart commented Apr 6, 2023

rousseldenis commented Apr 6, 2023

OCA-git-bot commented Apr 6, 2023

OCA-git-bot commented Apr 6, 2023

[MIG] product_multi_barcode: Migration to 16.0 #471

[MIG] product_multi_barcode: Migration to 16.0 #471

Conversation

StefanRijnhart commented Dec 2, 2022 • edited Loading

rousseldenis commented Feb 8, 2023

rousseldenis commented Feb 8, 2023

StefanRijnhart commented Feb 8, 2023 • edited Loading

StefanRijnhart commented Feb 8, 2023

rousseldenis 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

aliciagaarzo left a comment

Choose a reason for hiding this comment

hugo-cordoba commented Feb 28, 2023

liebana commented Mar 28, 2023

rousseldenis commented Mar 29, 2023

rousseldenis left a comment

Choose a reason for hiding this comment

StefanRijnhart commented Apr 4, 2023

rousseldenis commented Apr 4, 2023

StefanRijnhart commented Apr 6, 2023

rousseldenis commented Apr 6, 2023

OCA-git-bot commented Apr 6, 2023

OCA-git-bot commented Apr 6, 2023

StefanRijnhart commented Dec 2, 2022 •

edited

Loading

StefanRijnhart commented Feb 8, 2023 •

edited

Loading