From a03549d77171de5b487acd738abfb4516b649ac9 Mon Sep 17 00:00:00 2001 From: "Paolo Gatti (pgi)" Date: Thu, 9 Mar 2023 10:18:18 +0000 Subject: [PATCH] [IMP] account, account_edi, l10n_it_edi: allow link from imported VB to PO, fixes Every imported vendor bill now will have the chance to make its invoice_origin linked to a Purchase Order. The function is moved from account_journal to account_edi_format to allow the link being done from all webservices, thread attachments and upload. - Avoid mocking the proxy testing The test on the check that the same attachment is coming twice from the proxy doesn't actually need to test the proxy. By splitting the function, we avoid mocking the proxy for no added value. Added an ir.rule for companies to only look for their account_edi_proxy_client.users - PA Index label should be Destination Code PA Index is a completely wrong description. This is the destination "address" of the partner at which our EDI documents (invoices) should be directed to inside the SdI e-invoicing system, much like an IP address. It's not an index, doesn't have much to share with the Public Administration. The correct literal translation of the name should be "Destination Code" for Codice Destinatario. We have clients opening tickets because they don't recognize this field on the partner form because of the wrong translation. - Fixes on taxes import Lines didn't have their taxes cleared, so invoices actually added the taxes in the XML to the default supplier taxes of the product VAT taxes on import search was conflicting with actual withholding / pension fund taxes, so extra conditions are added in the search if withholding / pension fund fields are not specified Task link: https://www.odoo.com/web#id=3175353&model=project.task Task-3175353 closes odoo/odoo#114870 Forward-port-of: #111365 Signed-off-by: Josse Colpaert --- addons/account/models/account_move.py | 14 ++- addons/account_edi/models/account_journal.py | 9 -- .../account_edi_proxy_client/__manifest__.py | 1 + .../models/account_edi_proxy_user.py | 3 +- .../account_edi_proxy_client_security.xml | 10 ++ addons/l10n_it_edi/i18n/it.po | 8 +- addons/l10n_it_edi/i18n/l10n_it_edi.pot | 4 +- .../l10n_it_edi/models/account_edi_format.py | 113 +++++++++++------- addons/l10n_it_edi/models/account_invoice.py | 2 +- addons/l10n_it_edi/models/res_partner.py | 4 +- addons/l10n_it_edi/tests/common.py | 8 -- addons/l10n_it_edi/tests/test_edi_import.py | 48 ++++---- .../models/account_edi_format.py | 30 +++-- 13 files changed, 148 insertions(+), 106 deletions(-) create mode 100644 addons/account_edi_proxy_client/security/account_edi_proxy_client_security.xml diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 1e39c7eeaad..4f4d5c70afb 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2852,9 +2852,9 @@ class AccountMove(models.Model): """Main entry point to extend/enhance invoices with attachments. Either coming from the chatter or the journal. It will unwrap all - attachemnts by priority then try to decode untill it succeed. + attachments by priority then try to decode until it succeed. - :returns: True if at least one doccument successfully imported + :returns: True if at least one document is successfully imported """ success = False @@ -2864,9 +2864,11 @@ class AccountMove(models.Model): try: if decoder and not success: - with self._get_edi_creation() as invoice: + with self.env.cr.savepoint(), self._get_edi_creation() as invoice: # pylint: disable=not-callable success = decoder(invoice, file_data, new) + if success: + invoice._link_bill_origin_to_purchase_orders(timeout=4) except RedirectWarning as rw: raise rw @@ -3503,6 +3505,12 @@ class AccountMove(models.Model): # hook to be used with purchase, so that vendor bills are sync/autocompleted with purchase orders self.ensure_one() + def _link_bill_origin_to_purchase_orders(self, timeout=10): + for move in self.filtered(lambda m: m.move_type in self.get_purchase_types()): + references = [move.invoice_origin] if move.invoice_origin else [] + move._find_and_set_purchase_orders(references, move.partner_id.id, move.amount_total, timeout) + return self + # ------------------------------------------------------------------------- # PUBLIC ACTIONS # ------------------------------------------------------------------------- diff --git a/addons/account_edi/models/account_journal.py b/addons/account_edi/models/account_journal.py index 268fe38ed17..be5d975d79f 100644 --- a/addons/account_edi/models/account_journal.py +++ b/addons/account_edi/models/account_journal.py @@ -81,12 +81,3 @@ class AccountJournal(models.Model): protected_edi_formats = journal.edi_format_ids.filtered(lambda e: e.id in protected_edi_format_ids) journal.edi_format_ids = enabled_edi_formats + protected_edi_formats - - def _create_document_from_attachment(self, attachment_ids): - # tries to match purchasing orders - moves = super()._create_document_from_attachment(attachment_ids) - for move in moves: - if move.move_type == 'in_invoice': - references = [move.invoice_origin] if move.invoice_origin else [] - move._find_and_set_purchase_orders(references, move.partner_id.id, move.amount_total, timeout=4) - return moves diff --git a/addons/account_edi_proxy_client/__manifest__.py b/addons/account_edi_proxy_client/__manifest__.py index 6e5104bcfa9..478a52fe9ed 100644 --- a/addons/account_edi_proxy_client/__manifest__.py +++ b/addons/account_edi_proxy_client/__manifest__.py @@ -17,6 +17,7 @@ Odoo database. }, 'data': [ 'security/ir.model.access.csv', + 'security/account_edi_proxy_client_security.xml', ], 'installable': True, 'license': 'LGPL-3', diff --git a/addons/account_edi_proxy_client/models/account_edi_proxy_user.py b/addons/account_edi_proxy_client/models/account_edi_proxy_user.py index 57b2f825f39..8250d38d170 100644 --- a/addons/account_edi_proxy_client/models/account_edi_proxy_user.py +++ b/addons/account_edi_proxy_client/models/account_edi_proxy_user.py @@ -99,8 +99,7 @@ class AccountEdiProxyClientUser(models.Model): error_code = proxy_error['code'] if error_code == 'refresh_token_expired': self._renew_token() - if not self.env.context.get('test_skip_commit'): - self.env.cr.commit() # We do not want to lose it if in the _make_request below something goes wrong + self.env.cr.commit() # We do not want to lose it if in the _make_request below something goes wrong return self._make_request(url, params) if error_code == 'no_such_user': # This error is also raised if the user didn't exchange data and someone else claimed the edi_identificaiton. diff --git a/addons/account_edi_proxy_client/security/account_edi_proxy_client_security.xml b/addons/account_edi_proxy_client/security/account_edi_proxy_client_security.xml new file mode 100644 index 00000000000..8ac46b1145c --- /dev/null +++ b/addons/account_edi_proxy_client/security/account_edi_proxy_client_security.xml @@ -0,0 +1,10 @@ + + + + + Account EDI Proxy Client User + + [('company_id', 'in', company_ids)] + + + diff --git a/addons/l10n_it_edi/i18n/it.po b/addons/l10n_it_edi/i18n/it.po index a0b5da21802..eae7f36afd0 100644 --- a/addons/l10n_it_edi/i18n/it.po +++ b/addons/l10n_it_edi/i18n/it.po @@ -638,13 +638,13 @@ msgstr "Notifica esito: %s
%s" #. module: l10n_it_edi #: model:ir.model.fields,field_description:l10n_it_edi.field_res_partner__l10n_it_pa_index #: model:ir.model.fields,field_description:l10n_it_edi.field_res_users__l10n_it_pa_index -msgid "PA index" -msgstr "Codice univoco" +msgid "Destination Code" +msgstr "Codice Destinatario" #. module: l10n_it_edi #: model:ir.model.constraint,message:l10n_it_edi.constraint_res_partner_l10n_it_pa_index -msgid "PA index must have between 6 and 7 characters." -msgstr "Il codice univoco deve avere 6/7 caratteri" +msgid "Destination Code must have between 6 and 7 characters." +msgstr "Il Codice Destinatario deve avere 6/7 caratteri" #. module: l10n_it_edi #: model_terms:ir.ui.view,arch_db:l10n_it_edi.account_invoice_it_FatturaPA_export diff --git a/addons/l10n_it_edi/i18n/l10n_it_edi.pot b/addons/l10n_it_edi/i18n/l10n_it_edi.pot index 6f668e4515f..387f04d4f0c 100644 --- a/addons/l10n_it_edi/i18n/l10n_it_edi.pot +++ b/addons/l10n_it_edi/i18n/l10n_it_edi.pot @@ -509,12 +509,12 @@ msgstr "" #. module: l10n_it_edi #: model:ir.model.fields,field_description:l10n_it_edi.field_res_partner__l10n_it_pa_index #: model:ir.model.fields,field_description:l10n_it_edi.field_res_users__l10n_it_pa_index -msgid "PA index" +msgid "Destination Code" msgstr "" #. module: l10n_it_edi #: model:ir.model.constraint,message:l10n_it_edi.constraint_res_partner_l10n_it_pa_index -msgid "PA index must have between 6 and 7 characters." +msgid "Destination Code must have between 6 and 7 characters." msgstr "" #. module: l10n_it_edi diff --git a/addons/l10n_it_edi/models/account_edi_format.py b/addons/l10n_it_edi/models/account_edi_format.py index 55cf020820d..41a2c7a307b 100644 --- a/addons/l10n_it_edi/models/account_edi_format.py +++ b/addons/l10n_it_edi/models/account_edi_format.py @@ -290,63 +290,76 @@ class AccountEdiFormat(models.Model): # ------------------------------------------------------------------------- def _cron_receive_fattura_pa(self): - ''' Check the proxy for incoming invoices. + ''' Check the proxy for incoming invoices for all companies. ''' - proxy_users = self.env['account_edi_proxy_client.user'].search([('edi_format_id', '=', self.env.ref('l10n_it_edi.edi_fatturaPA').id)]) - - if proxy_users._get_demo_state() == 'demo': + if self.env['account_edi_proxy_client.user']._get_demo_state() == 'demo': return - for proxy_user in proxy_users: - company = proxy_user.company_id + for proxy_user in self.env['account_edi_proxy_client.user'].search([('edi_format_code', '=', 'fattura_pa')]): + self._receive_fattura_pa(proxy_user) + + def _receive_fattura_pa(self, proxy_user): + ''' Check the proxy for incoming invoices for a specified proxy user. + ''' + try: + res = proxy_user._make_request( + proxy_user._get_server_url() + '/api/l10n_it_edi/1/in/RicezioneInvoice', + params={'recipient_codice_fiscale': proxy_user.company_id.l10n_it_codice_fiscale}) + except AccountEdiProxyError as e: + res = {} + _logger.error('Error while receiving file from SdiCoop: %s', e) + + proxy_acks = [] + for id_transaction, fattura in res.items(): + if self._save_incoming_attachment_fattura_pa(proxy_user, id_transaction, fattura['filename'], fattura['file'], fattura['key']): + proxy_acks.append(id_transaction) + + if proxy_acks: try: - res = proxy_user._make_request(proxy_user._get_server_url() + '/api/l10n_it_edi/1/in/RicezioneInvoice', - params={'recipient_codice_fiscale': company.l10n_it_codice_fiscale}) + proxy_user._make_request( + proxy_user._get_server_url() + '/api/l10n_it_edi/1/ack', + params={'transaction_ids': proxy_acks}) except AccountEdiProxyError as e: - res = {} _logger.error('Error while receiving file from SdiCoop: %s', e) - proxy_acks = [] - for id_transaction, fattura in res.items(): - if self.env['ir.attachment'].search([('name', '=', fattura['filename']), ('res_model', '=', 'account.move')], limit=1): - # name should be unique, the invoice already exists - _logger.info('E-invoice already exists: %s', fattura['filename']) - proxy_acks.append(id_transaction) - continue + def _save_incoming_attachment_fattura_pa(self, proxy_user, id_transaction, filename, content, key): + ''' Save an incoming file from the SdI as an attachment. + Commits if successful. - file = proxy_user._decrypt_data(fattura['file'], fattura['key']) + :param proxy_user: the user that saves the attachment. + :param id_transaction: id of the SdI transaction for communication with the IAP proxy. + :param filename: name of the file to be saved. + :param content: encrypted content of the file to be saved. + :param key: key to decrypt the file. + :returns: True if everything went well, or the file already exists. + False if the file cannot be parsed as an XML. + ''' - try: - tree = etree.fromstring(file) - except Exception: - # should not happen as the file has been checked by SdiCoop - _logger.info('Received file badly formatted, skipping: \n %s', file) - continue + if self.env['ir.attachment'].search([('name', '=', filename), ('res_model', '=', 'account.move')], limit=1): + # name should be unique, the invoice already exists + _logger.info('E-invoice already exists: %s', filename) + return True - attachment = self.env['ir.attachment'].create({ - 'name': fattura['filename'], - 'raw': file, - 'type': 'binary', - }) + decrypted_content = proxy_user._decrypt_data(content, key) + attachment = self.env['ir.attachment'].create({ + 'name': filename, + 'raw': decrypted_content, + 'type': 'binary' + }) - if not self.env.context.get('test_skip_commit'): - self.env.cr.commit() # In case something fails after, we still have the attachment + # Import the move from the attachment. + # `_create_document_from_attachment` will create an empty move + # then try and fill it with the content imported from the attachment. + # Should the import fail, thanks to try..except and savepoint, + # we will anyway end up with an empty `in_invoice` with the attachment posted on it. + (self.env['account.journal'].with_company(proxy_user.company_id) + .with_context(default_move_type='in_invoice') + ._create_document_from_attachment(attachment.ids)) - self.env['account.journal'].with_company(company)\ - .with_context(default_move_type='in_invoice')\ - ._create_document_from_attachment(attachment.ids) + # Commit the created move, be it correctly imported or empty. + self.env.cr.commit() - proxy_acks.append(id_transaction) - if not self.env.context.get('test_skip_commit'): - self.env.cr.commit() - - - if proxy_acks: - try: - proxy_user._make_request(proxy_user._get_server_url() + '/api/l10n_it_edi/1/ack', - params={'transaction_ids': proxy_acks}) - except AccountEdiProxyError as e: - _logger.error('Error while receiving file from SdiCoop: %s', e) + return True def _check_filename_is_fattura_pa(self, filename): return re.search("[A-Z]{2}[A-Za-z0-9]{2,28}_[A-Za-z0-9]{0,5}.((?i:xml.p7m|xml))", filename) @@ -525,6 +538,13 @@ class AccountEdiFormat(models.Model): invoice._compose_info_message(elements[0], '.') )) + # Information related to the purchase order <2.1.2> + po_refs = [] + elements = tree.xpath('//DatiGenerali/DatiOrdineAcquisto/IdDocumento') + if elements: + po_refs = [element.text.strip() for element in elements] + invoice.invoice_origin = ", ".join(po_refs) + # Total amount. <2.4.2.6> elements = tree.xpath('.//ImportoPagamento') amount_total_import = 0 @@ -575,7 +595,10 @@ class AccountEdiFormat(models.Model): elements = tree.xpath('.//DatiBeniServizi') for element in (elements or []): - invoice_line_form = invoice.invoice_line_ids.create({'move_id': invoice.id}) + invoice_line_form = invoice.invoice_line_ids.create({ + 'move_id': invoice.id, + 'tax_ids': [fields.Command.clear()], + }) if invoice_line_form: message_to_log += self._import_fattura_pa_line(element, invoice_line_form, extra_info) diff --git a/addons/l10n_it_edi/models/account_invoice.py b/addons/l10n_it_edi/models/account_invoice.py index 6b5d122f6b0..ec7e356d82d 100644 --- a/addons/l10n_it_edi/models/account_invoice.py +++ b/addons/l10n_it_edi/models/account_invoice.py @@ -333,7 +333,7 @@ class AccountMove(models.Model): self.env['account.edi.format']._import_fattura_pa(tree_list[0], invoice) # there might be other invoices inside the EDI. - for tree in tree_list[:1]: + for tree in tree_list[1:]: other_invoice = invoice.create({ 'journal_id': invoice.journal_id.id, }) diff --git a/addons/l10n_it_edi/models/res_partner.py b/addons/l10n_it_edi/models/res_partner.py index 300c6645fdf..36e8cd26904 100644 --- a/addons/l10n_it_edi/models/res_partner.py +++ b/addons/l10n_it_edi/models/res_partner.py @@ -14,7 +14,7 @@ class ResPartner(models.Model): l10n_it_pec_email = fields.Char(string="PEC e-mail") l10n_it_codice_fiscale = fields.Char(string="Codice Fiscale", size=16) - l10n_it_pa_index = fields.Char(string="PA index", + l10n_it_pa_index = fields.Char(string="Destination Code", size=7, help="Must contain the 6-character (or 7) code, present in the PA\ Index in the information relative to the electronic invoicing service,\ @@ -28,7 +28,7 @@ class ResPartner(models.Model): ('l10n_it_pa_index', "CHECK(l10n_it_pa_index IS NULL OR l10n_it_pa_index = '' OR LENGTH(l10n_it_pa_index) >= 6)", - "PA index must have between 6 and 7 characters."), + "Destination Code must have between 6 and 7 characters."), ] @api.model diff --git a/addons/l10n_it_edi/tests/common.py b/addons/l10n_it_edi/tests/common.py index de95ea23cdf..07beff11288 100644 --- a/addons/l10n_it_edi/tests/common.py +++ b/addons/l10n_it_edi/tests/common.py @@ -9,14 +9,6 @@ from odoo.tests import tagged from odoo.addons.account_edi.tests.common import AccountEdiTestCommon from odoo.addons.account_edi_proxy_client.models.account_edi_proxy_user import AccountEdiProxyClientUser -def patch_proxy_user(func): - @patch.object(AccountEdiProxyClientUser, '_make_request', MagicMock(spec=AccountEdiProxyClientUser._make_request)) - @patch.object(AccountEdiProxyClientUser, '_decrypt_data', MagicMock(spec=AccountEdiProxyClientUser._decrypt_data)) - @patch.object(AccountEdiProxyClientUser, '_get_demo_state', MagicMock(spec=AccountEdiProxyClientUser._get_demo_state)) - def patched(self, *args, **kwargs): - return func(self, *args, **kwargs) - return patched - @tagged('post_install_l10n', 'post_install', '-at_install') class TestItEdi(AccountEdiTestCommon): diff --git a/addons/l10n_it_edi/tests/test_edi_import.py b/addons/l10n_it_edi/tests/test_edi_import.py index 0f0917062be..daf398306d1 100644 --- a/addons/l10n_it_edi/tests/test_edi_import.py +++ b/addons/l10n_it_edi/tests/test_edi_import.py @@ -1,9 +1,11 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import fields +from unittest.mock import MagicMock, patch + +from odoo import fields, sql_db from odoo.tests import tagged -from odoo.addons.l10n_it_edi.tests.common import TestItEdi, patch_proxy_user +from odoo.addons.l10n_it_edi.tests.common import TestItEdi @tagged('post_install_l10n', 'post_install', '-at_install') @@ -68,31 +70,31 @@ class TestItEdiImport(TestItEdi): }], }]) - @patch_proxy_user def test_receive_same_vendor_bill_twice(self): """ Test that the second time we are receiving an SdiCoop invoice, the second is discarded """ - # The make_request function is called twice when running the _cron_receive_fattura_pa - # first to the /in/RicezioneInvoice endpoint (to find new incoming invoices) - # second to the /api/l10n_it_edi/1/ack to acknowledge the invoices have been recieved + fattura_pa = self.env.ref('l10n_it_edi.edi_fatturaPA') content = self.fake_test_content.encode() - fake_responses = [ - # Response of the format id_transaction: fattura dict - {'9999999999': {'filename': 'IT01234567890_FPR02.xml', 'key': '123', 'file': content}}, - # The response from the _make_request for the ack can be None - None, - ] * 2 # Since the cron is run twice, and we want the fake results both times - self.proxy_user._make_request.side_effect = fake_responses - # When calling the decrypt function, the file we're accessing is already decrypted, just return the file - self.proxy_user._decrypt_data.side_effect = lambda file, _key: file - # In order for the cron function to progress to the point that it imports, we cannot be in demo mode - self.proxy_user._get_demo_state.return_value = 'unit_test' - # TODO: we need to see it does not apply to non-test companies which it does right now (missing security rule on client user) + # Our test content is not encrypted + proxy_user = MagicMock() + proxy_user.company_id = self.company + proxy_user._decrypt_data.return_value = content + + def mock_commit(self): + pass + + with patch.object(sql_db.Cursor, "commit", mock_commit): + for dummy in range(2): + fattura_pa._save_incoming_attachment_fattura_pa( + proxy_user=proxy_user, + id_transaction='9999999999', + filename='IT01234567890_FPR02.xml', + content=content, + key=None) - #self.edi_format.with_context({'test_skip_commit': True}).sudo()._cron_receive_fattura_pa() # There should be one attachement with this filename - #attachment = self.env['ir.attachment'].search([('name', '=', self.invoice_filename2)]) - #self.assertEqual(len(attachment), 1) - #invoice = self.env['account.move'].search([('payment_reference', '=', 'TWICE_TEST')]) - #self.assertEqual(len(invoice), 1) + attachments = self.env['ir.attachment'].search([('name', '=', 'IT01234567890_FPR02.xml')]) + self.assertEqual(len(attachments), 1) + invoices = self.env['account.move'].search([('payment_reference', '=', 'TWICE_TEST')]) + self.assertEqual(len(invoices), 1) diff --git a/addons/l10n_it_edi_withholding/models/account_edi_format.py b/addons/l10n_it_edi_withholding/models/account_edi_format.py index e8f118b75e0..7ea839bdc74 100644 --- a/addons/l10n_it_edi_withholding/models/account_edi_format.py +++ b/addons/l10n_it_edi_withholding/models/account_edi_format.py @@ -11,6 +11,14 @@ _logger = logging.getLogger(__name__) class AccountEdiFormat(models.Model): _inherit = 'account.edi.format' + def _l10n_it_edi_search_tax_for_import(self, company, percentage, extra_domain=None, vat_only=True): + """ In case no withholding_type or pension_fund is specified, exclude taxes that have it. + It means that we're searching for VAT taxes, especially in the base l10n_it_edi module + """ + if vat_only: + extra_domain += [('l10n_it_withholding_type', '=', False), ('l10n_it_pension_fund_type', '=', False)] + return super()._l10n_it_edi_search_tax_for_import(company, percentage, extra_domain) + def _l10n_it_edi_check_taxes_configuration(self, invoice): """ Override to also allow pension_fund, withholding taxes. @@ -38,9 +46,12 @@ class AccountEdiFormat(models.Model): withholding_type = tipo_ritenuta.text if tipo_ritenuta is not None else "RT02" withholding_reason = reason.text if reason is not None else "A" withholding_percentage = -float(percentage.text if percentage is not None else "0.0") - withholding_tax = self._l10n_it_edi_search_tax_for_import(company, withholding_percentage, + withholding_tax = self._l10n_it_edi_search_tax_for_import( + company, + withholding_percentage, [('l10n_it_withholding_type', '=', withholding_type), - ('l10n_it_withholding_reason', '=', withholding_reason)]) + ('l10n_it_withholding_reason', '=', withholding_reason)], + vat_only=False) if withholding_tax: withholding_taxes.append(withholding_tax) else: @@ -59,8 +70,11 @@ class AccountEdiFormat(models.Model): pension_fund_type = pension_fund_type.text if pension_fund_type is not None else "" tax_factor_percent = float(tax_factor_percent.text or "0.0") vat_tax_factor_percent = float(vat_tax_factor_percent.text or "0.0") - pension_fund_tax = self._l10n_it_edi_search_tax_for_import(company, tax_factor_percent, - [('l10n_it_pension_fund_type', '=', pension_fund_type)]) + pension_fund_tax = self._l10n_it_edi_search_tax_for_import( + company, + tax_factor_percent, + [('l10n_it_pension_fund_type', '=', pension_fund_type)], + vat_only=False) if pension_fund_tax: pension_fund_taxes.append(pension_fund_tax) else: @@ -100,9 +114,11 @@ class AccountEdiFormat(models.Model): continue enasarco_amount = float(number_text) enasarco_percentage = -self.env.company.currency_id.round(enasarco_amount / price_subtotal * 100) - enasarco_tax = self._l10n_it_edi_search_tax_for_import(company, enasarco_percentage, [ - ('l10n_it_pension_fund_type', '=', 'TC07'), - ]) + enasarco_tax = self._l10n_it_edi_search_tax_for_import( + company, + enasarco_percentage, + [('l10n_it_pension_fund_type', '=', 'TC07')], + vat_only=False) if enasarco_tax: invoice_line_form.tax_ids |= enasarco_tax else: