From edb58695b447e098c8a97f1acde143ee63ae371e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Wed, 22 Feb 2023 10:22:26 +0000 Subject: [PATCH] [FIX] spreadsheet_account: don't sum account twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's say there is one account with code 431 and another with code 4310, `ODOO.BALANCE("431,4310", 2022)` counts 4310 twice. The reason is queries are done one code at a time, one by one. The account 4310 is counted first when using 431 as a prefix (431%) and once more with 4310 (4310%). Now, the query uses all codes all at once. The original idea was to cache queries as much as we could. If you later add the formula `ODOO.BALANCE("431", 2022)`, the result would have been cached because queries were done code by code. Given the issue it brings, we thing this opmitisation is not worth it and is therefore removed. It also makes the code client-side simpler. opw-3144473 closes odoo/odoo#113418 X-original-commit: 07bd67f3a389bde4f33e08d97967dbb70ad5a19f Signed-off-by: Pierre Rousseau (pro) Signed-off-by: Lucas Lefèvre (lul) --- addons/spreadsheet_account/models/account.py | 65 ++++--- .../static/src/accounting_datasource.js | 30 +--- .../static/tests/model/accounting_tests.js | 53 ++---- .../tests/test_debit_credit.py | 158 ++++++++++++++---- 4 files changed, 177 insertions(+), 129 deletions(-) diff --git a/addons/spreadsheet_account/models/account.py b/addons/spreadsheet_account/models/account.py index 68d975235a1..08de7fef72b 100644 --- a/addons/spreadsheet_account/models/account.py +++ b/addons/spreadsheet_account/models/account.py @@ -41,48 +41,42 @@ class AccountMove(models.Model): return start, end def _build_spreadsheet_formula_domain(self, formula_params): - code = formula_params["code"] - if not code: + codes = [code for code in formula_params["codes"] if code] + if not codes: return expression.FALSE_DOMAIN company_id = formula_params["company_id"] or self.env.company.id company = self.env["res.company"].browse(company_id) start, end = self._get_date_period_boundaries( formula_params["date_range"], company ) - - base_domain = [ - ("account_id.code", "=like", f"{code}%"), - ("company_id", "=", company_id), + balance_domain = [ + ("account_id.include_initial_balance", "=", True), + ("date", "<=", end), ] + pnl_domain = [ + ("account_id.include_initial_balance", "=", False), + ("date", ">=", start), + ("date", "<=", end), + ] + code_domain = expression.OR( + [ + expression.AND([ + [("account_id.code", "=like", f"{code}%")], + expression.OR([balance_domain, pnl_domain]), + ]) + for code in codes + ] + ) + domain = expression.AND([code_domain, [("company_id", "=", company_id)]]) if formula_params["include_unposted"]: - base_domain = expression.AND( - [base_domain, [("move_id.state", "!=", "cancel")]] + domain = expression.AND( + [domain, [("move_id.state", "!=", "cancel")]] ) else: - base_domain = expression.AND( - [base_domain, [("move_id.state", "=", "posted")]] + domain = expression.AND( + [domain, [("move_id.state", "=", "posted")]] ) - - balance_domain = expression.AND( - [ - base_domain, - [ - ("account_id.include_initial_balance", "=", True), - ("date", "<=", end), - ], - ] - ) - result_domain = expression.AND( - [ - base_domain, - [ - ("account_id.include_initial_balance", "=", False), - ("date", ">=", start), - ("date", "<=", end), - ], - ] - ) - return expression.OR([balance_domain, result_domain]) + return domain @api.model def spreadsheet_move_line_action(self, args): @@ -94,7 +88,7 @@ class AccountMove(models.Model): "views": [[False, "list"]], "target": "current", "domain": domain, - "name": _("Journal items for account prefix %s", args["code"]), + "name": _("Journal items for account prefix %s", ", ".join(args["codes"])), } @api.model @@ -107,7 +101,7 @@ class AccountMove(models.Model): year: int }, company_id: int - code: str + codes: str[] include_unposted: bool }] """ @@ -139,7 +133,10 @@ class AccountMove(models.Model): @api.model def get_account_group(self, account_types): data = self._read_group( - [("account_type", "in", account_types), ("company_id", "=", self.env.company.id)], + [ + ("account_type", "in", account_types), + ("company_id", "=", self.env.company.id), + ], ["code:array_agg"], ["account_type"], ) diff --git a/addons/spreadsheet_account/static/src/accounting_datasource.js b/addons/spreadsheet_account/static/src/accounting_datasource.js index 4f3200d6534..ffbd93f3cd4 100644 --- a/addons/spreadsheet_account/static/src/accounting_datasource.js +++ b/addons/spreadsheet_account/static/src/accounting_datasource.js @@ -1,5 +1,5 @@ /** @odoo-module */ -import { camelToSnakeObject, sum, toServerDateString } from "@spreadsheet/helpers/helpers"; +import { camelToSnakeObject, toServerDateString } from "@spreadsheet/helpers/helpers"; import { _t } from "@web/core/l10n/translation"; import { sprintf } from "@web/core/utils/strings"; @@ -89,29 +89,11 @@ export class AccountingDataSource { if (dateRange.year < 1900) { throw new Error(sprintf(_t("%s is not a valid year."), dateRange.year)); } - const results = []; - let error = undefined; - // If some payload were to raise an error, we still need to process the others - // to make sure they are part of the next batch call. - for (const code of codes) { - try { - const result = this.serverData.batch.get( - "account.account", - "spreadsheet_fetch_debit_credit", - camelToSnakeObject({ dateRange, code, companyId, includeUnposted }) - ); - results.push(result); - } catch (err) { - error = error || err; - } - } - if (error) { - throw error; - } - return { - debit: sum(results.map((values) => values.debit)), - credit: sum(results.map((values) => values.credit)), - }; + return this.serverData.batch.get( + "account.account", + "spreadsheet_fetch_debit_credit", + camelToSnakeObject({ dateRange, codes, companyId, includeUnposted }) + ); } /** diff --git a/addons/spreadsheet_account/static/tests/model/accounting_tests.js b/addons/spreadsheet_account/static/tests/model/accounting_tests.js index 7d959fd83de..3a4ecbb5f31 100644 --- a/addons/spreadsheet_account/static/tests/model/accounting_tests.js +++ b/addons/spreadsheet_account/static/tests/model/accounting_tests.js @@ -93,10 +93,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { mockRPC: async function (route, args) { if (args.method === "spreadsheet_fetch_debit_credit") { assert.step("spreadsheet_fetch_debit_credit"); - return [ - { debit: 42, credit: 16 }, - { debit: 100, credit: 10 }, - ]; + return [{ debit: 142, credit: 26 }]; } }, }); @@ -152,7 +149,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "100", + codes: ["100"], companyId: null, includeUnposted: true, }) @@ -160,7 +157,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("01/2022"), - code: "100", + codes: ["100"], companyId: null, includeUnposted: true, }) @@ -168,7 +165,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("Q2/2022"), - code: "100", + codes: ["100"], companyId: null, includeUnposted: true, }) @@ -176,7 +173,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2021"), - code: "10", + codes: ["10"], companyId: null, includeUnposted: true, }) @@ -184,7 +181,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2021"), - code: "5", + codes: ["5"], companyId: 2, includeUnposted: true, }) @@ -192,7 +189,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("05/04/2022"), - code: "5", + codes: ["5"], companyId: null, includeUnposted: true, }) @@ -200,7 +197,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "5", + codes: ["5"], companyId: null, includeUnposted: false, }) @@ -208,7 +205,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("05/05/2022"), - code: "100", + codes: ["100"], companyId: null, includeUnposted: true, }) @@ -238,15 +235,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "100", - companyId: null, - includeUnposted: true, - }) - ), - JSON.stringify( - camelToSnakeObject({ - dateRange: parseAccountingDate("2022"), - code: "200", + codes: ["100", "200"], companyId: null, includeUnposted: true, }) @@ -279,15 +268,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "100104", - companyId: null, - includeUnposted: true, - }) - ), - JSON.stringify( - camelToSnakeObject({ - dateRange: parseAccountingDate("2022"), - code: "200104", + codes: ["100104", "200104"], companyId: null, includeUnposted: true, }) @@ -327,7 +308,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "100", + codes: ["100"], companyId: null, includeUnposted: true, }) @@ -336,15 +317,7 @@ QUnit.module("spreadsheet_account > Accounting", { beforeEach }, () => { JSON.stringify( camelToSnakeObject({ dateRange: parseAccountingDate("2022"), - code: "100104", - companyId: null, - includeUnposted: true, - }) - ), - JSON.stringify( - camelToSnakeObject({ - dateRange: parseAccountingDate("2022"), - code: "200104", + codes: ["100104", "200104"], companyId: null, includeUnposted: true, }) diff --git a/addons/spreadsheet_account/tests/test_debit_credit.py b/addons/spreadsheet_account/tests/test_debit_credit.py index 72e01ab78b3..fa8bdeece20 100644 --- a/addons/spreadsheet_account/tests/test_debit_credit.py +++ b/addons/spreadsheet_account/tests/test_debit_credit.py @@ -113,7 +113,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -122,13 +122,75 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): [{"credit": 0.0, "debit": 500.0}], ) + def test_two_codes(self): + self.assertEqual( + self.env["account.account"].spreadsheet_fetch_debit_credit( + [ + { + "date_range": { + "range_type": "year", + "year": 2022, + }, + "codes": [self.account_revenue_c1.code, self.account_expense_c1.code], + "company_id": None, + "include_unposted": True, + } + ] + ), + [{"credit": 500, "debit": 500.0}], + ) + + def test_two_codes_mixing_balance(self): + self.account_revenue_c1.sudo().include_initial_balance = True + self.env["account.move"].create( + { + "company_id": self.company_data["company"].id, + "move_type": "entry", + "date": "2000-07-02", + "line_ids": [ + Command.create( + { + "name": "line_debit_c1", + "account_id": self.account_revenue_c1.id, + "debit": 555, + }, + ), + Command.create( + { + "name": "line_credit_c1", + "account_id": self.account_expense_c1.id, + # not taken into account because the account + # has include_initial_balance=False + "credit": 555, + }, + ), + ], + } + ) + self.assertEqual( + self.env["account.account"].spreadsheet_fetch_debit_credit( + [ + { + "date_range": { + "range_type": "year", + "year": 2022, + }, + "codes": [self.account_revenue_c1.code, self.account_expense_c1.code], + "company_id": None, + "include_unposted": True, + } + ] + ), + [{"credit": 500, "debit": 1055.0}], + ) + def test_response_order(self): request_1 = { "date_range": { "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -137,7 +199,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2020, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -160,7 +222,27 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": code, + "codes": [code], + "company_id": None, + "include_unposted": True, + } + ] + ), + [ + {"credit": 500.0, "debit": 500.0}, + ], + ) + + def test_duplicated_prefix_code(self): + self.assertEqual( + self.env["account.account"].spreadsheet_fetch_debit_credit( + [ + { + "date_range": { + "range_type": "year", + "year": 2022, + }, + "codes": ["sp1234", "sp1234"], "company_id": None, "include_unposted": True, } @@ -203,7 +285,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2021, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -246,7 +328,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2021, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -291,7 +373,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -333,7 +415,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "year": 2022, "quarter": 3, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -371,7 +453,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): [ { "date_range": {"range_type": "month", "year": 2022, "month": 7}, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -414,7 +496,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "month": 7, "day": 2, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -482,7 +564,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "month": 2, "day": 4, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -504,7 +586,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2025, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -549,7 +631,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "year": 2022, "quarter": 3, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -590,7 +672,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): [ { "date_range": {"range_type": "month", "year": 2022, "month": 7}, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -636,7 +718,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "month": 7, "day": 2, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -681,7 +763,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": False, } @@ -699,7 +781,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -744,7 +826,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": False, } @@ -762,7 +844,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": None, "include_unposted": True, } @@ -773,6 +855,26 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): ], ) + def test_empty_code(self): + self.assertEqual( + self.env["account.account"].spreadsheet_fetch_debit_credit( + [ + { + "date_range": { + "range_type": "year", + "year": 2022, + }, + "codes": [""], + "company_id": None, + "include_unposted": False, + } + ] + ), + [ + {"credit": 0, "debit": 0}, + ], + ) + def test_no_code(self): self.assertEqual( self.env["account.account"].spreadsheet_fetch_debit_credit( @@ -782,7 +884,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": "", + "codes": [], "company_id": None, "include_unposted": False, } @@ -800,7 +902,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": self.account_revenue_c1.code, + "codes": [self.account_revenue_c1.code], "company_id": self.account_revenue_c1.company_id.id, "include_unposted": True, } @@ -814,27 +916,21 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "views": [[False, "list"]], "target": "current", "domain": [ - "|", "&", "&", "&", ("account_id.code", "=like", "sp1234566%"), - ("company_id", "=", self.account_revenue_c1.company_id.id), - ("move_id.state", "!=", "cancel"), + "|", "&", ("account_id.include_initial_balance", "=", True), ("date", "<=", date(2022, 12, 31)), "&", "&", - "&", - ("account_id.code", "=like", "sp1234566%"), - ("company_id", "=", self.account_revenue_c1.company_id.id), - ("move_id.state", "!=", "cancel"), - "&", - "&", ("account_id.include_initial_balance", "=", False), ("date", ">=", date(2022, 1, 1)), ("date", "<=", date(2022, 12, 31)), + ("company_id", "=", self.account_revenue_c1.company_id.id), + ("move_id.state", "!=", "cancel"), ], "name": "Journal items for account prefix sp1234566", }, @@ -847,7 +943,7 @@ class SpreadsheetAccountingFunctionsTest(AccountTestInvoicingCommon): "range_type": "year", "year": 2022, }, - "code": "", + "codes": [""], "company_id": None, "include_unposted": True, }