Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.0]FIX account.config.settings #31

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

luc-demeyer
Copy link
Contributor

Fix invalid domain in account.config.settings view

@@ -14,6 +14,7 @@
<field name="intrastat_transaction_out_refund"/>
<field name="intrastat_transaction_in_invoice"/>
<field name="intrastat_transaction_in_refund"/>
<field name="country_id" invisible="1"/>
Copy link

@moylop260 moylop260 Apr 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a doubt.

This field needs be added in order to fix a domain error for intrastat_region_id field.
But I don't understand what is the reason to adds a field with domain but invisible in the case of intrastat_region_id

What about remove from the view intrastat_region_id since that is not used from other domain or another field?
And both spent time to compute them the related (Because they are not using store=True)

@luc-demeyer
Copy link
Contributor Author

The intention of the 'intrastat_product' module is to have a common base for the localization modules.
The 'region' of destination/origin is required for the intrastat declarations of 30% of the EU member states. Because it's not mandatory for all states we decided when creating the intrastat_product module a couple of years ago to add this field to the view but as invisible so that a localisation module can simply turn it on. (cf. https://github.com/luc-demeyer/noviat-apps/blob/10.0/l10n_be_intrastat_product/views/account_config_settings.xml for an example)

But I agree that the approach is not optimal, especially in a multi-country system where this field will become visible for all countries once it's turned one by e.g. the belgian localization (and hence the french and spanish users will be confused since in France the localisation module does not use the 'region' field but the 'department' field whereas in Spain the standard 'state' field is used for this purpose.)

As an alternative we could change the view in the intrastat_product module as follows:

      <field name="intrastat_region_id" domain="[('country_id','=', country_id)]"
             attrs="{'invisible': [('country_code', 'not in', ['BE'])]}"/>

The consequence of this approach is that a editors of a localization module using the region field should make PR to add their country code to the list.

Any feedback on the preferred way forward is welcomed so that the Intrastat PSC can use this input to make a decision on the best approach to fix this issue.

@alexis-via
Copy link
Contributor

@luc-demeyer I prefer the current "simple" method with just visible/invisible.

@alexis-via alexis-via merged commit 1b7041c into OCA:10.0 Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants