[HACK] crm: allow ordering on my activities deadline

RATIONALE

Activities are a way to organize our daily pipe. Notably in a sales pipeline
salesmen may have multiple leads with activities to perform in the next few
days. We would like to be able to have a list view displaying a list of leads
with "my" deadline, and to order leads based on this field.

PURPOSE

As activities are linked to records through a one2many and as my activities
are only a subset of those activities, it is impossible to store a field
on lead model. Indeed it would depend on the current user which makes storing
it impossible.

However ordering on a column in list view asks for a stored field. As it is
not possible, this commit proposes to hijack the web client and search of lead
model in order to allow it.

SPECIFICATIONS

Introduce a activity_date_deadline_my field, computed and searchable that
behaves like activity_date_deadline. It is filtered on current user activities
instead of being global to the record activities. "My" deadline is the earliest
deadline of my activities, even in the past.

Ordering through web client calls search_read with an order parameter set.
Search_read then calls search. In this commit we therefore override search
to intercept a search without count with an order on activity_date_deadline_my.
In that case we do the search in two steps.

First step: fill with deadline-based results

  * Perform a read_group on my activities to get a mapping lead_id / deadline
    Remember date_deadline is required, we always have a value for it. Only
    the earliest deadline per lead is kept.
  * Search leads linked to those activities that also match the asked domain
    and order from the original search request.
  * Results of that search will be at the top of returned results. Use limit
    None because we have to search all leads linked to activities as ordering
    on deadline is done in post processing.
  * Reorder them according to deadline asc or desc depending on original
    search ordering. Finally take only a subset of those leads to fill with
    results matching asked offset / limit.

Second step: fill with other results. If first step does not gives results
enough to match offset and limit parameters we fill with a search on other
leads. We keep the asked domain and ordering while filtering out already
scanned leads to keep a coherent results.

All other search and search_read are left untouched by this commit to avoid
side effects. Search_count is not affected by this commit.

As web client order availability is simply based on field being stored we
have to allow hijacking its value. A new option 'allow_order' is added that
display the order icon in list view column headers. It allows calling
search_read with an order based on the column but nothing more. Using it
on a not-stored field will give a warning log and ORM ignores it by default.

LIMITATIONS

At least following limitations apply

  * search on "my activities" gives a list of ids that is used for searching
    and ordering. It means that having too much activities on a given user can
    result in poor performances.
    -> we consider that activities assigned to a given user should be kept
    under control. Leads with activities for a given user should be a small
    subset of leads without activities.
  * ordering on activity_date_deadline_my always put those records on the top
    (deadline asc or desc) whatever the actual ordering of order items given
    to search. This is done to avoid complex implementation where deadline
    would be used to order records within groups ordered by another field.
    -> for example: order = 'priority DESC, activity_date_deadline_my ASC'
    actually behaves like 'activity_date_deadline_my ASC, priority DESC'.

WARNING

This implementation is a hack. It has been done by professionals. Do not try
to do it at home.

More seriously this is hackish and should be avoided. IN our case we consider
that the use of activities in sales pipeline is mainly driven by "my activities
deadline", which is not necessarily the case with longer processes like tasks.

Moreover user tests tend to show that this feature could be interesting. It
will improve quality of Odoo CRM and its adoption by improving its daily
use. A framework solution has to be found. Until this is done we keep this
hack and hope it is not duplicated in other applications.

LINKS

Task ID 2276011
Prepares Task ID 2243124 (My activities in CRM)
PR #52232

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/crm/models/crm_lead.py

@@ -7,6 +7,7 @@ from datetime import date, datetime
 from psycopg2 import sql
 
 from odoo import api, fields, models, tools, SUPERUSER_ID
+from odoo.osv import expression
 from odoo.tools.translate import _
 from odoo.tools import email_re, email_split
 from odoo.exceptions import UserError, AccessError
@@ -97,6 +98,10 @@ class Lead(models.Model):
         ('red', 'Next activity late'),
         ('green', 'Next activity is planned')], string='Kanban State',
         compute='_compute_kanban_state')
+    activity_date_deadline_my = fields.Date(
+        'My Activities Deadline', compute='_compute_activity_date_deadline_my',
+        search='_search_activity_date_deadline_my', compute_sudo=False,
+        readonly=True, store=False, groups="base.group_user")
     tag_ids = fields.Many2many(
         'crm.tag', 'crm_tag_rel', 'lead_id', 'tag_id', string='Tags',
         help="Classify and analyze your lead/opportunity categories like: Training, Service")
@@ -176,6 +181,7 @@ class Lead(models.Model):
         ('check_probability', 'check(probability >= 0 and probability <= 100)', 'The probability of closing the deal should be between 0% and 100%!')
     ]
 
+    @api.depends('activity_date_deadline')
     def _compute_kanban_state(self):
         today = date.today()
         for lead in self:
@@ -188,6 +194,25 @@ class Lead(models.Model):
                     kanban_state = 'red'
             lead.kanban_state = kanban_state
 
+    @api.depends('activity_ids.date_deadline')
+    @api.depends_context('uid')
+    def _compute_activity_date_deadline_my(self):
+        todo_activities = []
+        if self.ids:
+            todo_activities = self.env['mail.activity'].search([
+                ('user_id', '=', self._uid),
+                ('res_model', '=', self._name),
+                ('res_id', 'in', self.ids)
+            ], order='date_deadline ASC')
+        for record in self:
+            record.activity_date_deadline_my = next(
+                (activity.date_deadline for activity in todo_activities if activity.res_id == record.id),
+                False
+            )
+
+    def _search_activity_date_deadline_my(self, operator, operand):
+        return ['&', ('activity_ids.user_id', '=', self._uid), ('activity_ids.date_deadline', operator, operand)]
+
     @api.depends('user_id', 'type')
     def _compute_team_id(self):
         """ When changing the user, also set a team_id or restrict team id
@@ -455,6 +480,86 @@ class Lead(models.Model):
 
         return write_result
 
+    @api.model
+    def search(self, args, offset=0, limit=None, order=None, count=False):
+        """ Override to support ordering on activity_date_deadline_my.
+
+        Ordering through web client calls search_read with an order parameter set.
+        Search_read then calls search. In this override we therefore override search
+        to intercept a search without count with an order on activity_date_deadline_my.
+        In that case we do the search in two steps.
+
+        First step: fill with deadline-based results
+
+          * Perform a read_group on my activities to get a mapping lead_id / deadline
+            Remember date_deadline is required, we always have a value for it. Only
+            the earliest deadline per lead is kept.
+          * Search leads linked to those activities that also match the asked domain
+            and order from the original search request.
+          * Results of that search will be at the top of returned results. Use limit
+            None because we have to search all leads linked to activities as ordering
+            on deadline is done in post processing.
+          * Reorder them according to deadline asc or desc depending on original
+            search ordering. Finally take only a subset of those leads to fill with
+            results matching asked offset / limit.
+
+        Second step: fill with other results. If first step does not gives results
+        enough to match offset and limit parameters we fill with a search on other
+        leads. We keep the asked domain and ordering while filtering out already
+        scanned leads to keep a coherent results.
+
+        All other search and search_read are left untouched by this override to avoid
+        side effects. Search_count is not affected by this override.
+        """
+        if count or not order or 'activity_date_deadline_my' not in order:
+            return super(Lead, self).search(args, offset=offset, limit=limit, order=order, count=count)
+        order_items = [order_item.strip().lower() for order_item in (order or self._order).split(',')]
+
+        # Perform a read_group on my activities to get a mapping lead_id / deadline
+        # Remember date_deadline is required, we always have a value for it. Only
+        # the earliest deadline per lead is kept.
+        activity_asc = any('activity_date_deadline_my asc' in item for item in order_items)
+        my_lead_activities = self.env['mail.activity'].read_group(
+            [('res_model', '=', self._name), ('user_id', '=', self.env.uid)],
+            ['res_id', 'date_deadline:min'],
+            ['res_id'],
+            orderby='date_deadline ASC'
+        )
+        my_lead_mapping = dict((item['res_id'], item['date_deadline']) for item in my_lead_activities)
+        my_lead_ids = list(my_lead_mapping.keys())
+        my_lead_domain = expression.AND([[('id', 'in', my_lead_ids)], args])
+        my_lead_order = ', '.join(item for item in order_items if 'activity_date_deadline_my' not in item)
+
+        # Search leads linked to those activities and order them. See docstring
+        # of this method for more details.
+        search_res = super(Lead, self).search(my_lead_domain, offset=0, limit=None, order=my_lead_order, count=count)
+        my_lead_ids_ordered = sorted(search_res.ids, key=lambda lead_id: my_lead_mapping[lead_id], reverse=not activity_asc)
+        # keep only requested window (offset + limit, or offset+)
+        my_lead_ids_keep = my_lead_ids_ordered[offset:(offset + limit)] if limit else my_lead_ids_ordered[offset:]
+        # keep list of already skipped lead ids to exclude them from future search
+        my_lead_ids_skip = my_lead_ids_ordered[:(offset + limit)] if limit else my_lead_ids_ordered
+
+        # do not go further if limit is achieved
+        if limit and len(my_lead_ids_keep) >= limit:
+            return self.browse(my_lead_ids_keep)
+
+        # Fill with remaining leads. If a limit is given, simply remove count of
+        # already fetched. Otherwise keep none. If an offset is set we have to
+        # reduce it by already fetch results hereabove. Order is updated to exclude
+        # activity_date_deadline_my when calling super() .
+        lead_limit = (limit - len(my_lead_ids_keep)) if limit else None
+        if offset:
+            lead_offset = max((offset - len(search_res), 0))
+        else:
+            lead_offset = 0
+        lead_order = ', '.join(item for item in order_items if 'activity_date_deadline_my' not in item)
+
+        other_lead_res = super(Lead, self).search(
+            expression.AND([[('id', 'not in', my_lead_ids_skip)], args]),
+            offset=lead_offset, limit=lead_limit, order=lead_order, count=count
+        )
+        return self.browse(my_lead_ids_keep) + other_lead_res
+
     def _update_probability(self):
         lead_probabilities = self.sudo()._pls_get_naive_bayes_probabilities()
         for lead in self:

addons/crm/tests/common.py

@@ -150,6 +150,24 @@ class TestCrmCommon(TestSalesCommon, MailCase):
             'zip': '97648',
         })
 
+    def _create_leads_batch(self, lead_type='lead', count=10):
+        """ Helper tool method creating a batch of leads, useful when dealing
+        with batch processes. Please update me.
+
+        :param string type: 'lead', 'opportunity', 'mixed' (lead then opp),
+          None (depends on configuration);
+        """
+        types = ['lead', 'opportunity']
+        partners = [self.contact_1.id, self.contact_2.id, False]
+        leads_data = [{
+            'name': 'TestLead_%02d' % (x),
+            'type': lead_type if lead_type else types[x % 2],
+            'partner_id': partners[x % 3],
+            'priority': '%s' % (x % 3),
+        } for x in range(count)]
+
+        return self.env['crm.lead'].create(leads_data)
+
     def _create_duplicates(self, lead, create_opp=True):
         """ Helper tool method creating, based on a given lead
 

addons/crm/tests/test_crm_activity.py

@@ -1,7 +1,10 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from datetime import date, timedelta
+
 from odoo.addons.crm.tests.common import TestCrmCommon
+from odoo.tests.common import users
 
 
 class TestCrmMailActivity(TestCrmCommon):
@@ -22,6 +25,89 @@ class TestCrmMailActivity(TestCrmCommon):
             'summary': 'ACT 2 : I want to show you my ERP !',
             'res_model_id': cls.env['ir.model']._get('crm.lead').id,
         })
+        for activity_type in cls.activity_type_1 + cls.activity_type_2:
+            cls.env['ir.model.data'].create({
+                'name': activity_type.name.lower().replace(' ', '_'),
+                'module': 'crm',
+                'model': activity_type._name,
+                'res_id': activity_type.id,
+            })
+
+    @users('user_sales_leads')
+    def test_crm_activity_ordering(self):
+        """ Test ordering on "my activities", linked to a hack introduced for a b2b.
+        Purpose is to be able to order on "my activities", which is a filtered o2m.
+        In this test we will check search, limit, order and offset linked to the
+        override of search for this non stored computed field. """
+        # Initialize some batch data
+        default_order = self.env['crm.lead']._order
+        self.assertEqual(default_order, "priority desc, id desc")  # force updating this test is order changes
+        test_leads = self._create_leads_batch(count=10).sorted('id')
+
+        # assert initial data, ensure we did not break base behavior
+        for lead in test_leads:
+            self.assertFalse(lead.activity_date_deadline_my)
+        search_res = self.env['crm.lead'].search([('id', 'in', test_leads.ids)], limit=5, offset=0, order='id ASC')
+        self.assertEqual(search_res.ids, test_leads[:5].ids)
+        search_res = self.env['crm.lead'].search([('id', 'in', test_leads.ids)], limit=5, offset=5, order='id ASC')
+        self.assertEqual(search_res.ids, test_leads[5:10].ids)
+
+        # Let's schedule some activities for "myself" and "my bro"and check those are correctly computed
+        # LEAD NUMBER     DEADLINE (MY) PRIORITY   LATE DEADLINE (MY)
+        # 0               +2D           0          +2D
+        # 1               -1D           1          +2D
+        # 2               -2D           2          +2D
+        # 3               -1D           0          -1D
+        # 4               -2D           1          -2D
+        # 5               +2D           2          +2D
+        # 6+              /             0/1/2/0    /
+        today = date.today()
+        deadline_in2d, deadline_in1d = today + timedelta(days=2), today + timedelta(days=1)
+        deadline_was2d, deadline_was1d = today + timedelta(days=-2), today + timedelta(days=-1)
+        deadlines_my = [deadline_in2d, deadline_was1d, deadline_was2d, deadline_was1d, deadline_was2d,
+                        deadline_in2d, False, False, False, False]
+        deadlines_gl = [deadline_in1d, deadline_was1d, deadline_was2d, deadline_was1d, deadline_was2d,
+                        deadline_in2d, False, False, False, False]
+
+        test_leads[0:4].activity_schedule(act_type_xmlid='crm.call_for_demo', user_id=self.user_sales_manager.id, date_deadline=deadline_in1d)
+        test_leads[0:3].activity_schedule(act_type_xmlid='crm.initial_contact', date_deadline=deadline_in2d)
+        test_leads[5].activity_schedule(act_type_xmlid='crm.initial_contact', date_deadline=deadline_in2d)
+        (test_leads[1] | test_leads[3]).activity_schedule(act_type_xmlid='crm.initial_contact', date_deadline=deadline_was1d)
+        (test_leads[2] | test_leads[4]).activity_schedule(act_type_xmlid='crm.call_for_demo', date_deadline=deadline_was2d)
+        test_leads.invalidate_cache()
+
+        expected_ids_asc = [2, 4, 1, 3, 5, 0, 8, 7, 9, 6]
+        expected_leads_asc = self.env['crm.lead'].browse([test_leads[lid].id for lid in expected_ids_asc])
+        expected_ids_desc = [5, 0, 1, 3, 2, 4, 8, 7, 9, 6]
+        expected_leads_desc = self.env['crm.lead'].browse([test_leads[lid].id for lid in expected_ids_desc])
+
+        for idx, lead in enumerate(test_leads):
+            self.assertEqual(lead.activity_date_deadline_my, deadlines_my[idx])
+            self.assertEqual(lead.activity_date_deadline, deadlines_gl[idx], 'Fail at %s' % idx)
+
+        # Let's go for a first batch of search
+        _order = 'activity_date_deadline_my ASC, %s' % default_order
+        _domain = [('id', 'in', test_leads.ids)]
+
+        search_res = self.env['crm.lead'].search(_domain, limit=None, offset=0, order=_order)
+        self.assertEqual(expected_leads_asc.ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=4, offset=0, order=_order)
+        self.assertEqual(expected_leads_asc[:4].ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=4, offset=3, order=_order)
+        self.assertEqual(expected_leads_asc[3:7].ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=None, offset=3, order=_order)
+        self.assertEqual(expected_leads_asc[3:].ids, search_res.ids)
+
+        _order = 'activity_date_deadline_my DESC, %s' % default_order
+        search_res = self.env['crm.lead'].search(_domain, limit=None, offset=0, order=_order)
+        self.assertEqual(expected_leads_desc.ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=4, offset=0, order=_order)
+        self.assertEqual(expected_leads_desc[:4].ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=4, offset=3, order=_order)
+        self.assertEqual(expected_leads_desc[3:7].ids, search_res.ids)
+        search_res = self.env['crm.lead'].search(_domain, limit=None, offset=3, order=_order)
+        self.assertEqual(expected_leads_desc[3:].ids, search_res.ids)
+
 
     def test_crm_activity_recipients(self):
         """ This test case checks

addons/crm/views/crm_lead_views.xml

@@ -584,7 +584,6 @@
             <field name="model">crm.lead</field>
             <field name="arch" type="xml">
                 <tree string="Opportunities" decoration-muted="probability == 100" decoration-danger="activity_date_deadline and (activity_date_deadline &lt; current_date)">
-                    <field name="date_deadline" invisible="1"/>
                     <field name="create_date" optional="hide"/>
                     <field name="name" string="Opportunity"/>
                     <field name="partner_id" string="Customer" optional="show"/>
@@ -594,6 +593,7 @@
                     <field name="state_id" optional="hide"/>
                     <field name="country_id" optional="show"/>
                     <field name="activity_date_deadline" optional="hide"/>
+                    <field name="activity_date_deadline_my" optional="hide" options="{'allow_order': '1'}"/>
                     <field name="activity_summary" optional="hide" readonly="1"/>
                     <field name="stage_id" required="1" optional="show"/>
                     <field name="probability" avg="Avg. of Probability" optional="hide"/>

addons/mail/models/mail_activity.py

@@ -670,7 +670,7 @@ class MailActivityMixin(models.AbstractModel):
     activity_date_deadline = fields.Date(
         'Next Activity Deadline',
         compute='_compute_activity_date_deadline', search='_search_activity_date_deadline',
-        readonly=True, store=False,
+        compute_sudo=False, readonly=True, store=False,
         groups="base.group_user")
     activity_summary = fields.Char(
         'Next Activity Summary',

addons/web/static/src/js/views/list/list_renderer.js

@@ -891,7 +891,7 @@ var ListRenderer = BasicRenderer.extend({
             .attr('tabindex', -1)
             .toggleClass('o-sort-down', isNodeSorted ? !order[0].asc : false)
             .toggleClass('o-sort-up', isNodeSorted ? order[0].asc : false)
-            .addClass(field.sortable && 'o_column_sortable');
+            .addClass((field.sortable || this.state.fieldsInfo.list[name].options.allow_order || false) && 'o_column_sortable');
 
         if (isNodeSorted) {
             $th.attr('aria-sort', order[0].asc ? 'ascending' : 'descending');