[FIX] payment: properly (re)create journals for providers

This commit fixes two bugs, the first one is a reinstall bug that
happens whenever the modules `payment` or `payment_test` are uninstalled
then reinstalled and the second one is a bug in which for some payment
acquirers, a journal is never created.

For the first bug, the problem is that when uninstalling the
aforementioned modules, the journals linked to each provider are not
deleted (which is ok from a business POV) and thus when reinstalling
said modules we simply recreate new journals, but journals have a
unicity constraint on (name, code, company_id), therefore the
re-creation of these journals will most likely fail.

This bug doesn't happen with other payment_* modules because all *main*
acquirers are defined in the payment module, except for payment_test
that defines its own acquirer (thus the fact that it fails to reinstall
too).

The solution chosen is to try to force the creation of the journals,
even if it may fail, within a try/except block: if it does fail, we
simply do a lookup for matching (name, code, company_id) and assign
whatever we find to the acquirer's journal_id. This approach was chosen
because the most common case is that of an install (journal creation),
so checking for existing journals first would make the most common case
less performant.

For the second bug, the part of the code that creates journals for
providers being installed made a false assumption: To find the acquirers
for which to create journals, it would gather the name of the
acquirer/provider from the module name of the modules being installed
(or that were already installed) and would compare them to existing
acquirers whose provider would match the names extracted from the module
name. However, not all payment_ modules contain the actual name of the
provider in the module name! payment_ingenico is one such case, while
the module name contains ingenico, the technical name for the provider
is 'ogone', meaning that the heuristic would never find an existing
acquirer with provider set to 'ingenico', therefore no journal would be
created for that specific payment provider.

The fix is trivial, payment.acquirer has related fields that point to
the modules that implement each provider, meaning that one can simply
check for all payment.acquirer whose module_state is 'to_install' or
'installed'.

This bug does not happen in v12 because when v12 was released ogone was
still ogone and not ingenico, and all payment acquirers actually
followed the convention of module name == provider name, but for
simplicity's sake we keep this change for v12 too since it's in the same
scope as for the first bug and the code is cleaner anyway.

closes odoo/odoo#62562

X-original-commit: 0b70f188ca441d6cf53b2ef63f4856239bbee8e4
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
Signed-off-by: Adrian Torres (adt) <adt@odoo.com>

1 files changed, 35 insertions, 11 deletions

diff --git a/addons/payment/models/payment_acquirer.py b/addons/payment/models/payment_acquirer.py
index 24cd4b44cdb..926f0603d28 100644
--- a/addons/payment/models/payment_acquirer.py
+++ b/addons/payment/models/payment_acquirer.py
@@ -6,6 +6,7 @@ import logging
 from datetime import datetime
 from dateutil import relativedelta
 import pprint
+import psycopg2
 
 from odoo import api, exceptions, fields, models, _, SUPERUSER_ID
 from odoo.tools import consteq, float_round, image_process, ustr
@@ -275,6 +276,16 @@ class PaymentAcquirer(models.Model):
             'outbound_payment_method_ids': [],
         }
 
+    def _get_acquirer_journal_domain(self):
+        """Returns a domain for finding a journal corresponding to an acquirer"""
+        self.ensure_one()
+        code_cutoff = self.env['account.journal']._fields['code'].size
+        return [
+            ('name', '=', self.name),
+            ('code', '=', self.name.upper()[:code_cutoff]),
+            ('company_id', '=', self.company_id.id),
+        ]
+
     @api.model
     def _create_missing_journal_for_acquirers(self, company=None):
         '''Create the journal for active acquirers.
@@ -283,22 +294,35 @@ class PaymentAcquirer(models.Model):
         (e.g. payment_paypal for Paypal). We can't do that in such modules because we have no guarantee the chart template
         is already installed.
         '''
-        # Search for installed acquirers modules.
+        # Search for installed acquirers modules that have no journal for the current company.
         # If this method is triggered by a post_init_hook, the module is 'to install'.
         # If the trigger comes from the chart template wizard, the modules are already installed.
-        acquirer_modules = self.env['ir.module.module'].search(
-            [('name', 'like', 'payment_%'), ('state', 'in', ('to install', 'installed'))])
-        acquirer_names = [a.name.split('_', 1)[1] for a in acquirer_modules]
-
-        # Search for acquirers having no journal
         company = company or self.env.company
-        acquirers = self.env['payment.acquirer'].search(
-            [('provider', 'in', acquirer_names), ('journal_id', '=', False), ('company_id', '=', company.id)])
+        acquirers = self.env['payment.acquirer'].search([
+            ('module_state', 'in', ('to install', 'installed')),
+            ('journal_id', '=', False),
+            ('company_id', '=', company.id),
+        ])
 
-        journals = self.env['account.journal']
+        # Here we will attempt to first create the journal since the most common case (first
+        # install) is to successfully to create the journal for the acquirer, in the case of a
+        # reinstall (least common case), the creation will fail because of a unique constraint
+        # violation, this is ok as we catch the error and then perform a search if need be
+        # and assign the existing journal to our reinstalled acquirer. It is better to ask for
+        # forgiveness than to ask for permission as this saves us the overhead of doing a select
+        # that would be useless in most cases.
+        Journal = journals = self.env['account.journal']
         for acquirer in acquirers.filtered(lambda l: not l.journal_id and l.company_id.chart_template_id):
-            acquirer.journal_id = self.env['account.journal'].create(acquirer._prepare_account_journal_vals())
-            journals += acquirer.journal_id
+            try:
+                with self.env.cr.savepoint():
+                    journal = Journal.create(acquirer._prepare_account_journal_vals())
+            except psycopg2.IntegrityError as e:
+                if e.pgcode == psycopg2.errorcodes.UNIQUE_VIOLATION:
+                    journal = Journal.search(acquirer._get_acquirer_journal_domain(), limit=1)
+                else:
+                    raise
+            acquirer.journal_id = journal
+            journals += journal
         return journals
 
     @api.model