[14.0]intrastat Brexit support #163

luc-demeyer · 2022-01-02T14:43:40Z

No description provided.

pedrobaeza · 2022-01-02T14:57:37Z

Please rebase/cherry-pick instead of pulling for avoiding the merge commits.

…_origin_country_code to avoid breaking the localisation modules

…alisation modules

luc-demeyer · 2022-05-03T08:39:49Z

@alexis-via
I have tested the changes in this module with your l10n_fr_intrastat_product localisation and it looks ok: my changes seem to be non-breaking for the localisation modules but imho we should forward port this PR asap to 15.0 and remove the deprecated fields and logic from the localisation modules.

While testing l10n_fr_intrastat_product I encountered what I believe is a functonal bug (but unrelated to the brexit change in this PR). I have created a PR for this one, cf. OCA/l10n-france#357

I was also surprised to find the _get_product_origin_country in your module with a warning on missing product country of origin. In Belgium we can simply specify 'QU' as country code in this case.

jdidderen-be · 2022-05-08T09:11:16Z

intrastat_product/models/intrastat_product_declaration.py

@@ -490,8 +490,30 @@ def _get_incoterm(self, inv_line, notedict):
        return incoterm

    def _get_product_origin_country(self, inv_line, notedict):
+        _logger.warning(


In Odoo Community, they are using warnings.warn with a DeprecationWarning and this warning is shown in logs but also in the IDE (at least Pycharm does it) as Deprecated https://github.com/odoo/odoo/blob/687d88eeea836aacbcd1b079e82e597d1bd9309c/addons/account/models/account_move.py#L119

luc-demeyer · 2022-05-08T20:34:22Z

@jdidderen-be
Thanks for your review. Code adapted accordingly.

jdidderen-be · 2022-05-08T09:11:21Z

intrastat_product/models/account_move.py

+            vals["product_origin_country_code"] = (
+                vals["product_origin_country_code"].upper().strip()
+            )
+            if len(vals["product_origin_country_code"]) != 2:


Instead of having a check here on the length, I think it would be more easier and user-friendly to have a max-length directly on the field to be warned beforehand.

vanderperre

Hi Luc,

the version must take into account of the 14.0.1.5.4 changes ....

When i copy/paste in my current last oca update ....:

 "name": "Intrastat Product",

"version": "14.0.1.5.4",

"version": "14.0.1.5.3",
"category": "Intrastat",

A+

aaah Ok, the versioning is automatic with the runbot ... ;)

OCA-git-bot · 2022-05-12T11:10:19Z

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

luc-demeyer · 2022-05-15T17:40:00Z

/ocabot merge minor

OCA-git-bot · 2022-05-15T17:40:10Z

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-163-by-luc-demeyer-bump-minor, awaiting test results.

OCA-git-bot · 2022-05-15T17:47:09Z

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

alexis-via · 2022-09-07T19:55:36Z

With this code, we now have this bug when a French company sells to a belgian companies but the goods are delivered to spain: cf last line of the screenshot
Src/Dest Country = Spain
Country code = BE

This is because the country code is now only generated by looking at the commercial_partner_id of the invoice, without consideration for the field "src_dest_country_id". It's great to have super code for Northern-Ireland, but I should not break basic country code scenarios!

alexis-via · 2022-09-20T19:42:53Z

Other issues caused by this PR:

we don't see the product country of origin any more, but only it's code, which makes it more difficult for the user to control the data before validating the declaration.
the field "origin_state_id" is always displayed on product form view in the first tab, which is really a waste given that's it is only for products from Northern Ireland... I don't think we can afford that on the product form ! => what do you think about displaying it only when the country is UK ?

I really think that we shouldn't have gone so far for Northern Ireland. Northern Ireland is a mess anyway, so we should not try to make the "perfect" implementation with so many drawbacks for all users. I think my previous implementation was "good enough" and didn't cause so many drawback for such a corner-case scenario.

alexis-via · 2022-09-22T15:38:41Z

intrastat_base/models/res_partner.py

+        xu_counties = uk_counties.filtered(lambda r: r not in self._get_xi_counties())
+        return xu_counties
+
+    def _get_intrastat_country_code(self, country=None, state=None):


This method is really strange with it's "if self / else" ! When self has a value, then the arguments country and state are ignored, which is surprising. For me, instead of this wired code, we should have 2 separate methods, the first one calling the second one.

alexis-via · 2022-09-22T16:26:24Z

intrastat_product/models/account_move.py

+        string="Country of Origin of the Product",
+        size=2,
+        required=True,
+        default="QU",


I find it a bit "too easy" to use QU by default. I think we should display a warning in the warning dialog box about the fact that the country of origin is missing on product X and say that we've set the country code to QU as a temporary solution.

alexis-via · 2022-09-22T16:29:09Z

intrastat_product/views/intrastat_product_declaration.xml

+                <field
+                    name="product_origin_country_code"
+                    attrs="{'column_invisible': [('parent.declaration_type', '=', 'arrivals')]}"
+                    string="Product C/O"


In France, since for reform of January 2022 1st, we have to declare the product country of origin on arrivals AND departures! I would be very surprised if it was not the case in other countries. Cf page 28 section "9. Pays d'origine" of this PDF : https://www.douane.gouv.fr/sites/default/files/2021-12/21/Note-enquete-statistique-EMEBI-2022.pdf

luc-demeyer force-pushed the 14-brexit-support branch from 80a97a7 to e49bac5 Compare April 10, 2022 13:33

luc-demeyer added 6 commits May 2, 2022 15:40

[14.0]Intrastat Brexit support

47cbc65

[14.0]intrastat_product - Intrastat Brexit support

fa5667b

[14.0] brexit support - fix pipeline

5127bbd

[14.0] brexit support - fix pipeline

2b096b2

fix unit tests - keep product_origin_country_id on top of new product…

a8cdd74

…_origin_country_code to avoid breaking the localisation modules

[14.0]intrastat - improved brexit support

2874b76

luc-demeyer force-pushed the 14-brexit-support branch from 8487f1c to 2874b76 Compare May 2, 2022 18:44

luc-demeyer requested a review from alexis-via May 2, 2022 19:07

luc-demeyer added 2 commits May 3, 2022 09:54

[14.0]improved brexit support - keep backwards compatibility with loc…

085e24f

…alisation modules

[14.0]brexit support - increase test coverage

698dbfc

jdidderen-be reviewed May 8, 2022

View reviewed changes

[14.0]brexit support - DeprecationWarning

516b49d

jdidderen-be suggested changes May 12, 2022

View reviewed changes

[14.0]brexit support - size=2 on product_origin_country_code

964037e

jdidderen-be approved these changes May 12, 2022

View reviewed changes

vanderperre reviewed May 12, 2022

View reviewed changes

vanderperre approved these changes May 12, 2022

View reviewed changes

OCA-git-bot added approved ready to merge labels May 12, 2022

OCA-git-bot added the bot is merging ⏳ label May 15, 2022

OCA-git-bot merged commit 9faa481 into OCA:14.0 May 15, 2022

OCA-git-bot removed the bot is merging ⏳ label May 15, 2022

OCA-git-bot added the merged 🎉 label May 15, 2022

This was referenced Jun 14, 2022

[13.0][IMP] intrastat*: brexit support #179

Merged

[15.0][IMP] intrastat_base: Brexit support #180

Merged

[15.0][MIG] intrastat_product #165

Closed

[15.0][MIG] product_harmonized_system #158

Closed

alexis-via reviewed Sep 22, 2022

View reviewed changes

This was referenced Nov 4, 2022

[14.0][IMP] Big simplification, update and cleanup #198

Closed

[14.0] DES and EMEBI : update, improve and simplify OCA/l10n-france#383

Open

RabbitJon-S73 mentioned this pull request Sep 7, 2023

[16.0][MIG] l10n_es_intrastat_report: Migration to 16.0 OCA/l10n-spain#3111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0]intrastat Brexit support #163

[14.0]intrastat Brexit support #163

luc-demeyer commented Jan 2, 2022

pedrobaeza commented Jan 2, 2022

luc-demeyer commented May 3, 2022

jdidderen-be May 8, 2022

luc-demeyer commented May 8, 2022

jdidderen-be May 8, 2022

vanderperre left a comment •

edited

Loading

OCA-git-bot commented May 12, 2022

luc-demeyer commented May 15, 2022

OCA-git-bot commented May 15, 2022

OCA-git-bot commented May 15, 2022

alexis-via commented Sep 7, 2022

alexis-via commented Sep 20, 2022

alexis-via Sep 22, 2022

alexis-via Sep 22, 2022

alexis-via Sep 22, 2022

[14.0]intrastat Brexit support #163

[14.0]intrastat Brexit support #163

Conversation

luc-demeyer commented Jan 2, 2022

pedrobaeza commented Jan 2, 2022

luc-demeyer commented May 3, 2022

jdidderen-be May 8, 2022

Choose a reason for hiding this comment

luc-demeyer commented May 8, 2022

jdidderen-be May 8, 2022

Choose a reason for hiding this comment

vanderperre left a comment • edited Loading

Choose a reason for hiding this comment

OCA-git-bot commented May 12, 2022

luc-demeyer commented May 15, 2022

OCA-git-bot commented May 15, 2022

OCA-git-bot commented May 15, 2022

alexis-via commented Sep 7, 2022

alexis-via commented Sep 20, 2022

alexis-via Sep 22, 2022

Choose a reason for hiding this comment

alexis-via Sep 22, 2022

Choose a reason for hiding this comment

alexis-via Sep 22, 2022

Choose a reason for hiding this comment

vanderperre left a comment •

edited

Loading