diff --git a/addons/im_livechat/models/chatbot_script_step.py b/addons/im_livechat/models/chatbot_script_step.py index 91114584a95..3392f0c2091 100644 --- a/addons/im_livechat/models/chatbot_script_step.py +++ b/addons/im_livechat/models/chatbot_script_step.py @@ -329,7 +329,7 @@ class ChatbotScriptStep(models.Model): posted_message = False if discuss_channel.livechat_channel_id: - human_operator = discuss_channel.livechat_channel_id._get_random_operator( + human_operator = discuss_channel.livechat_channel_id._get_operator( lang=discuss_channel.livechat_visitor_id.lang_id.code, country_id=discuss_channel.country_id.id ) diff --git a/addons/im_livechat/models/im_livechat_channel.py b/addons/im_livechat/models/im_livechat_channel.py index c0ff04e959b..12c516e2052 100644 --- a/addons/im_livechat/models/im_livechat_channel.py +++ b/addons/im_livechat/models/im_livechat_channel.py @@ -3,6 +3,7 @@ import base64 import random import re +from operator import itemgetter from odoo import api, Command, fields, models, modules, _ @@ -139,16 +140,8 @@ class ImLivechatChannel(models.Model): if chatbot_script: if chatbot_script.id not in self.browse(self.ids).mapped('rule_ids.chatbot_script_id.id'): return False - elif previous_operator_id: - # previous_operator_id is the partner_id of the previous operator, need to convert to user - if previous_operator_id in self.available_operator_ids.mapped("partner_id").ids: - user_operator = next( - available_user - for available_user in self.available_operator_ids - if available_user.partner_id.id == previous_operator_id - ) - if not user_operator and not chatbot_script: - user_operator = self._get_random_operator(lang=lang, country_id=country_id) + else: + user_operator = self._get_operator(previous_operator_id=previous_operator_id, lang=lang, country_id=country_id) if not user_operator: # no one available return False @@ -181,37 +174,52 @@ class ImLivechatChannel(models.Model): 'name': name, } - def _get_less_active_operator(self, active_channels, operators): - """ Get the operator with the less active conversations. - :param active_channels: list of active livechats (with at least - one message within the last 30 minutes) + def _get_less_active_operator(self, operator_statuses, operators): + """ Retrieve the most available operator based on the following criteria: + - Lowest number of active chats. + - Not in a call. + - If an operator is in a call and has two or more active chats, don't + give priority over an operator with more conversations who is not in a + call. + + :param operator_statuses: list of dictionaries containing the operator's + id, the number of active chats and a boolean indicating if the + operator is in a call. The list is ordered by the number of active + chats (ascending) and whether the operator is in a call + (descending). :param operators: list of operators to choose from """ if not operators: return False - active_channels = [active_channel for active_channel in active_channels if active_channel['livechat_operator_id'] in operators.mapped('partner_id').ids] - active_operators = [active_channel['livechat_operator_id'] for active_channel in active_channels] - inactive_operators = [operator for operator in operators if operator.partner_id.id not in active_operators] + active_operator_ids = set(map(lambda s: s['livechat_operator_id'], operator_statuses)) + inactive_operators = [ + operator for operator in operators if operator.partner_id.id not in active_operator_ids + ] if inactive_operators: return random.choice(inactive_operators) - - # Get the less active operator using the active_channels first element's count (since they are sorted 'ascending') - lowest_number_of_conversations = active_channels[0]['count'] - less_active_operator = random.choice([ - active_channel['livechat_operator_id'] for active_channel in active_channels - if active_channel['count'] == lowest_number_of_conversations]) - + count, in_call = itemgetter('count', 'in_call')(operator_statuses[0]) + i = 1 + while i < len(operator_statuses) and operator_statuses[i]['in_call'] == in_call and operator_statuses[i]['count'] == count: + i += 1 + less_active_operator_id = random.choice(operator_statuses[:i])['livechat_operator_id'] # convert the selected 'partner_id' to its corresponding res.users - return next(operator for operator in operators if operator.partner_id.id == less_active_operator) + return next(operator for operator in operators if operator.partner_id.id == less_active_operator_id) - def _get_random_operator(self, lang=None, country_id=None): - """ Return a random operator from the available users of the channel that have the lowest number of active livechats. - A livechat is considered 'active' if it has at least one message within the 30 minutes. - This method will try to match the given lang and country_id. + def _get_operator(self, previous_operator_id=None, lang=None, country_id=None): + """ Return an operator for a livechat. Try to return the previous + operator if available. If not, one of the most available operators be + returned. - (Some annoying conversions have to be made on the fly because this model holds 'res.users' as available operators - and the discuss_channel model stores the partner_id of the randomly selected operator) + A livechat is considered 'active' if it has at least one message within + the 30 minutes. This method will try to match the given lang and + country_id. + (Some annoying conversions have to be made on the fly because this model + holds 'res.users' as available operators and the discuss_channel model + stores the partner_id of the randomly selected operator) + + :param previous_operator_id: id of the previous operator with whom the + visitor was chatting. :param lang: code of the preferred lang of the visitor. :param country_id: id of the country of the visitor. :return : user @@ -219,30 +227,50 @@ class ImLivechatChannel(models.Model): """ if not self.available_operator_ids: return False - - self.env.cr.execute("""SELECT COUNT(DISTINCT c.id), c.livechat_operator_id + self.env.cr.execute(""" + SELECT COUNT(DISTINCT c.id), COUNT(DISTINCT rtc.id) > 0 as in_call, c.livechat_operator_id FROM discuss_channel c LEFT OUTER JOIN mail_message m ON c.id = m.res_id AND m.model = 'discuss.channel' + LEFT OUTER JOIN discuss_channel_member member ON member.partner_id = c.livechat_operator_id + LEFT OUTER JOIN discuss_channel_rtc_session rtc ON rtc.channel_member_id = member.id WHERE c.channel_type = 'livechat' + AND ( + c.livechat_active IS TRUE + OR m.create_date > ((now() at time zone 'UTC') - interval '30 minutes') + ) AND c.livechat_operator_id in %s - AND m.create_date > ((now() at time zone 'UTC') - interval '30 minutes') GROUP BY c.livechat_operator_id - ORDER BY COUNT(DISTINCT c.id) asc""", (tuple(self.available_operator_ids.mapped('partner_id').ids),)) - active_channels = self.env.cr.dictfetchall() + ORDER BY COUNT(DISTINCT c.id) < 2 OR COUNT(DISTINCT rtc.id) = 0 DESC, COUNT(DISTINCT c.id) ASC, COUNT(DISTINCT rtc.id) = 0 DESC""", + (tuple(self.available_operator_ids.partner_id.ids),) + ) + operator_statuses = self.env.cr.dictfetchall() operator = None + # Try to match the previous operator + if previous_operator_id in self.available_operator_ids.partner_id.ids: + previous_operator_status = next( + (status for status in operator_statuses if status['livechat_operator_id'] == previous_operator_id), + None + ) + if not previous_operator_status or previous_operator_status['count'] < 2 or not previous_operator_status['in_call']: + previous_operator_user = next( + available_user + for available_user in self.available_operator_ids + if available_user.partner_id.id == previous_operator_id + ) + return previous_operator_user # Try to match an operator with the same lang as the visitor if lang: same_lang_operator_ids = self.available_operator_ids.filtered(lambda operator: operator.partner_id.lang == lang) if same_lang_operator_ids: - operator = self._get_less_active_operator(active_channels, same_lang_operator_ids) + operator = self._get_less_active_operator(operator_statuses, same_lang_operator_ids) # Try to match an operator with the same country as the visitor if country_id and not operator: same_country_operator_ids = self.available_operator_ids.filtered(lambda operator: operator.partner_id.country_id.id == country_id) if same_country_operator_ids: - operator = self._get_less_active_operator(active_channels, same_country_operator_ids) + operator = self._get_less_active_operator(operator_statuses, same_country_operator_ids) # Try to get a random operator, regardless of the lang or the country if not operator: - operator = self._get_less_active_operator(active_channels, self.available_operator_ids) + operator = self._get_less_active_operator(operator_statuses, self.available_operator_ids) return operator def _get_channel_infos(self): diff --git a/addons/im_livechat/static/tests/helpers/mock_server/models/im_livechat_channel.js b/addons/im_livechat/static/tests/helpers/mock_server/models/im_livechat_channel.js index b534d30a3ef..a53d4b92730 100644 --- a/addons/im_livechat/static/tests/helpers/mock_server/models/im_livechat_channel.js +++ b/addons/im_livechat/static/tests/helpers/mock_server/models/im_livechat_channel.js @@ -29,15 +29,7 @@ patch(MockServer.prototype, { previous_operator_id, country_id ) { - let operator; - if (previous_operator_id) { - const availableOperators = this._mockImLivechatChannel__computeAvailableOperatorIds(id); - operator = availableOperators.find( - (operator) => operator.partner_id === previous_operator_id - ); - } else { - operator = this._mockImLivechatChannel_getRandomOperator(id); - } + const operator = this._mockImLivechatChannel_getOperator(id, previous_operator_id); if (!operator) { return false; } @@ -69,15 +61,19 @@ patch(MockServer.prototype, { }; }, /** - * Simulates `_get_random_operator` on `im_livechat.channel`. - * Simplified mock implementation: returns the first available operator. + * Simulates `_get_operator` on `im_livechat.channel`. Simplified mock + * implementation: returns the previous operator if he is still available + * or the first available operator. * * @private * @param {integer} id * @returns {Object} */ - _mockImLivechatChannel_getRandomOperator(id) { + _mockImLivechatChannel_getOperator(id, previous_operator_id) { const availableUsers = this._mockImLivechatChannel__computeAvailableOperatorIds(id); - return availableUsers[0]; + return ( + availableUsers.find((operator) => operator.partner_id === previous_operator_id) ?? + availableUsers[0] + ); }, }); diff --git a/addons/im_livechat/tests/__init__.py b/addons/im_livechat/tests/__init__.py index b2509c74792..fd5663558d5 100644 --- a/addons/im_livechat/tests/__init__.py +++ b/addons/im_livechat/tests/__init__.py @@ -6,7 +6,7 @@ from . import test_chatbot_form_ui from . import test_chatbot_internals from . import test_digest from . import test_get_discuss_channel -from . import test_get_random_operator +from . import test_get_operator from . import test_im_livechat_report from . import test_im_livechat_support_page from . import test_js diff --git a/addons/im_livechat/tests/test_get_operator.py b/addons/im_livechat/tests/test_get_operator.py new file mode 100644 index 00000000000..26da74003b3 --- /dev/null +++ b/addons/im_livechat/tests/test_get_operator.py @@ -0,0 +1,209 @@ +import odoo +from odoo import Command +from odoo.tests import HttpCase +from odoo.tests.common import new_test_user + +from unittest.mock import patch + + +@odoo.tests.tagged("-at_install", "post_install") +class TestGetOperator(HttpCase): + def _create_operator(self, lang_code=None, country_code=None): + operator = new_test_user(self.env, login=f"operator_{lang_code or country_code}_{self.operator_id}") + operator.partner_id = self.env["res.partner"].create( + { + "name": f"Operator {lang_code or country_code}", + "lang": lang_code, + "country_id": self.env["res.country"].search([("code", "=", country_code)]).id + if country_code + else None, + } + ) + self.env["bus.presence"].create({"user_id": operator.id, "status": "online"}) # Simulate online status + self.operator_id += 1 + return operator + + def _create_chat(self, livechat, operator, in_call=False): + channel = self.env["discuss.channel"].create( + { + "name": "Visitor 1", + "channel_type": "livechat", + "livechat_active": True, + "livechat_channel_id": livechat.id, + "livechat_operator_id": operator.partner_id.id, + "channel_member_ids": [Command.create({"partner_id": operator.partner_id.id})], + } + ) + if in_call: + self.env["discuss.channel.rtc.session"].create( + { + "channel_id": channel.id, + "channel_member_id": self.env["discuss.channel.member"] + .search([["partner_id", "=", operator.partner_id.id], ["channel_id", "=", channel.id]]) + .id, + } + ) + return channel + + def setUp(self): + super().setUp() + self.operator_id = 0 + self.env["res.lang"].with_context(active_test=False).search( + [("code", "in", ["fr_FR", "es_ES", "de_DE", "en_US"])] + ).write({"active": True}) + random_choice_patch = patch("random.choice", lambda arr: arr[0]) + self.startPatcher(random_choice_patch) + + def test_get_by_lang(self): + fr_operator = self._create_operator("fr_FR") + en_operator = self._create_operator("en_US") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [fr_operator.id, en_operator.id], + } + ) + self.assertEqual(fr_operator, livechat_channel._get_operator(lang="fr_FR")) + self.assertEqual(en_operator, livechat_channel._get_operator(lang="en_US")) + + def test_get_by_lang_no_operator_matching_lang(self): + fr_operator = self._create_operator("fr_FR") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [fr_operator.id], + } + ) + self.assertEqual(fr_operator, livechat_channel._get_operator(lang="en_US")) + + def test_get_by_country(self): + fr_operator = self._create_operator(country_code="FR") + en_operator = self._create_operator(country_code="US") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [fr_operator.id, en_operator.id], + } + ) + self.assertEqual( + fr_operator, + livechat_channel._get_operator(country_id=self.env["res.country"].search([("code", "=", "FR")]).id), + ) + self.assertEqual( + en_operator, + livechat_channel._get_operator(country_id=self.env["res.country"].search([("code", "=", "US")]).id), + ) + + def test_get_by_country_no_operator_matching_country(self): + fr_operator = self._create_operator(country_code="FR") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [fr_operator.id], + } + ) + self.assertEqual( + fr_operator, + livechat_channel._get_operator(country_id=self.env["res.country"].search([("code", "=", "US")]).id), + ) + + def test_get_by_lang_and_country_prioritize_lang(self): + fr_operator = self._create_operator("fr_FR", "FR") + en_operator = self._create_operator("en_US", "US") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [fr_operator.id, en_operator.id], + } + ) + self.assertEqual( + fr_operator, + livechat_channel._get_operator( + lang="fr_FR", country_id=self.env["res.country"].search([("code", "=", "US")]).id + ), + ) + self.assertEqual( + en_operator, + livechat_channel._get_operator( + lang="en_US", country_id=self.env["res.country"].search([("code", "=", "FR")]).id + ), + ) + + def test_operator_in_call_no_more_than_two_chats(self): + first_operator = self._create_operator("fr_FR", "FR") + second_operator = self._create_operator("fr_FR", "FR") + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [first_operator.id, second_operator.id], + } + ) + self._create_chat(livechat_channel, first_operator) + self._create_chat(livechat_channel, first_operator) + # Previous operator is not in a call so it should be available, even if + # he already has two ongoing chats. + self.assertEqual( + first_operator, livechat_channel._get_operator(previous_operator_id=first_operator.partner_id.id) + ) + self._create_chat(livechat_channel, first_operator, in_call=True) + # Previous operator is in a call so it should not be available anymore. + self.assertEqual( + second_operator, livechat_channel._get_operator(previous_operator_id=first_operator.partner_id.id) + ) + + def test_priority_by_number_of_chat(self): + first_operator = self._create_operator() + second_operator = self._create_operator() + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [first_operator.id, second_operator.id], + } + ) + self._create_chat(livechat_channel, first_operator) + self._create_chat(livechat_channel, second_operator) + self._create_chat(livechat_channel, second_operator) + self.assertEqual(first_operator, livechat_channel._get_operator()) + + def test_in_call_operator_not_prioritized(self): + first_operator = self._create_operator() + second_operator = self._create_operator() + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [first_operator.id, second_operator.id], + } + ) + self._create_chat(livechat_channel, first_operator, in_call=True) + self._create_chat(livechat_channel, second_operator) + self.assertEqual(second_operator, livechat_channel._get_operator()) + + def test_priority_by_number_of_chat_with_call_limit_not_exceeded(self): + first_operator = self._create_operator() + second_operator = self._create_operator() + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [first_operator.id, second_operator.id], + } + ) + self._create_chat(livechat_channel, first_operator, in_call=True) + self._create_chat(livechat_channel, second_operator) + self._create_chat(livechat_channel, second_operator) + self.assertEqual(first_operator, livechat_channel._get_operator()) + + def test_priority_by_number_of_chat_all_operators_exceed_limit(self): + first_operator = self._create_operator() + second_operator = self._create_operator() + livechat_channel = self.env["im_livechat.channel"].create( + { + "name": "Livechat Channel", + "user_ids": [first_operator.id, second_operator.id], + } + ) + self._create_chat(livechat_channel, first_operator, in_call=True) + self._create_chat(livechat_channel, first_operator) + self._create_chat(livechat_channel, second_operator, in_call=True) + self._create_chat(livechat_channel, second_operator) + self._create_chat(livechat_channel, second_operator) + self.assertEqual(first_operator, livechat_channel._get_operator()) diff --git a/addons/im_livechat/tests/test_get_random_operator.py b/addons/im_livechat/tests/test_get_random_operator.py deleted file mode 100644 index ef876c34268..00000000000 --- a/addons/im_livechat/tests/test_get_random_operator.py +++ /dev/null @@ -1,85 +0,0 @@ -import odoo -from odoo.tests import HttpCase -from odoo.tests.common import new_test_user - - -@odoo.tests.tagged('-at_install', 'post_install') -class TestGetRandomOperator(HttpCase): - def _create_operator(self, lang_code=None, country_code=None): - operator = new_test_user(self.env, login=f'operator_{lang_code or country_code}_{self.operator_id}') - operator.partner_id = self.env['res.partner'].create({ - 'name': f'Operator {lang_code or country_code}', - 'lang': lang_code, - 'country_id': self.env['res.country'].search([('code', '=', country_code)]).id if country_code else None, - }) - self.env['bus.presence'].create({'user_id': operator.id, 'status': 'online'}) # Simulate online status - self.operator_id += 1 - return operator - - def setUp(self): - super().setUp() - self.operator_id = 0 - self.env['res.lang'].with_context(active_test=False).search([ - ('code', 'in', ['fr_FR', 'es_ES', 'de_DE', 'en_US']) - ]).write({'active': True}) - - def test_get_by_lang(self): - fr_operator = self._create_operator('fr_FR') - en_operator = self._create_operator('en_US') - livechat_channel = self.env['im_livechat.channel'].create({ - 'name': 'Livechat Channel', - 'user_ids': [fr_operator.id, en_operator.id], - }) - self.assertEqual(fr_operator, livechat_channel._get_random_operator(lang='fr_FR')) - self.assertEqual(en_operator, livechat_channel._get_random_operator(lang='en_US')) - - def test_get_by_lang_no_operator_matching_lang(self): - fr_operator = self._create_operator('fr_FR') - livechat_channel = self.env['im_livechat.channel'].create({ - 'name': 'Livechat Channel', - 'user_ids': [fr_operator.id], - }) - self.assertEqual(fr_operator, livechat_channel._get_random_operator(lang='en_US')) - - def test_get_by_country(self): - fr_operator = self._create_operator(country_code='FR') - en_operator = self._create_operator(country_code='US') - livechat_channel = self.env['im_livechat.channel'].create({ - 'name': 'Livechat Channel', - 'user_ids': [fr_operator.id, en_operator.id], - }) - self.assertEqual( - fr_operator, - livechat_channel._get_random_operator(country_id=self.env['res.country'].search([('code', '=', 'FR')]).id) - ) - self.assertEqual( - en_operator, - livechat_channel._get_random_operator(country_id=self.env['res.country'].search([('code', '=', 'US')]).id) - ) - - def test_get_by_country_no_operator_matching_country(self): - fr_operator = self._create_operator(country_code='FR') - livechat_channel = self.env['im_livechat.channel'].create({ - 'name': 'Livechat Channel', - 'user_ids': [fr_operator.id], - }) - self.assertEqual( - fr_operator, - livechat_channel._get_random_operator(country_id=self.env['res.country'].search([('code', '=', 'US')]).id) - ) - - def test_get_by_lang_and_country_prioritize_lang(self): - fr_operator = self._create_operator('fr_FR', 'FR') - en_operator = self._create_operator('en_US', 'US') - livechat_channel = self.env['im_livechat.channel'].create({ - 'name': 'Livechat Channel', - 'user_ids': [fr_operator.id, en_operator.id], - }) - self.assertEqual( - fr_operator, - livechat_channel._get_random_operator(lang='fr_FR', country_id=self.env['res.country'].search([('code', '=', 'US')]).id) - ) - self.assertEqual( - en_operator, - livechat_channel._get_random_operator(lang='en_US', country_id=self.env['res.country'].search([('code', '=', 'FR')]).id) - )