[14.0][MIG] l10n_eu_product_adr #109

StefanRijnhart · 2021-04-27T11:04:59Z

14.0.1.0.0 (2021-05-04)
~~~~~~~~~~~~~~~~~~~~~~~
* [BREAKING] Gather dangerous goods attributes on a new model adr.goods,
  and replace all product attributes with a Many2one on this model.
  Remove all other unused or unclear attributes.
* [ADD] XML data for the dangerous goods table from the 2019 edition,
  including translations in 3 languages.

StefanRijnhart · 2021-04-27T11:08:33Z

@i-vyshnevska I have observed some points to improve this module; https://github.com/OCA/community-data-files/pull/109/files#diff-d967fda5116ea9b63e06dd49e2557fbfc802b8b6476598adcc50acd9d8b890b5R49-R73. Do you agree, or are there good reasons for some of the current design choices? What part of the specifications does great.class refer to?

i-vyshnevska · 2021-04-27T15:47:47Z

@StefanRijnhart your comments looks reasonable, what I can say would be good to have one more POV as work was based on just one customer requirements, also related report appeared to be more specific for the country

simahawk · 2021-09-14T09:30:28Z

@StefanRijnhart is this ready now?

StefanRijnhart · 2021-09-14T09:48:32Z

@simahawk thank you for your interest. I've now pushed the latest version as accepted by our customer. I believe it is more correct as it allows for multiple entries in the dangerous goods table with the same UN number, distinguished by different packing instructions or limited quantities. It does mean however that the already very bare migration from the previous version was invalidated entirely so I removed it.

l10n_eu_product_adr/models/adr_class.py

i-vyshnevska

in new version we not distinguish good and waste

l10n_eu_product_adr/models/adr_label.py

StefanRijnhart · 2021-09-14T20:01:21Z

in new version we not distinguish good and waste

@i-vyshnevska sorry, I don't know what you mean by this comment. Can you clarify?

mmequignon · 2021-10-05T14:22:08Z

l10n_eu_product_adr/scripts/import_adr_multilang_xlsx.py

+from collections import defaultdict
+
+from lxml import etree
+from openpyxl import load_workbook  # pylint: disable=W7936


Please wrap this import with a try, except
https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#32imports

Ah, but this is not necessary in recent versions of Odoo. The linked page (now) mentions this as well:

This rule doesn't apply neither to Odoo >= v12, as an unmet dependency in an uninstalled module doesn't block the service thanks to this commit:

mmequignon · 2021-10-14T13:42:09Z

l10n_eu_product_adr/models/adr_goods.py

+            if len(goods.un_number) != 4:
+                raise ValidationError(
+                    _(
+                        "UN Number %s is invalid because it does not have "
+                        "a length of 4."
+                    )
+                    % goods.un_number
+                )


We had to implement that as well. I don't recall why, but the our customer has some cases where the unnumber isn't a 4 digits long string. Here's where I implemented the solution we agreed on https://github.com/OCA/delivery-carrier/compare/0278821af40dc8fef2fc2518f53038a37bb07deb..0758137ff35a9a0e3170b641d566749de802df00

It would be interesting to hear when such a number can appear, because no such number appears in any of the specs I've seen!

mmequignon · 2021-10-14T13:49:01Z

l10n_eu_product_adr/models/product_template.py

+    is_dangerous = fields.Boolean(help="This product belongs to a dangerous class")
+    adr_goods_id = fields.Many2one("adr.goods", "Dangerous Goods")
+    adr_class_id = fields.Many2one(
+        "adr.class", related="adr_goods_id.class_id", readonly=True


Since the dangerous attributes can vary from a variant to another, we moved those fields from product.template to product.variant.
see this PR #112

Could you please do the same?

Alright, I will take care of that.

See new commit [RFR] l10n_eu_product_adr: allow different adr goods per variant

mmequignon

functional tests ok, even if it'll break a lot of module which depends on this one ^^'
However, @i-vyshnevska knows better about this.

i-vyshnevska · 2021-10-14T15:17:27Z

in new version we not distinguish good and waste

@i-vyshnevska sorry, I don't know what you mean by this comment. Can you clarify?

previously we had two classes to distinct dangerous "good" and "waste" (products which producing dangerous staff after they used) they treated differently in reports etc

Highcooley · 2021-10-19T14:34:10Z

in new version we not distinguish good and waste

@i-vyshnevska sorry, I don't know what you mean by this comment. Can you clarify?

previously we had two classes to distinct dangerous "good" and "waste" (products which producing dangerous staff after they used) they treated differently in reports etc

since our customer does not work with dangerous waste at the moment, I think we can ignore this.

mmequignon

@StefanRijnhart
We discussed internally about this, and it does not provide a way to tell that a product is valid or not, regarding the limited_qty.
I provided you a possible solution in the comments.
Thanks!

mmequignon · 2021-11-03T14:43:20Z

l10n_eu_product_adr/models/adr_goods.py

+            "vehicles.)"
+        ),
+    )
+    limited_quantity = fields.Char()


This one should be different

Suggested change

limited_quantity = fields.Char()

limited_quantity = fields.Float()

limited_quantity_uom = fields.Many2one("uom.uom")

OK. I will not commit your suggestion directly because of the implications, but I will have a look at this.

See new commit "[IMP] l10n_eu_product_adr: distinct uom for limited quantity" in this PR for this.

mmequignon · 2021-11-03T15:09:40Z

l10n_eu_product_adr/models/product_template.py

+    )
+    adr_label_ids = fields.Many2many(
+        "adr.label", related="adr_goods_id.label_ids", readonly=True
+    )


Suggested change

)

)

limited_quantity = fields.Boolean("Limited Qty", compute="_compute_limited_quantity", store=True)

With the compute method like so

@api.onchange("adr_goods_id", "adr_goods_id.limited_quantity_uom", "adr_goods_id.limited_quantity", "uom_id", ) def _compute_limited_quantity(self): for record in self: compare product's uom_qty with adr.goods uom_qty: product.limited_qty = adr.good uom_qty < product uom_qty

We should also ensure than the adr.label uom has the same uom family as the product one.

I'm sorry, but my customer is a bulk trader. They trade in pallets, or gallons, and the individual subpackaging is encoded in the product name rather than the UoM of the product. So what suits your requirements is not generally valid.

mmequignon · 2021-11-10T09:40:36Z

@StefanRijnhart
Hello Stefan, I see that you put this PR back in progress.
Could you please list what you will do, and give us a raw estimate of how many time it will take?
Thank you!

StefanRijnhart · 2021-11-14T19:43:48Z

@mmequignon I tried to indicate in my comments what I was going to refactor:

I hope to finish it this week on Monday or Tuesday.

Currently translated at 81.1% (99 of 122 strings) Translation: community-data-files-13.0/community-data-files-13.0-l10n_eu_product_adr Translate-URL: https://translation.odoo-community.org/projects/community-data-files-13-0/community-data-files-13-0-l10n_eu_product_adr/nl/

Co-authored-by: Iryna Vyshnevska <iryna.vyshnevska@camptocamp.com>

StefanRijnhart · 2022-06-16T12:18:17Z

I've rebased. As for the status, I'm considering to let this one slip because as expected this does not align well with the team that provided the previous implementation, and I think the communication has not been easy for either side. Also, I'm not working for the project that required this anymore and I'm a bit of a hippie so I prefer to work with bicycles rather than with dangerous goods anyway 😁

marylla · 2022-06-16T13:25:44Z

@bosd The runboat is now working, so you can test it. :) I would too but I don't know enough about this topic to test it properly.

bosd

LGTM!

OCA-git-bot · 2022-07-16T19:00:01Z

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). 🤖

marylla · 2022-07-26T13:32:34Z

@jgrandguillaume Can we please merge this one?

bosd · 2022-07-27T20:09:31Z

Maybe @OCA/community-data-files-maintainers can help

simahawk · 2022-07-28T06:02:36Z

FYI On our side we are waiting for @mmequignon to get back to this for a final review.
We have many other modules depending (and blocked) on this and we need to check how they are impacted after the last changes.
To name some:

simahawk · 2022-08-29T08:28:05Z

/ocabot merge nobump

OCA-git-bot · 2022-08-29T08:28:20Z

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-109-by-simahawk-bump-nobump, awaiting test results.

OCA-git-bot · 2022-08-29T08:37:12Z

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

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from f235435 to 0a7849e Compare May 4, 2021 18:21

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from 0a7849e to fd0a3b5 Compare September 14, 2021 09:44

StefanRijnhart marked this pull request as ready for review September 14, 2021 09:48

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from fd0a3b5 to 7366702 Compare September 14, 2021 09:49

i-vyshnevska reviewed Sep 14, 2021

View reviewed changes

l10n_eu_product_adr/models/adr_class.py Show resolved Hide resolved

i-vyshnevska reviewed Sep 14, 2021

View reviewed changes

l10n_eu_product_adr/models/adr_label.py Outdated Show resolved Hide resolved

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from 7366702 to 6aaf7d0 Compare September 14, 2021 19:40

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from 6aaf7d0 to 6675b8a Compare September 14, 2021 20:03

mmequignon reviewed Oct 14, 2021

View reviewed changes

mmequignon approved these changes Oct 14, 2021

View reviewed changes

sonhd91 mentioned this pull request Oct 20, 2021

[14.0][ADD] module l10n_ch_adr_report chucorp/l10n-switzerland#2

Open

TDu mentioned this pull request Oct 27, 2021

[14.0][ADD] module l10n_ch_adr_report OCA/l10n-switzerland#628

Closed

mmequignon reviewed Nov 3, 2021

View reviewed changes

StefanRijnhart added the work in progress label Nov 9, 2021

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch 4 times, most recently from ee881f5 to 88ba907 Compare November 16, 2021 16:23

StefanRijnhart removed the work in progress label Nov 16, 2021

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from 88ba907 to 2b3fd18 Compare November 17, 2021 17:48

mmequignon approved these changes Nov 22, 2021

View reviewed changes

bosd and others added 8 commits June 16, 2022 14:15

[IMP] l10n_eu_product_adr: black, isort, prettier

c824e61

[MIG] l10n_eu_product_adr: Migration to 14.0

2931deb

[RFR] Full refactoring of l10n_eu_product_adr

b05b2c1

[IMP] l10n_eu_product_adr: distinct uom for limited quantity

d27fc1d

[RFR] l10n_eu_product_adr: allow different adr goods per variant

cb3c349

Update l10n_eu_product_adr/models/stock_picking.py

16f55c5

Co-authored-by: Iryna Vyshnevska <iryna.vyshnevska@camptocamp.com>

[IMP] l10n_eu_product_adr: refactor security

f2779c1

StefanRijnhart force-pushed the 14.0-mig-l10n_eu_product_adr branch from 55ed548 to f2779c1 Compare June 16, 2022 12:15

bosd mentioned this pull request Jun 22, 2022

Migration to version 14.0 #96

Closed

8 tasks

bosd approved these changes Jul 16, 2022

View reviewed changes

OCA-git-bot added approved ready to merge labels Jul 16, 2022

simahawk requested a review from mmequignon July 28, 2022 06:02

mmequignon approved these changes Aug 29, 2022

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Aug 29, 2022

simahawk mentioned this pull request Aug 29, 2022

[14.0][MIG] stock_dangerous_goods OCA/stock-logistics-workflow#937

Merged

2 tasks

OCA-git-bot merged commit 41bfaf1 into OCA:14.0 Aug 29, 2022

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Aug 29, 2022

mmequignon mentioned this pull request Feb 20, 2023

[14.0][MIG] delivery_postlogistics_dangerous_goods OCA/delivery-carrier#609

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0][MIG] l10n_eu_product_adr #109

[14.0][MIG] l10n_eu_product_adr #109

StefanRijnhart commented Apr 27, 2021 •

edited

Loading

StefanRijnhart commented Apr 27, 2021

i-vyshnevska commented Apr 27, 2021

simahawk commented Sep 14, 2021

StefanRijnhart commented Sep 14, 2021

i-vyshnevska left a comment

StefanRijnhart commented Sep 14, 2021

mmequignon Oct 5, 2021

StefanRijnhart Nov 9, 2021

mmequignon Oct 14, 2021

StefanRijnhart Nov 9, 2021

mmequignon Oct 14, 2021

StefanRijnhart Nov 9, 2021

StefanRijnhart Nov 16, 2021

mmequignon left a comment

i-vyshnevska commented Oct 14, 2021

Highcooley commented Oct 19, 2021

mmequignon left a comment

mmequignon Nov 3, 2021

StefanRijnhart Nov 9, 2021

StefanRijnhart Nov 14, 2021

mmequignon Nov 3, 2021

StefanRijnhart Nov 9, 2021

mmequignon commented Nov 10, 2021

StefanRijnhart commented Nov 14, 2021

StefanRijnhart commented Jun 16, 2022

marylla commented Jun 16, 2022

bosd left a comment

OCA-git-bot commented Jul 16, 2022

marylla commented Jul 26, 2022

bosd commented Jul 27, 2022

simahawk commented Jul 28, 2022 •

edited

Loading

simahawk commented Aug 29, 2022

OCA-git-bot commented Aug 29, 2022

OCA-git-bot commented Aug 29, 2022

	limited_quantity = fields.Char()
	limited_quantity = fields.Float()
	limited_quantity_uom = fields.Many2one("uom.uom")

	)
	)
	limited_quantity = fields.Boolean("Limited Qty", compute="_compute_limited_quantity", store=True)

[14.0][MIG] l10n_eu_product_adr #109

[14.0][MIG] l10n_eu_product_adr #109

Conversation

StefanRijnhart commented Apr 27, 2021 • edited Loading

StefanRijnhart commented Apr 27, 2021

i-vyshnevska commented Apr 27, 2021

simahawk commented Sep 14, 2021

StefanRijnhart commented Sep 14, 2021

i-vyshnevska left a comment

Choose a reason for hiding this comment

StefanRijnhart commented Sep 14, 2021

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

mmequignon left a comment

Choose a reason for hiding this comment

i-vyshnevska commented Oct 14, 2021

Highcooley commented Oct 19, 2021

mmequignon 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

mmequignon commented Nov 10, 2021

StefanRijnhart commented Nov 14, 2021

StefanRijnhart commented Jun 16, 2022

marylla commented Jun 16, 2022

bosd left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Jul 16, 2022

marylla commented Jul 26, 2022

bosd commented Jul 27, 2022

simahawk commented Jul 28, 2022 • edited Loading

simahawk commented Aug 29, 2022

OCA-git-bot commented Aug 29, 2022

OCA-git-bot commented Aug 29, 2022

StefanRijnhart commented Apr 27, 2021 •

edited

Loading

simahawk commented Jul 28, 2022 •

edited

Loading