[18.0][IMP] account_chart_update: Visible unchecked fields by default by u0f · Pull Request #2196 · OCA/account-financial-tools

u0f · 2025-12-17T10:55:26Z

Given the old logic, ignored fields simply did not appear and were not taken into consideration. #2185

With this improve, some fields have been moved from ignored to a new method which causes them to be displayed unchecked by default.

https://www.loom.com/share/47a70d6113804757a077f0a1955722c1

@moduon MT-12947

EmilioPascual

LGTM

account_chart_update/wizard/wizard_chart_update.py

yajo

Code review.

OCA-git-bot · 2026-01-07T08:00:02Z

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

Shide

Code review

Gelojr

I do not approve the PR. Review requested.

Module configuration (change in the wizard default behavior)

Changes introduced by the PR (default wizard configuration):
For the following models, the listed fields are included in the wizard but unchecked by default:

account.tax.group: sequence
account.tax: sequence, children_tax_ids
account.account: root_id
account.group: parent_id, code_prefix_end
account.fiscal.position: sequence

Tests performed and results

TEST1 — OK
TEST2 — OK
TEST3 — NOT OK (Taxes: Sequence)
- Starting point
  During the test, the Sequence field was explicitly selected in the wizard for account.tax.
- However, after running the update, the tax sequence order was not updated, so the expected change was not applied.

Reason for requesting a review

The test performed on Taxes shows that selecting the Sequence field does not result in an actual sequence update. This behavior does not match the expected outcome and requires further review before the PR can be approved.

u0f · 2026-01-07T14:36:43Z

Reason for requesting a review

The test performed on Taxes shows that selecting the Sequence field does not result in an actual sequence update. This behavior does not match the expected outcome and requires further review before the PR can be approved.

@Gelojr I have been reviewing this with @EmilioPascual, and it is not an error. In most cases, they do not have any sequence defined by default. If you try to update one that does have a sequence in the XML file that defines it, it will be placed in its correct position.

Also, I changed the fields I unchecked by default to:

account.fiscal.position: sequence

Gelojr

TEST3 has been performed again. I´m agree with the change has been introduced so that the final log now explicitly lists the accounts that have not been updated because they are not part of the chart of accounts and were created by other methods.
As a possible improvement, it would be useful if the log showed not only the account name but also the account code.

OCA-git-bot · 2026-01-15T17:00:06Z

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

Gelojr

Great job on this contribution @u0f
The following tests have been performed:

Test 1: Fiscal positions – verified that the field “Sequence” is not included by default when reviewing fiscal positions, and the behavior matches the expected configuration.
Test 2: Fiscal positions – manually selected the field “Sequence”, executed the update wizard, and confirmed that differences are correctly proposed and applied only when real discrepancies exist.

pedrobaeza

Now it seems o2m fields are shown, although unchecked, while they should be forbidden explicitly, as they can't be handled properly. Example, field tax_ids in tax groups. Can you please check?

EmilioPascual · 2026-02-09T06:41:46Z

Now it seems o2m fields are shown, although unchecked, while they should be forbidden explicitly, as they can't be handled properly. Example, field tax_ids in tax groups. Can you please check?

Done. Thank you for your review!

pedrobaeza · 2026-02-10T07:35:27Z

Thanks for the changes. Now I'm checking it, and it seems the changes on the tax distribution lines are not detected. Can it be affected by the o2m cleaning you have done?

@pedrobaeza

The previous implementation moved the one2many filter to _domain_per_name() which blocked one2many fields from being added to the wizard manually. This broke existing tests and prevented detection of changes in tax repartition lines. Solution: Move the one2many filter from _domain_per_name() to each _default_*_field_ids() method. This way: - One2many fields are NOT included by default (desired behavior) - One2many fields CAN be added manually (needed for tests) - Comparison in diff_fields() works correctly for nested models Fixes issue reported by @pedrobaeza in PR OCA#2196

EmilioPascual · 2026-02-11T15:02:19Z

Thanks for the changes. Now I'm checking it, and it seems the changes on the tax distribution lines are not detected. Can it be affected by the o2m cleaning you have done?

Updated. Now distribution lines are shown but they are unchecked

pedrobaeza · 2026-02-11T15:28:35Z

Well, they should be checked by default, as that's the most important part in the tax configuration IMO.

EmilioPascual · 2026-03-04T12:36:40Z

Well, they should be checked by default, as that's the most important part in the tax configuration IMO

Now, in v18, the distribution fields are unchecked, perhaps because they are the only fields that ‘odoo reload’ updates.

What do you think is best? Should we leave them unchecked as before, or should they always be checked?

pedrobaeza · 2026-03-04T12:40:47Z

If they are automatically checked and updated, no matter if the mark is checked or not, they should be hidden. If the check/update depends on the mark, then it should be checked by default.

EmilioPascual · 2026-03-05T09:35:25Z

Done.

pedrobaeza · 2026-03-05T15:20:08Z

Thanks, tested. Although there are 3 fields (invoice_repartition_line_ids, refund_repartition_line_ids and repartition_line_ids), the only one acting for putting update is the last one. If so, we may want to hide the other 2.

@moduon MT-12947

EmilioPascual · 2026-03-09T11:36:23Z

Thanks, tested. Although there are 3 fields (invoice_repartition_line_ids, refund_repartition_line_ids and repartition_line_ids), the only one acting for putting update is the last one. If so, we may want to hide the other 2.

You're right, it's better to leave only repartition_line_ids to avoid any confusion.
Updated code.

pedrobaeza · 2026-03-09T11:59:56Z

But they are shown, but unchecked, isn't it? I think they should be hidden as they was.

EmilioPascual · 2026-03-09T12:09:53Z

But they are shown, but unchecked, isn't it? I think they should be hidden as they was.

No, they are not shown, only repartition_line_ids.

pedrobaeza

OK, great!

/ocabot merge major

OCA-git-bot · 2026-03-09T12:12:45Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-2196-by-pedrobaeza-bump-major, awaiting test results.

OCA-git-bot · 2026-03-09T12:18:58Z

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

u0f marked this pull request as draft December 17, 2025 11:18

u0f marked this pull request as ready for review December 17, 2025 11:33

EmilioPascual approved these changes Jan 7, 2026

View reviewed changes

chienandalu reviewed Jan 7, 2026

View reviewed changes

account_chart_update/wizard/wizard_chart_update.py Outdated Show resolved Hide resolved

yajo approved these changes Jan 7, 2026

View reviewed changes

OCA-git-bot added approved ready to merge labels Jan 7, 2026

Shide approved these changes Jan 7, 2026

View reviewed changes

Gelojr suggested changes Jan 7, 2026

View reviewed changes

OCA-git-bot removed approved ready to merge labels Jan 7, 2026

u0f force-pushed the imp-account_chart_update branch 2 times, most recently from a49e557 to ec2f7aa Compare January 9, 2026 10:20

Gelojr approved these changes Jan 15, 2026

View reviewed changes

OCA-git-bot added approved ready to merge labels Jan 15, 2026

Gelojr approved these changes Jan 15, 2026

View reviewed changes

yajo approved these changes Jan 22, 2026

View reviewed changes

chienandalu approved these changes Jan 22, 2026

View reviewed changes

pedrobaeza changed the title ~~[IMP] account_chart_update: Visible unchecked fields by default~~ [18.0][IMP] account_chart_update: Visible unchecked fields by default Jan 30, 2026

pedrobaeza added this to the 18.0 milestone Jan 30, 2026

pedrobaeza reviewed Jan 30, 2026

View reviewed changes

EmilioPascual force-pushed the imp-account_chart_update branch from ec2f7aa to bfc3c0d Compare February 9, 2026 06:35

EmilioPascual force-pushed the imp-account_chart_update branch from 7e32ddf to 8ba16ee Compare February 11, 2026 13:52

EmilioPascual force-pushed the imp-account_chart_update branch from 8ba16ee to cfd3954 Compare February 11, 2026 15:00

EmilioPascual force-pushed the imp-account_chart_update branch from cfd3954 to beaa1b8 Compare March 5, 2026 09:23

[IMP] account_chart_update: Visible unchecked fields by default

b642ced

@moduon MT-12947

EmilioPascual force-pushed the imp-account_chart_update branch from beaa1b8 to b642ced Compare March 9, 2026 11:34

pedrobaeza approved these changes Mar 9, 2026

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Mar 9, 2026

OCA-git-bot merged commit 5857c99 into OCA:18.0 Mar 9, 2026
11 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Mar 9, 2026

Uh oh!

Conversation

u0f commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmilioPascual left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Jan 7, 2026

Uh oh!

Shide left a comment

Choose a reason for hiding this comment

Uh oh!

Gelojr left a comment

Choose a reason for hiding this comment

Module configuration (change in the wizard default behavior)

Tests performed and results

Reason for requesting a review

Uh oh!

u0f commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for requesting a review

Uh oh!

Gelojr left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Jan 15, 2026

Uh oh!

Gelojr left a comment

Choose a reason for hiding this comment

Uh oh!

pedrobaeza left a comment

Choose a reason for hiding this comment

Uh oh!

EmilioPascual commented Feb 9, 2026

Uh oh!

pedrobaeza commented Feb 10, 2026

Uh oh!

EmilioPascual commented Feb 11, 2026

Uh oh!

pedrobaeza commented Feb 11, 2026

Uh oh!

EmilioPascual commented Mar 4, 2026

Uh oh!

pedrobaeza commented Mar 4, 2026

Uh oh!

EmilioPascual commented Mar 5, 2026

Uh oh!

pedrobaeza commented Mar 5, 2026

Uh oh!

EmilioPascual commented Mar 9, 2026

Uh oh!

pedrobaeza commented Mar 9, 2026

Uh oh!

EmilioPascual commented Mar 9, 2026

Uh oh!

pedrobaeza left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Mar 9, 2026

Uh oh!

Uh oh!

OCA-git-bot commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

u0f commented Dec 17, 2025 •

edited

Loading

u0f commented Jan 7, 2026 •

edited

Loading