13 account chart refactor update taxes

[luc-demeyer]

No description provided.

[luc-demeyer]

I encountered a situation where I needed to sync some tax objects from the templates but some account settings on the repartition lines were customised and should be preserved.
Hence I started with adding two more options for the tax repartition lines-

• Update Tax Account
• Update Tax Tags
  

While implementing & testing this enhancement I came accross a whole set of issues in the tax update logic:

• same issue as in [13.0] account_chart_update FIX: not delete tax repartition lines #1052
• accounts not set when creating new repartition lines
• cash basis accounts not handled in 'deferred' mode
• tags settings incorrect

In order to fix it, I implemented logic based upon the approach in account/models/chart_template.py for tags and deferred account settings.

This PR also makes it clear that we should more unit tests to this module.

[MiquelRForgeFlow]

Hey @luc-demeyer I have been testing. Imagine in v12 you have a bill and put a tax (that have tax children) in a line. Then when migrating to v13, suppose new tax repartition lines are generated for each tax without tax children and the journal items are updated to point to those tax repartition lines. But happens in v13 that some localizations (ie, l10n_es) have substituted those old taxes with tax children for taxes with multiple tax repartition lines. I expected that running this module tool of update chart template after migration, I could adapt to the new localization taxes. But what I found was the following: in the journal items (move lines) the tax_repartition_line_id field is not updated. I mean, the tax repartition lines generated in the migration for tax children are still there but inactive. But those linked tax repartition lines should have been substituted for the new multiple repartition lines of their substituting taxes. It surprises me to see that the tax_ids field is correctly updated with new taxes but not the tax_repartition_line_id field.

I hope I explained the problem and could you test what I am saying. Let me know if I can help with anything.

[luc-demeyer]

@MiquelRForgeFlow
The purpose of this module is to synchronise the tax objects with the templates.
That means that a newly encoded invoice will take into account the updated taxes but legacy account.move.line entries are not updated.

We could add a wizard doing this to this module but I think we should make a new module for that (I'll do this exercise next month for a couple of customer projects hence we may use this experience as a starting point for such an OCA module).

[MiquelRForgeFlow]

@luc-demeyer In the method diff_fields, it can break in real[key] if key not in real. Can you fix this in a way to avoid the break?

[MiquelRForgeFlow]

@luc-demeyer I made a PR to your branch here luc-demeyer#14

[luc-demeyer]

@MiquelRForgeFlow
Thanks for proposing a solution to protect against a key error.
I don't think though that the proposed solution is the right one.
if we get a key error than there is probably something wrong in the self.fields_to_ignore(template._name) method.
Can you give me the concrete steps to reproduce this key error ?

[MiquelRForgeFlow]

@luc-demeyer for example, having a custom module that defines a new field for account.tax.template but not in the account.tax model. Then, if you don't have "expected", then breaks in the second "if".

BTW, if you look the code in previous versions like v12, you will see that the "try except" solution was already implemented.

[luc-demeyer]

@MiquelRForgeFlow
Adding a field to the template without the corresping real object seems strange to me but I suppose you have a valid use case for that.
I suggest to limit the number of lines in the try/except clause to the place where the problem can occur and add a _logger message since in most cases such an exception will be caused by an error in this module.
Concerning your custom module: I think it could be a good idea to add an auto-install account_chart_update_custom module to avoid ending up in this key exception.

[MiquelRForgeFlow]

I suggest to limit the number of lines in the try/except clause

I don't think it can be limited more. See in https://github.com/OCA/account-financial-tools/blob/12.0/account_chart_update/wizard/wizard_chart_update.py#L604L621, that's the same group of lines affected.

Adding a field to the template without the corresping real object seems strange to me but I suppose you have a valid use case for that.

Use case: OCA/l10n-spain#1443

[MiquelRForgeFlow]

@luc-demeyer cold you rebase? first commit is already in branch 13.0

[luc-demeyer]

@MiquelRForgeFlow
I fixed the issue reported by you this way 018b290
Please check if ok.

[pedrobaeza]

This branch is not clean. Please rebase properly and let the specific commits.

[gaikaz]

@luc-demeyer
As Pedro mentioned, could you please rebase your commits, so we could review?

[pedrobaeza]

Superseded by #1251