From 1b3ad92231f024b17bf8aeb0641cf870e33fe8c4 Mon Sep 17 00:00:00 2001 From: Giovanni Francesco Capalbo Date: Mon, 13 Nov 2023 15:16:45 +0100 Subject: [PATCH 1/4] [ADD] Do not raise error if protected fields are unchanged. --- .../models/account_move_line.py | 48 +++++++++++++++++-- .../tests/test_l10n_nl_vat_statement.py | 16 ++++++- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/l10n_nl_tax_statement/models/account_move_line.py b/l10n_nl_tax_statement/models/account_move_line.py index 82c1c368a..2e53991f7 100644 --- a/l10n_nl_tax_statement/models/account_move_line.py +++ b/l10n_nl_tax_statement/models/account_move_line.py @@ -31,10 +31,52 @@ def _get_l10n_nl_vat_statement_protected_fields(self): "tax_tag_ids", ] + def check_field_is_equal(self, changed_protected_field, values): + self.ensure_one() + if self._fields.get(changed_protected_field) and self._fields[ + changed_protected_field + ].type not in ["many2many", "one2many"]: + return self[changed_protected_field] == values[changed_protected_field] + # if field is X2M we must translate commands to values. + # the only acceptable value is [[6,0,self[changed_protected_field].ids]] + # wich is what the web client posts in case there is a editable X2M in form + # that is unchanged. + return ( + values[changed_protected_field][0] == 6 + and values[changed_protected_field][2] == self[changed_protected_field].ids + ) + def write(self, values): - if self._l10n_nl_vat_statement_should_check_write(values): - self._l10n_nl_vat_statement_check_state() - return super().write(values) + # before doing anything we check the nl_vat_statement:check_state, + # if this is not allowed it will raise and no further operations + # are necessary. + if not self._l10n_nl_vat_statement_should_check_write(values): + return super().write(values) + # now we add code to check if any of the modified fields present in + # values are the same as existing field value. this is a known limitation + # of odoo web client, it passes all non-readonly value fields in a form + # for writing , even if the value has not been changed. + res = True + protected_fields = self._get_l10n_nl_vat_statement_protected_fields() + protected_fields_in_values = [x for x in values.keys() if x in protected_fields] + for this in self: + # there are protected fields in dict, we proceed to check if such keys + # are in reality leaving the field the same. + # we need to recreate this dict every time + clean_values = dict(values) + for changed_protected_field in protected_fields_in_values: + is_equal = this.check_field_is_equal( + changed_protected_field, clean_values + ) + if is_equal: + clean_values.pop(changed_protected_field) + # in case of recordset write we need to evaluate write one by one + # and re-check that our new clean dict is allowed + if this._l10n_nl_vat_statement_should_check_write(clean_values): + if this._l10n_nl_vat_statement_should_check_write(clean_values): + this._l10n_nl_vat_statement_check_state() + res = res and super(AccountMoveLine, this).write(clean_values) + return res @api.model def _l10n_nl_vat_statement_should_check_write(self, values): diff --git a/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py b/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py index e9d4454e2..518ca5449 100644 --- a/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py +++ b/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py @@ -2,6 +2,7 @@ # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). import datetime +import logging from dateutil.relativedelta import relativedelta @@ -13,6 +14,8 @@ from odoo.tests.common import TransactionCase from odoo.tools import convert_file +_logger = logging.getLogger(__name__) + class TestVatStatement(TransactionCase): def _load(self, module, *args): @@ -157,6 +160,17 @@ def _create_test_invoice(self): self.invoice_1 = invoice_form.save() self.assertEqual(len(self.invoice_1.line_ids), 5) + # testing noop writing now allowed + for line in self.invoice_1.line_ids: + values = {"name": line.name} + self.assertEqual(line.write(values), True) + self.assertEqual(line.check_field_is_equal("name", values), True) + + # testing no protected field code branch: + line = self.invoice_1.line_ids[0] + values = {"ref": 12345} + self.assertEqual(line.write(values), True) + def test_01_onchange(self): daterange_type = self.env["date.range.type"].create({"name": "Type 1"}) daterange = self.env["date.range"].create( @@ -355,7 +369,7 @@ def test_12_undeclared_invoice(self): ) self.assertTrue(invoice_lines) with self.assertRaises(UserError): - invoice_lines[0].date = fields.Date.today() + invoice_lines[0].date = fields.Date.today() + relativedelta(day=1) def test_13_no_previous_statement_posted(self): statement2 = self.env["l10n.nl.vat.statement"].create({"name": "Statement 2"}) From 6d657b932bed32d25dcdbbb7cc8c7c03839a0658 Mon Sep 17 00:00:00 2001 From: Giovanni Francesco Capalbo Date: Mon, 27 Nov 2023 11:11:24 +0100 Subject: [PATCH 2/4] [ADD] review fixes --- .../models/account_move_line.py | 67 ++++++++++--------- .../tests/test_l10n_nl_vat_statement.py | 5 ++ 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/l10n_nl_tax_statement/models/account_move_line.py b/l10n_nl_tax_statement/models/account_move_line.py index 2e53991f7..02ffe5c96 100644 --- a/l10n_nl_tax_statement/models/account_move_line.py +++ b/l10n_nl_tax_statement/models/account_move_line.py @@ -33,50 +33,53 @@ def _get_l10n_nl_vat_statement_protected_fields(self): def check_field_is_equal(self, changed_protected_field, values): self.ensure_one() - if self._fields.get(changed_protected_field) and self._fields[ - changed_protected_field - ].type not in ["many2many", "one2many"]: - return self[changed_protected_field] == values[changed_protected_field] - # if field is X2M we must translate commands to values. - # the only acceptable value is [[6,0,self[changed_protected_field].ids]] - # wich is what the web client posts in case there is a editable X2M in form - # that is unchanged. - return ( - values[changed_protected_field][0] == 6 - and values[changed_protected_field][2] == self[changed_protected_field].ids - ) + field = self._fields.get(changed_protected_field) + old_value = self[changed_protected_field] + new_value = values[changed_protected_field] + if field.type in ["many2many", "one2many"]: + # if field is X2M , the only acceptable value is + # [[6,0,self[changed_protected_field].ids]] + # wich is what the web client posts in case there is a editable X2M in form + # that is unchanged. + # If it is a list of tuple-commands, and the last one is precisely + # values[changed_protected_field][2] == self[changed_protected_field].ids + # we will not accept any modification to protected fields, + # only the standard 6,0,[ids] coming from the default web edit. + return ( + len(values[changed_protected_field]) == 1 + and values[changed_protected_field][0][0] == 6 + and values[changed_protected_field][0][2] + == self[changed_protected_field].ids + ) + if new_value: + return old_value == new_value + return bool(old_value) == bool(new_value) def write(self, values): # before doing anything we check the nl_vat_statement:check_state, - # if this is not allowed it will raise and no further operations - # are necessary. if not self._l10n_nl_vat_statement_should_check_write(values): return super().write(values) # now we add code to check if any of the modified fields present in # values are the same as existing field value. this is a known limitation # of odoo web client, it passes all non-readonly value fields in a form # for writing , even if the value has not been changed. - res = True protected_fields = self._get_l10n_nl_vat_statement_protected_fields() protected_fields_in_values = [x for x in values.keys() if x in protected_fields] + invalid_fields = set() for this in self: - # there are protected fields in dict, we proceed to check if such keys - # are in reality leaving the field the same. - # we need to recreate this dict every time - clean_values = dict(values) - for changed_protected_field in protected_fields_in_values: - is_equal = this.check_field_is_equal( - changed_protected_field, clean_values - ) - if is_equal: - clean_values.pop(changed_protected_field) - # in case of recordset write we need to evaluate write one by one - # and re-check that our new clean dict is allowed - if this._l10n_nl_vat_statement_should_check_write(clean_values): - if this._l10n_nl_vat_statement_should_check_write(clean_values): - this._l10n_nl_vat_statement_check_state() - res = res and super(AccountMoveLine, this).write(clean_values) - return res + for protected_field in protected_fields_in_values: + is_equal = this.check_field_is_equal(protected_field, values) + if not is_equal: + # if the field is invalid in even one + # of the records it cannot be popped + invalid_fields.add(protected_field) + for protected_field in protected_fields_in_values: + if protected_field not in invalid_fields: + values.pop(protected_field) + for this in self: + if this._l10n_nl_vat_statement_should_check_write(values): + this._l10n_nl_vat_statement_check_state() + return super(AccountMoveLine, self).write(values) @api.model def _l10n_nl_vat_statement_should_check_write(self, values): diff --git a/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py b/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py index 518ca5449..16fdec7ad 100644 --- a/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py +++ b/l10n_nl_tax_statement/tests/test_l10n_nl_vat_statement.py @@ -165,6 +165,11 @@ def _create_test_invoice(self): values = {"name": line.name} self.assertEqual(line.write(values), True) self.assertEqual(line.check_field_is_equal("name", values), True) + # testing noop writing on tax_id Many2Many allowed + for line in self.invoice_1.line_ids: + values = {"tax_ids": [(6, 0, line.tax_ids.ids)]} + self.assertEqual(line.check_field_is_equal("tax_ids", values), True) + self.assertEqual(line.write(values), True) # testing no protected field code branch: line = self.invoice_1.line_ids[0] From 2fdf41965538a6d22025f19b5038456d51cd3072 Mon Sep 17 00:00:00 2001 From: Giovanni Francesco Capalbo Date: Mon, 4 Dec 2023 10:48:14 +0100 Subject: [PATCH 3/4] fixup! [ADD] review fixes --- l10n_nl_tax_statement/models/account_move_line.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/l10n_nl_tax_statement/models/account_move_line.py b/l10n_nl_tax_statement/models/account_move_line.py index 02ffe5c96..d23a3df81 100644 --- a/l10n_nl_tax_statement/models/account_move_line.py +++ b/l10n_nl_tax_statement/models/account_move_line.py @@ -46,10 +46,9 @@ def check_field_is_equal(self, changed_protected_field, values): # we will not accept any modification to protected fields, # only the standard 6,0,[ids] coming from the default web edit. return ( - len(values[changed_protected_field]) == 1 - and values[changed_protected_field][0][0] == 6 - and values[changed_protected_field][0][2] - == self[changed_protected_field].ids + len(new_value) == 1 + and new_value[0][0] == 6 + and new_value[0][2] == old_value.ids ) if new_value: return old_value == new_value @@ -77,8 +76,7 @@ def write(self, values): if protected_field not in invalid_fields: values.pop(protected_field) for this in self: - if this._l10n_nl_vat_statement_should_check_write(values): - this._l10n_nl_vat_statement_check_state() + this._l10n_nl_vat_statement_check_state() return super(AccountMoveLine, self).write(values) @api.model From 2ec172d558c77dc9693e84a6ba2ece7cc34fb086 Mon Sep 17 00:00:00 2001 From: Giovanni Francesco Capalbo Date: Tue, 5 Dec 2023 10:10:32 +0100 Subject: [PATCH 4/4] [FIX] --- l10n_nl_tax_statement/models/account_move_line.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/l10n_nl_tax_statement/models/account_move_line.py b/l10n_nl_tax_statement/models/account_move_line.py index d23a3df81..3de356bc7 100644 --- a/l10n_nl_tax_statement/models/account_move_line.py +++ b/l10n_nl_tax_statement/models/account_move_line.py @@ -75,9 +75,10 @@ def write(self, values): for protected_field in protected_fields_in_values: if protected_field not in invalid_fields: values.pop(protected_field) - for this in self: - this._l10n_nl_vat_statement_check_state() - return super(AccountMoveLine, self).write(values) + if self._l10n_nl_vat_statement_should_check_write(values): + for this in self: + this._l10n_nl_vat_statement_check_state() + return super().write(values) @api.model def _l10n_nl_vat_statement_should_check_write(self, values):