[IMP] account: Store customer/supplier rank on partners

Purpose
=======

To correctly choose in which account a line should be posted,
we have to know if the partner is a customer or a supplier.

Specification
=============

Keep track of the number of account moves "in" and "out"
a partner has. These counts should be based on the posted
account moves. A customer that has been created from the
'Customer' menuitem will have a rank=1. When a customer
invoice will be created for him, the generated account moves
will be taken into account to compute its rank. The most
invoices we have for a partner, the higher his rank is.

Note: To avoid any concurrent update failures on the partner,
if one transaction has already locked a partner row, the count
update will be skipped that time.

This means the values may be approximative in the database!
The exact values will eventually be correctly computed at
the next successfull try.

Known limitation of this approach: The computation ignores
the set of currently selected companies. Actually, storing
context dependent values in the database is a bad practice,
and is avoided in that case by taking all the companies into
account.

Use the stored fields `customer_rank` and `supplier_rank`
to order partners when searching by name. This allows to show
best customers or best suppliers on top.

To choose if best customer or supplier are shown on top,
the context key `res_partner_search_mode` is used.
The context key can take two values: 'customer' or 'supplier'.
This decision partially reverts/revamps 8766f38 to only use
account moves instead of PO and SO

On actions showing partners, set a default filters to menus to
only display customers (customer_rank > 0) if the string is
"Customers", and only suppliers if the string is "Vendors" or
"Suppliers".

TaskID: 2049131

closes odoo/odoo#35942

Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>

11 files changed, 219 insertions, 26 deletions

diff --git a/addons/account/__manifest__.py b/addons/account/__manifest__.py
index fb30ac2ebe7..c1951b19666 100644
--- a/addons/account/__manifest__.py
+++ b/addons/account/__manifest__.py
@@ -29,13 +29,13 @@ You could use this simplified accounting in case you work with an (external) acc
         'views/account_move_views.xml',
         'wizard/setup_wizards_view.xml',
         'wizard/pos_box.xml',
+        'views/partner_view.xml',
         'views/account_view.xml',
         'views/report_statement.xml',
         'views/account_report.xml',
         'data/mail_template_data.xml',
         'wizard/account_validate_move_view.xml',
         'views/account_end_fy.xml',
-        'views/partner_view.xml',
         'views/product_view.xml',
         'views/account_analytic_view.xml',
         'views/account_tip_data.xml',
diff --git a/addons/account/models/partner.py b/addons/account/models/partner.py
index a12bfa72c4c..47f32f38f0c 100644
--- a/addons/account/models/partner.py
+++ b/addons/account/models/partner.py
@@ -1,14 +1,17 @@
 # -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from ast import literal_eval
-from operator import itemgetter
 import time
+import psycopg2
+import logging
 
 from odoo import api, fields, models, _
 from odoo.tools import DEFAULT_SERVER_DATETIME_FORMAT
 from odoo.exceptions import ValidationError
 from odoo.addons.base.models.res_partner import WARNING_MESSAGE, WARNING_HELP
 
+_logger = logging.getLogger(__name__)
+
 class AccountFiscalPosition(models.Model):
     _name = 'account.fiscal.position'
     _description = 'Fiscal Position'
@@ -434,6 +437,85 @@ class ResPartner(models.Model):
     trust = fields.Selection([('good', 'Good Debtor'), ('normal', 'Normal Debtor'), ('bad', 'Bad Debtor')], string='Degree of trust you have in this debtor', default='normal', company_dependent=True)
     invoice_warn = fields.Selection(WARNING_MESSAGE, 'Invoice', help=WARNING_HELP, default="no-message")
     invoice_warn_msg = fields.Text('Message for Invoice')
+    # Computed fields to order the partners as suppliers/customers according to the
+    # amount of their generated incoming/outgoing account moves 
+    supplier_rank = fields.Integer(compute='_compute_rank', store=True)
+    customer_rank = fields.Integer(compute='_compute_rank', store=True)
+
+    def _get_name_search_order_by_fields(self):
+        res = super()._get_name_search_order_by_fields()
+        partner_search_mode = self.env.context.get('res_partner_search_mode')
+        if not partner_search_mode in ('customer', 'supplier'):
+            return res
+        order_by_field = 'COALESCE(res_partner.%s, 0) DESC,'
+        if partner_search_mode == 'customer':
+            field = 'customer_rank'
+        else:
+            field = 'supplier_rank'
+
+        order_by_field = order_by_field % field
+        return '%s, %s' % (res, order_by_field % field) if res else order_by_field
+
+    @api.depends('invoice_ids', 'invoice_ids.state')
+    def _compute_rank(self):
+        """
+        The two field values may not be exact in the database!
+        To avoid any concurrent update failure while writing on the partner,
+        the count is not updated if the lock cannot be directly acquired.
+        The count will eventually be correctly computed, at the next successfull try.
+        """
+        types_in = ('in_invoice', 'in_refund', 'in_receipt')
+        types_out = ('out_invoice', 'out_refund', 'out_receipt')
+        partner_ids = tuple(self.ids)
+        queries_params = {
+            'supplier_rank': {
+                'partner_id': partner_ids,
+                'move_types': types_in
+            },
+            'customer_rank': {
+                'partner_id': partner_ids,
+                'move_types': types_out
+            },
+        }
+
+        # Update database in case an invoice is just created
+        self.flush()
+        for field, params in queries_params.items():
+            try:
+                with self.env.cr.savepoint():
+                    # Check if rows are not locked
+                    # Take this opportunity to retrieve the current partner ranks
+                    # If a customer has been created from the "Customer" menuitem
+                    # his rank should be equal to 1, even if no invoice has been
+                    # created for him yet
+                    self.env.cr.execute("""
+                        SELECT id, COALESCE({}, 0)
+                        FROM res_partner
+                        WHERE id IN %(partner_id)s
+                        FOR UPDATE NOWAIT
+                    """.format(field), params, log_exceptions=False)
+                    current_counts = dict(self.env.cr.fetchall())
+                    # Compute the real ranks, based on the posted
+                    # account moves generated from the invoices
+                    self.env.cr.execute("""
+                        SELECT m.partner_id, COUNT(*) AS move_count
+                        FROM account_move AS m
+                        WHERE
+                            type in %(move_types)s
+                            AND state = 'posted'
+                            AND partner_id IN %(partner_id)s
+                        GROUP BY m.partner_id
+                    """, params)
+                    counts = dict(self.env.cr.fetchall())
+                    # Update all ranks
+                    for partner in self:
+                        partner[field] = max(counts.get(partner.id, 0), current_counts[partner.id])
+            except psycopg2.DatabaseError as e:
+                if e.pgcode == '55P03':
+                    _logger.debug('Another transaction already locked partner rows. Cannot update partner ranks now.')
+                    continue
+                else:
+                    raise e
 
     def _compute_bank_count(self):
         bank_data = self.env['res.partner.bank'].read_group([('partner_id', 'in', self.ids)], ['partner_id'], ['partner_id'])
@@ -481,3 +563,16 @@ class ResPartner(models.Model):
             ('state', '=', 'posted')
         ], limit=1)
         return can_edit_vat and not (bool(has_invoice))
+
+    @api.model_create_multi
+    def create(self, vals_list):
+        search_partner_mode = self.env.context.get('res_partner_search_mode')
+        is_customer = search_partner_mode == 'customer'
+        is_supplier = search_partner_mode == 'supplier'
+        if search_partner_mode:
+            for vals in vals_list:
+                if is_customer and 'customer_rank' not in vals:
+                    vals['customer_rank'] = 1
+                elif is_supplier and 'supplier_rank' not in vals:
+                    vals['supplier_rank'] = 1
+        return super().create(vals_list)
diff --git a/addons/account/tests/__init__.py b/addons/account/tests/__init__.py
index e4b29d36e38..0cca1a3c3a1 100644
--- a/addons/account/tests/__init__.py
+++ b/addons/account/tests/__init__.py
@@ -5,6 +5,7 @@ from . import test_account_move_out_refund
 from . import test_account_move_in_invoice
 from . import test_account_move_in_refund
 from . import test_account_move_entry
+from . import test_account_move_partner_count
 from . import test_account_invoice_report
 from . import test_account_journal_dashboard
 from . import test_bank_statement_reconciliation
diff --git a/addons/account/tests/test_account_move_partner_count.py b/addons/account/tests/test_account_move_partner_count.py
new file mode 100644
index 00000000000..7ec2b6cc410
--- /dev/null
+++ b/addons/account/tests/test_account_move_partner_count.py
@@ -0,0 +1,63 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.tests.common import TransactionCase
+
+
+class TestAccountMovePartnerCount(TransactionCase):
+
+    def setUp(self):
+        super().setUp()
+
+        # T1 Create partner and account move
+        # (Use a new cursor to make sure creations are committed for later transactions)
+        with self.registry.cursor() as cr:
+            env = self.env(cr=cr)
+            self.partner = self.env['res.partner'].with_env(env).create({'name': 'Lucien'})
+            self.journal = self.env['account.journal'].with_env(env).create({
+                'name': 'Journal',
+                'code': '7775',
+                'type': 'sale',
+            })
+            self.move = self.env['account.move'].with_env(env).create({
+                'partner_id': self.partner.id,
+                'type': 'out_invoice',
+                'journal_id': self.journal.id,
+                'currency_id': self.env.user.company_id.currency_id.id,
+            })
+            self.line = self.env['account.move.line'].with_env(env).create({
+                'move_id': self.move.id,
+                'name': 'account move line test',
+            })
+
+    def tearDown(self):
+        super().tearDown()
+        with self.registry.cursor() as cr:
+            env = self.env(cr=cr)
+            move = self.move.with_env(env)
+            move.state = 'draft'
+            move.unlink()
+            self.journal.with_env(env).unlink()
+            self.partner.with_env(env).unlink()
+
+    def test_account_move_count(self):
+        # T2 create and post account move
+        with self.registry.cursor() as cr:
+            env = self.env(cr=cr)
+            self.move.with_env(env).post()
+            partner = self.partner.with_env(env)
+            self.assertEqual(partner.supplier_rank, 0)
+            self.assertEqual(partner.customer_rank, 1)
+
+    def test_account_move_count_concurrent(self):
+        # T2 Lock the partner row
+        with self.registry.cursor() as cr:
+            cr.execute("""SELECT id FROM res_partner WHERE id = %s FOR UPDATE""" % self.partner.id)
+
+            # T3 concurrently posts account move and tries to update partner
+            with self.registry.cursor() as cr:
+                env = self.env(cr=cr)
+                self.move.with_env(env).state = 'posted'
+
+                self.assertEqual(self.partner.with_env(env).customer_rank,
+                                 0, "It should not wait the concurrent transaction for the update")
diff --git a/addons/account/views/account_payment_view.xml b/addons/account/views/account_payment_view.xml
index 0d4e51852da..293905d347f 100644
--- a/addons/account/views/account_payment_view.xml
+++ b/addons/account/views/account_payment_view.xml
@@ -242,8 +242,12 @@
             <field name="name">Payments</field>
             <field name="res_model">account.payment</field>
             <field name="view_mode">tree,kanban,form,graph</field>
-            <field name="context">{'default_payment_type': 'inbound', 'default_partner_type': 'customer', 'search_default_inbound_filter': 1}</field>
-            <field name="domain">[]</field>
+            <field name="context">{
+                'default_payment_type': 'inbound',
+                'default_partner_type': 'customer',
+                'search_default_inbound_filter': 1,
+                'res_partner_search_mode': 'customer',
+            }</field>
             <field name="view_id" ref="view_account_payment_tree"/>
             <field name="help" type="html">
                 <p class="o_view_nocontent_smiling_face">
@@ -258,8 +262,12 @@
             <field name="name">Payments</field>
             <field name="res_model">account.payment</field>
             <field name="view_mode">tree,kanban,form,graph</field>
-            <field name="context">{'default_payment_type': 'outbound', 'default_partner_type': 'supplier', 'search_default_outbound_filter': 1}</field>
-            <field name="domain">[]</field>
+            <field name="context">{
+                'default_payment_type': 'outbound',
+                'default_partner_type': 'supplier',
+                'search_default_outbound_filter': 1,
+                'res_partner_search_mode': 'supplier',
+            }</field>
             <field name="view_id" ref="view_account_supplier_payment_tree"/>
             <field name="help" type="html">
                 <p class="o_view_nocontent_smiling_face">
diff --git a/addons/account/views/account_view.xml b/addons/account/views/account_view.xml
index 0fec57c4c87..f744863e4b7 100644
--- a/addons/account/views/account_view.xml
+++ b/addons/account/views/account_view.xml
@@ -1723,7 +1723,7 @@ action = model.setting_init_bank_account_action()
 
         <menuitem id="menu_account_customer" name="Customers"
             parent="account.menu_finance_receivables"
-            action="base.action_partner_customer_form" sequence="110"/>
+            action="res_partner_action_customer" sequence="110"/>
 
         <record id="product_product_view_tree" model="ir.ui.view">
             <field name="name">product.product.tree</field>
diff --git a/addons/account/views/partner_view.xml b/addons/account/views/partner_view.xml
index 9b90e8fec88..6550a595192 100644
--- a/addons/account/views/partner_view.xml
+++ b/addons/account/views/partner_view.xml
@@ -204,5 +204,47 @@
                 </group>
             </field>
         </record>
+
+        <record id="res_partner_view_search" model="ir.ui.view">
+            <field name="name">res.partner.search.inherit</field>
+            <field name="model">res.partner</field>
+            <field name="inherit_id" ref="base.view_res_partner_filter"/>
+            <field name="arch" type="xml">
+                <xpath expr="//filter[@name='inactive']" position="before">
+                   <filter string="Customers" name="customer" domain="[('customer_rank','>', 0)]"/>
+                   <filter string="Vendors" name="supplier" domain="[('supplier_rank','>', 0)]"/>
+                </xpath>
+            </field>
+        </record>
+
+        <record id="res_partner_action_customer" model="ir.actions.act_window">
+            <field name="name">Customers</field>
+            <field name="type">ir.actions.act_window</field>
+            <field name="res_model">res.partner</field>
+            <field name="view_mode">kanban,tree,form</field>
+            <field name="context">{'search_default_customer': 1,'res_partner_search_mode': 'customer', 'default_is_company': True}</field>
+            <field name="help" type="html">
+              <p class="o_view_nocontent_smiling_face">
+                Create a new customer in your address book
+              </p><p>
+                Odoo helps you easily track all activities related to a customer.
+              </p>
+            </field>
+        </record>
+
+        <record id="res_partner_action_supplier" model="ir.actions.act_window">
+            <field name="name">Suppliers</field>
+            <field name="type">ir.actions.act_window</field>
+            <field name="res_model">res.partner</field>
+            <field name="view_mode">kanban,tree,form</field>
+            <field name="context">{'search_default_supplier': 1,'res_partner_search_mode': 'supplier', 'default_is_company': True}</field>
+            <field name="help" type="html">
+              <p class="o_view_nocontent_smiling_face">
+                Create a new supplier in your address book
+              </p><p>
+                Odoo helps you easily track all activities related to a supplier.
+              </p>
+            </field>
+        </record>
     </data>
 </odoo>
diff --git a/addons/purchase/models/res_partner.py b/addons/purchase/models/res_partner.py
index 063c7143401..9e9f391c95a 100644
--- a/addons/purchase/models/res_partner.py
+++ b/addons/purchase/models/res_partner.py
@@ -59,11 +59,3 @@ class res_partner(models.Model):
     supplier_invoice_count = fields.Integer(compute='_compute_supplier_invoice_count', string='# Vendor Bills')
     purchase_warn = fields.Selection(WARNING_MESSAGE, 'Purchase Order', help=WARNING_HELP, default="no-message")
     purchase_warn_msg = fields.Text('Message for Purchase Order')
-
-    def _get_name_search_join_clause(self):
-        res = super()._get_name_search_join_clause()
-        partner_search_mode = self.env.context.get('res_partner_search_mode')
-        if partner_search_mode == 'supplier':
-            join_clause = "LEFT JOIN {table} ON res_partner.id = {table}.partner_id".format(table=self.env['purchase.order']._table)
-            return '%s %s' % (res, join_clause) if res else join_clause
-        return res
diff --git a/addons/purchase/views/purchase_views.xml b/addons/purchase/views/purchase_views.xml
index ce1057ed9f2..427198204f7 100644
--- a/addons/purchase/views/purchase_views.xml
+++ b/addons/purchase/views/purchase_views.xml
@@ -14,7 +14,7 @@
         <!--Supplier menu-->
         <menuitem id="menu_procurement_management_supplier_name" name="Vendors"
             parent="menu_procurement_management"
-            action="base.action_partner_supplier_form" sequence="15"/>
+            action="account.res_partner_action_supplier" sequence="15"/>
 
         <menuitem id="menu_purchase_config" name="Configuration" parent="menu_purchase_root" sequence="100" groups="group_purchase_manager"/>
 
diff --git a/addons/sale/models/res_partner.py b/addons/sale/models/res_partner.py
index ebebf045ae4..c14e091d9ae 100644
--- a/addons/sale/models/res_partner.py
+++ b/addons/sale/models/res_partner.py
@@ -43,11 +43,3 @@ class ResPartner(models.Model):
             ('state', 'in', ['sent', 'sale', 'done'])
         ], limit=1)
         return can_edit_vat and not bool(has_so)
-
-    def _get_name_search_join_clause(self):
-        res = super()._get_name_search_join_clause()
-        partner_search_mode = self.env.context.get('res_partner_search_mode')
-        if partner_search_mode == 'customer':
-            join_clause = "LEFT JOIN {table} ON res_partner.id = {table}.partner_id".format(table=self.env['sale.order']._table)
-            return '%s %s' % (res, join_clause) if res else join_clause
-        return res
diff --git a/addons/sale/views/sale_views.xml b/addons/sale/views/sale_views.xml
index f77b3d532cb..6c4a5c3e2af 100644
--- a/addons/sale/views/sale_views.xml
+++ b/addons/sale/views/sale_views.xml
@@ -22,7 +22,7 @@
 
         <menuitem id="res_partner_menu"
             parent="sale_order_menu"
-            action="base.action_partner_form"
+            action="account.res_partner_action_customer"
             sequence="4" groups="sales_team.group_sale_salesman"/>
 
         <menuitem id="menu_sale_report"