Skip to content

Commit 6c4133a

Browse files
antoine162smetl
authored andcommitted
[IMP] account: Normalize factors in _distribute_delta_amounts_smoothly
At the moment, `_distribute_delta_amounts_smoothly` requires the factors to be pre-nomalized. However, normalizing the factors robustly requires more logic than just dividing by the sum of factors, especially if the factors have different signs. This commit moves the normalization into `_distribute_delta_amounts_smoothly` itself meaning the calling code doesn't need to take care of it anymore. task-none X-original-commit: 80d5eed Part-of: odoo#232025 Related: odoo/enterprise#97443 Signed-off-by: Antoine Dupuis (andu) <[email protected]> Signed-off-by: Laurent Smet (las) <[email protected]>
1 parent 0871c96 commit 6c4133a

File tree

2 files changed

+28
-21
lines changed

2 files changed

+28
-21
lines changed

addons/account/models/account_tax.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,13 +1744,21 @@ def _distribute_delta_amount_smoothly(self, precision_digits, delta_amount, targ
17441744
sign = -1 if delta_amount < 0.0 else 1
17451745
nb_of_errors = round(abs(delta_amount / precision_rounding))
17461746
remaining_errors = nb_of_errors
1747-
for i, target_factor in enumerate(target_factors):
1748-
factor = target_factor['factor']
1747+
1748+
# Take absolute value of factors and sort them by largest absolute value first
1749+
factors = [(i, abs(target_factor['factor'])) for i, target_factor in enumerate(target_factors)]
1750+
factors.sort(key=lambda x: x[1], reverse=True)
1751+
sum_of_factors = sum(x[1] for x in factors)
1752+
1753+
if sum_of_factors == 0.0:
1754+
return amounts_to_distribute
1755+
1756+
for i, factor in factors:
17491757
if not remaining_errors:
17501758
break
17511759

17521760
nb_of_amount_to_distribute = min(
1753-
math.ceil(abs(factor * nb_of_errors)),
1761+
math.ceil(factor / sum_of_factors * nb_of_errors),
17541762
remaining_errors,
17551763
)
17561764
remaining_errors -= nb_of_amount_to_distribute
@@ -2067,7 +2075,7 @@ def _round_base_lines_tax_details(self, base_lines, company, tax_lines=None):
20672075
delta_amount = tax_amounts[f'raw_{delta_field}'] - tax_amounts[delta_field]
20682076
target_factors = [
20692077
{
2070-
'factor': abs(base_line['tax_details']['total_included_currency'] / tax_amounts['total_included_currency']),
2078+
'factor': base_line['tax_details']['total_included_currency'],
20712079
'base_line': base_line,
20722080
'index_tax_data': index_tax_data,
20732081
}
@@ -2116,7 +2124,7 @@ def _round_base_lines_tax_details(self, base_lines, company, tax_lines=None):
21162124

21172125
target_factors = [
21182126
{
2119-
'factor': abs(base_line['tax_details']['total_included_currency'] / tax_amounts['total_included_currency']),
2127+
'factor': base_line['tax_details']['total_included_currency'],
21202128
'base_line': base_line,
21212129
'index_tax_data': index_tax_data,
21222130
}
@@ -2175,7 +2183,7 @@ def _round_base_lines_tax_details(self, base_lines, company, tax_lines=None):
21752183
continue
21762184

21772185
# Dispatch the base delta evenly on the base lines, starting from the biggest line.
2178-
factors = [{'factor': 1.0 / len(base_amounts['base_lines'])}] * len(base_amounts['base_lines'])
2186+
factors = [{'factor': 1.0}] * len(base_amounts['base_lines'])
21792187
base_lines_sorted = sorted(
21802188
base_amounts['base_lines'],
21812189
key=lambda base_line: base_line['tax_details']['total_included_currency'],
@@ -2361,7 +2369,7 @@ def _add_accounting_data_to_base_line_tax_details(self, base_line, company, incl
23612369
delta_amount = tax_amount - total_tax_rep_amounts[field]
23622370
target_factors = [
23632371
{
2364-
'factor': abs(tax_rep_data[field] / tax_amount) if tax_amount else 0.0,
2372+
'factor': tax_rep_data[field],
23652373
'tax_rep_data': tax_rep_data,
23662374
}
23672375
for tax_rep_data in sorted_tax_reps_data

addons/account/static/src/helpers/account_tax.js

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -696,14 +696,21 @@ export const accountTaxHelpers = {
696696
const nb_of_errors = Math.round(Math.abs(delta_amount / precision_rounding));
697697
let remaining_errors = nb_of_errors;
698698

699-
for (let i = 0; i < target_factors.length; i++) {
700-
const factor = target_factors[i].factor;
699+
// Take absolute value of factors and sort them by largest absolute value first
700+
const factors = target_factors.map((x, i) => [i, Math.abs(x.factor)]);
701+
factors.sort((a, b) => b[1] - a[1]);
702+
const sum_of_factors = factors.reduce((sum, x) => sum + x[1], 0.0);
703+
if (sum_of_factors === 0.0) {
704+
return amounts_to_distribute;
705+
}
706+
707+
for (const [i, factor] of factors) {
701708
if (remaining_errors === 0) {
702709
break;
703710
}
704711

705712
const nb_of_amount_to_distribute = Math.min(
706-
Math.ceil(Math.abs(factor * nb_of_errors)),
713+
Math.ceil(Math.abs((factor / sum_of_factors) * nb_of_errors)),
707714
remaining_errors
708715
);
709716

@@ -1069,10 +1076,7 @@ export const accountTaxHelpers = {
10691076
const target_factors = tax_amounts.sorted_base_line_x_tax_data
10701077
.filter(([base_line, index_tax_data]) => index_tax_data)
10711078
.map(([base_line, index_tax_data]) => ({
1072-
factor: Math.abs(
1073-
base_line.tax_details.total_included_currency /
1074-
tax_amounts.total_included_currency
1075-
),
1079+
factor: base_line.tax_details.total_included_currency,
10761080
base_line: base_line,
10771081
index_tax_data: index_tax_data,
10781082
}));
@@ -1134,10 +1138,7 @@ export const accountTaxHelpers = {
11341138

11351139
const target_factors = tax_amounts.sorted_base_line_x_tax_data.map(
11361140
([base_line, index_tax_data]) => ({
1137-
factor: Math.abs(
1138-
base_line.tax_details.total_included_currency /
1139-
tax_amounts.total_included_currency
1140-
),
1141+
factor: base_line.tax_details.total_included_currency,
11411142
base_line: base_line,
11421143
index_tax_data: index_tax_data,
11431144
})
@@ -1215,9 +1216,7 @@ export const accountTaxHelpers = {
12151216
}
12161217

12171218
// Dispatch the base delta evenly on the base lines, starting from the biggest line.
1218-
const factors = Array(base_amounts.base_lines.length).fill({
1219-
factor: 1.0 / base_amounts.base_lines.length,
1220-
});
1219+
const factors = Array(base_amounts.base_lines.length).fill({ factor: 1.0 });
12211220
const base_lines_sorted = base_amounts.base_lines.sort(
12221221
(a, b) =>
12231222
a.tax_details.total_included_currency - b.tax_details.total_included_currency

0 commit comments

Comments
 (0)