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

alexis-via · 2022-11-04T20:16:37Z

This commit aims at removing the over-complexity of intrastat modules while increasing simplicity/usability for users. Some of you will probably disagree with some of the change in propose in this PR. But I have a strong disagreement with the over-complexity and the over-engineering that was introduced by this Brexit PR #163 For me, this over-complexity has become so expensive in term of software maintenance that, if the changes that I propose here are not accepted in v16, my intrastat v16 module for France will not be based on intrastat_product any more. So far, developping the generic module "intrastat_product" was an investment to try to reach the "ideal" where the biggest part of the code possible is generic. But, when looking at the past experience and where we are today with the codebase and the experience for the user, I think it may not have been the best decision in term of user experience and cost of software maintenance.

There are so many changes that I don't really expect to have it merged in v14 (If you want to merge it in v14, I'll add the migration scripts). But it's a good basis of discussion for v16 with a concrete example. My l10n_fr_intrastat_product module for v14 will be based on this PR (it was broken following the big Brexit changes of PR 163, so I had to fix it anyway).

Here are the main changes:

Move default intrastat transaction from res.company to account.fiscal.position to add support for B2C (and not just B2B)
improve usability: auto-generate declaration lines and XML export when going from draft to done. Auto-delete declaration lines and XML export when going from done to draft (and add confirmation pop-up).
declaration lines are now readonly. Only computation lines can be created/edited manually
add field region_code on computation lines and declaration lines. Remove region_id on declaration lines. This change allows big simplification in some localization modules such as l10n_fr_intrastat_product.
simplify Brexit implementation. Northern Ireland is important, but we can't afford to have so many lines of code and add a field on product.template (origin_state_id) for a territory of 1.9 million inhabitants! This is too costly to maintain and too complex for users.
improve default visibility of fields when reporting_level = 'standard'
add support for weight calculation from uom categories other than units and weight, supposing that the 'weight' field on product.template is the weight per uom of the product
add EU companies from several different countries in demo data with valid VAT numbers

OCA-git-bot · 2022-11-04T20:16:50Z

Hi @luc-demeyer,
some modules you are maintaining are being modified, check this out!

pedrobaeza · 2022-11-04T20:17:37Z

@victoralmau please check this

victoralmau

First of all thank you very much for trying to simplify the logic and unnecessary fields of these modules (even if the diff is large).

I think I understand the reason for removing the fields in res.company and adding them now in account.fiscal.position, would a migration script be necessary to define the corresponding values in all fiscal positions and thus maintain a similar behavior to the current one?
What do you think about this approach @pedrobaeza?

Could you try to separate all these changes in small commits easier to review (I know this is a bit difficult now, but it will help to understand the changes better and also in the future if it is merged)?

victoralmau · 2022-11-07T14:21:22Z

intrastat_product/models/intrastat_product_declaration.py

-        if country and country.code == "GB" and self.year >= "2021":
-            vat = inv.commercial_partner_id.vat
-            if not vat:
-                line_notes = [
-                    _(
-                        "On invoice '%s', the source/destination country "
-                        "is United-Kingdom and the fiscal position is '%s'. "
-                        "Make sure that the fiscal position is right. If "
-                        "the origin/destination is Northern Ireland, please "
-                        "set the VAT number of the partner '%s' in Odoo with "
-                        "its new VAT number starting with 'XI' following Brexit."
-                    )
-                    % (
-                        inv.name,
-                        inv.fiscal_position_id.display_name,
-                        inv.commercial_partner_id.display_name,
-                    )
-                ]
-                self._format_line_note(inv_line, notedict, line_notes)
-            elif not vat.startswith("XI"):
-                line_notes = [
-                    _(
-                        "On invoice '%s', the source/destination country "
-                        "is United-Kingdom, the fiscal position is '%s' and "
-                        "the partner's VAT number is '%s'. "
-                        "Make sure that the fiscal position is right. If "
-                        "the origin/destination is Northern Ireland, please "
-                        "update the VAT number of the partner '%s' in Odoo with "
-                        "its new VAT number starting with 'XI' following Brexit."
-                    )
-                    % (
-                        inv.name,
-                        inv.fiscal_position_id.display_name,
-                        vat,
-                        inv.commercial_partner_id.display_name,
-                    )
-                ]
-                self._format_line_note(inv_line, notedict, line_notes)


Can we maintain these warnings with this new approach?

Yes, we could keep the second warning and restrict it for B2B (vat_required=True on fiscal position). We should drop the first warning (or make it generic for B2B when vat_required=True on the fiscal position.

victoralmau · 2022-11-07T14:29:50Z

intrastat_product/models/intrastat_product_declaration.py

-    # adapted accordingly
+    # Note that, in l10n_fr_intrastat_product and maybe in other localization modules
+    # region_id is left empty and Odoo writes directly in region_code
+    region_code = fields.Char()


Perhaps another option would be to make the compute store=True field to avoid the onchange and be able to overwrite it in other modules.

No, because, in l10n_fr_intrastat_product, I don't use the object "intrastat.region", because I already have another object for that in the french localisation. That's why I want to have the char field region_code.

pedrobaeza · 2022-11-07T14:35:47Z

Why not having both the fields in account.fiscal.position and res.company as fallback?

luc-demeyer · 2022-11-13T18:39:57Z

We must welcome simplifications but unfortunately legal reports must also adhere to legal requirments.

I agree with some of the proposed changes but imho we should go a lot further.
We should move more logic from the localisation modules to the base modules hence making the localisation modules smaller and hence easier to maintain.

I also think we can go further in simplifying the user experience while keeping at least the current level of legal compliancy (for compliancy we should imho improve the link with the shipments, this is e.g. required in order to correctly handle refunds).

It's a very complex legal report and the legal specs are also changing constantly (as we have seen with the brexit, the harmonisation of the transactions codes, the b2b & b2c handling, ... ) and unfortunately not with the same speed in the different EU member countries.

I think it's going to be hard (unproductive) to take the right decisions on base module functionality versus localisation modules via a github thread.

What about organising a 3 days code sprint with a limited set of people having a good functional functional view on the regulations ? We currently have the french, spanish and belgian localisaiton modules and the generic declaration for use in the other countries. Hence I propose a code sprint with representatives of at least France (Alexis ?), Belgium (myself) and Spain where we define on how to move forward and create the 16.0 intrastat modules.
I can host it in our offices if we have a limited set of participants (<10) but I have no problem moving to France or Spain either.

A question specifically for @pedrobaeza :
would be hard to create and 'extended' pipeline so that any change to e.g. the intrastat_product module also triggers the pipelines of the localisation modules in the l10n-belgium, l10n-france, l10n-spain, ... repos ?

luc-demeyer · 2022-11-13T18:59:07Z

in the meanwhile I figured out that our italian colleagues also maintain an intrastat module (cf. https://github.com/OCA/l10n-italy/blob/14.0/l10n_it_intrastat) but this one does not build upon intrastat_product.
Hence I hope they are also willing to help to build a solid base module.

pedrobaeza · 2022-11-14T08:08:31Z

It's not feasible to make inter-repositories checks from a change on the base one, sorry.

alexis-via · 2022-12-28T22:20:12Z

@pedrobaeza having the m2o fields for intrastat.transaction both on account.fiscal.position and res.company as fallback is a possibility. I didn't feel the need for it because I usually have 1 or 2 fiscal position for intrastat, not more. But, if you do B2C and you are above the 10k€/year limit, then I suppose that you end up with 26 intrastat B2C fiscal positions, and that's where it could be an advantage to have the fallback configuration on res.company.

alexis-via · 2022-12-28T22:43:55Z

@luc-demeyer Thanks for your proposal to organise a code sprint on intrastat. But, for me, it illustrates the problem of this generic module: it is very complex and time-consuming to agree on the implementation details of this generic module. And, in all the very important things that we need to work on to improve OCA accounting (bank statement, payment orders, ...), intrastat should not be so high on the list. In other words : if I wanted to participate to an OCA code sprint in the coming months, I would prefer to have it on other topics than intrastat.

When I made this PR, I was quite upset about the situation of the intrastat code base, in particular about the over-complexity for Brexit/Northern Ireland and I was evaluating the decision to stop using intrastat_product.
I must say I was expecting more critics on this PR, especially on the removal of the file intrastat_base/model/res_partner.py and the removal of the field origin_state_id on product.template, which are the 2 main points that illustrate the over-complexity of the Brexit implementation.

So, reading the current comments on the PR, I have the feeling that you agree on my proposal. @luc-demeyer are you really ok about this, in particular the removal of intrastat_base/model/res_partner.py and the removal of the field origin_state_id on product.template ?

This commit aims at removing the over-complexity of intrastat modules while increasing simplicity/usability for users. - Move default intrastat transaction from res.company to account.fiscal.position to add su pport for B2C (and not just B2B) - improve usability: auto-generate declaration lines and XML export when going from draft to done. Auto-delete declaration lines and XML export when going from done to draft (and add confirmation pop-up). - declaration lines are now readonly. Only computation lines can be created/edited manuall y - add field region_code on computation lines and declaration lines. Remove region_id on declaration lines. This change allows big simplification in some localization modules such as l10n_fr_intrastat_product. - simplify Brexit implementation. Northern Ireland is important, but we can't afford to have so many lines of code and add a field on product.template (origin_state_id) for a territory of 1.9 million inhabitants! This is too costly to maintain and too complex for users. - improve default visibility of fields when reporting_level = 'standard' - add support for weight calculation from uom categories other than units and weight, supposing that the 'weight' field on product.template is the weight per uom of the product - add EU companies from several different countries in demo data with valid VAT numbers

…ND company Odoo uses, by order of priority: 1) intrastat transaction of the invoice 2) default intrastat transaction of the fiscal position of the invoice 3) default intrastat transaction of the company

alexis-via · 2023-01-23T18:02:38Z

@pedrobaeza I implemented your idea in this PR: default intrastat transaction is now both on fiscal position and company.
Now, Odoo uses by order of priority:

intrastat transaction of the invoice
default intrastat transaction of the fiscal position of the invoice
default intrastat transaction of the company of the invoice
I like this solution!

…ning pop-up This is completely new implementation of the messages that are displayed in the warning pop-up: - no double messages - messages are grouped by section and record - nice HTML display insead of raw text

alexis-via · 2023-01-23T23:33:10Z

OK guys, I've been dreaming of doing this for so many years, here it is:

No more double warning messages. Nice HTML display. Warnings grouped by section and record.

pedrobaeza · 2023-01-24T07:35:59Z

Nice. Please check CI.

Make tests work

alexis-via · 2023-01-24T22:50:59Z

CI is now green

pedrobaeza · 2023-01-25T07:31:33Z

intrastat_product/models/account_move.py

@@ -55,10 +55,6 @@ def _compute_src_dest_country_id(self):
                country = inv.company_id.country_id
            inv.src_dest_country_id = country.id

-    @api.model
-    def _default_src_dest_region_id(self):


This hook was done for a reason, so it's not good to remove it.

intrastat_product/wizards/intrastat_result_view.py

When manually creating a computation line, auto-set vat from partner_id (partner_id is not a related field any more) Add method _generate_xml()

fmdl · 2023-04-16T04:21:26Z

intrastat_product/views/intrastat_region.xml

@@ -4,7 +4,7 @@
        <field name="name">intrastat.region.form</field>
        <field name="model">intrastat.region</field>
        <field name="arch" type="xml">
-            <form string="Intrastat Region">
+            <form>


Don't forget to add sheet.

github-actions · 2023-08-20T12:06:40Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

alexis-via mentioned this pull request Nov 4, 2022

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

Open

pedrobaeza added this to the 14.0 milestone Nov 4, 2022

alexis-via mentioned this pull request Nov 4, 2022

Migration to version 16.0 #194

Open

3 tasks

alexis-via force-pushed the 14-brexit-simplification-and-type-on-fp branch from fbf24b6 to 766d016 Compare November 4, 2022 23:37

victoralmau suggested changes Nov 7, 2022

View reviewed changes

alexis-via force-pushed the 14-brexit-simplification-and-type-on-fp branch from 766d016 to 889f9f7 Compare January 23, 2023 17:18

alexis-via added 2 commits January 23, 2023 18:57

intrastat_product: default intrastat transaction on fiscal position A…

83ef1d6

…ND company Odoo uses, by order of priority: 1) intrastat transaction of the invoice 2) default intrastat transaction of the fiscal position of the invoice 3) default intrastat transaction of the company

intrastat_product: move res_config_settings.py to wizard subdir

99594ea

alexis-via added 2 commits January 23, 2023 19:12

intrastat_product: Improve warning message on intrastat transaction

10d43eb

intrastat_product: Backport updates from v15

3b54045

Make tests work

pedrobaeza reviewed Jan 25, 2023

View reviewed changes

alexis-via added 2 commits February 3, 2023 14:59

intrastat_product: update tests

f7a3c6d

When manually creating a computation line, auto-set vat from partner_id (partner_id is not a related field any more) Add method _generate_xml()

intrastat.transaction are now shared among companies by default

04db69c

alexis-via mentioned this pull request Feb 8, 2023

[16.0][MIG] intrastat_product #214

Merged

fmdl reviewed Apr 16, 2023

View reviewed changes

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 20, 2023

github-actions bot closed this Sep 24, 2023

alexis-via mentioned this pull request Aug 22, 2024

[16.0][BIG] Large refactoring/improvement/cleanup of OCA/bank-payment OCA/bank-payment#1174

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

alexis-via commented Nov 4, 2022 •

edited

Loading

OCA-git-bot commented Nov 4, 2022

pedrobaeza commented Nov 4, 2022

victoralmau left a comment

victoralmau Nov 7, 2022

alexis-via Dec 28, 2022

victoralmau Nov 7, 2022

alexis-via Dec 28, 2022

pedrobaeza commented Nov 7, 2022

luc-demeyer commented Nov 13, 2022

luc-demeyer commented Nov 13, 2022

pedrobaeza commented Nov 14, 2022

alexis-via commented Dec 28, 2022

alexis-via commented Dec 28, 2022

alexis-via commented Jan 23, 2023

alexis-via commented Jan 23, 2023

pedrobaeza commented Jan 24, 2023

alexis-via commented Jan 24, 2023

pedrobaeza Jan 25, 2023

fmdl Apr 16, 2023

github-actions bot commented Aug 20, 2023

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

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

Conversation

alexis-via commented Nov 4, 2022 • edited Loading

OCA-git-bot commented Nov 4, 2022

pedrobaeza commented Nov 4, 2022

victoralmau left a comment

Choose a reason for hiding this comment

victoralmau Nov 7, 2022

Choose a reason for hiding this comment

alexis-via Dec 28, 2022

Choose a reason for hiding this comment

victoralmau Nov 7, 2022

Choose a reason for hiding this comment

alexis-via Dec 28, 2022

Choose a reason for hiding this comment

pedrobaeza commented Nov 7, 2022

luc-demeyer commented Nov 13, 2022

luc-demeyer commented Nov 13, 2022

pedrobaeza commented Nov 14, 2022

alexis-via commented Dec 28, 2022

alexis-via commented Dec 28, 2022

alexis-via commented Jan 23, 2023

alexis-via commented Jan 23, 2023

pedrobaeza commented Jan 24, 2023

alexis-via commented Jan 24, 2023

pedrobaeza Jan 25, 2023

Choose a reason for hiding this comment

fmdl Apr 16, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 20, 2023

alexis-via commented Nov 4, 2022 •

edited

Loading