Refactoring match making

README.md

 # Mattermost Coffee Bot
 The purpose of this small bot is to act as a matchmaker for users that shares channel with the bot.
 
-## Run
-Currently the bot only supports one entrypoint when it will run in every channel it knows and make pairs of 2 for every channel. Also, the messages are currently hard-coded in Hungarian, and custom-tailored to KSZK's Mattermost Installation.
-
-To run this entrypoint
-```
-. ./virtualenv/bin/activate
- MATTERMOST_URL=<mattermost-url> MATTERMOST_TOKEN=<mattermost-token> ./coffee_bot/run_for_all_channels.py
-```
-
 ## Development
 To set up a local development environment, run:
 ```

coffee_bot/coffee_bot.py → coffee_bot/match_making.py

 import copy
 import random
-from typing import Generator, Iterable
+from typing import Generator, Iterable, Tuple, List
 import mattermost
 
 #pylint: disable=invalid-name
@@ -26,77 +26,58 @@ def random_nonrepeating_tuples(population: Iterable, n: int) -> Generator[tuple,
         yield tuple(buffer)
 
 
-def matching(client: mattermost.MMApi) -> None:
+# pylint: disable=too-many-arguments
+def matching_in_channel(client: mattermost.MMApi, channel_id: str,
+                        group_hail=None, left_out_message=None, too_few_message=None,
+                        minimum_members=4, group_size=2) -> None:
     """
-    Run through every channel associated with the client and pairs channel members into group chats.
+    Get members of a channel, make matches between these members and handle communication regarding these actions
     """
-    teams = get_my_teams(client)
 
-    for team in teams:
-        team_channels = client.get_channel_memberships_for_user(
-            'me', team['id'])
-        for channel in team_channels:
-            channel_id = channel['channel_id']
     members = members_in_channel_besides_me(client, channel_id)
 
-            if len(members) <= 4:
-                # Too few members in this channel to make meaningful groups
-                continue
+    if len(members) < minimum_members:
+        if too_few_message:
+            client.create_post(channel_id=channel_id, message=too_few_message)
+        return
 
-            make_match_in_channel(client, channel_id, members)
+    groups_created, left_out = make_matches_for_members(client, members, group_size)
 
+    if group_hail:
+        for group_id in groups_created:
+            client.create_post(channel_id=group_id, message=group_hail)
 
-def handle_no_pair(client: mattermost.MMApi, channel_id: str, group: Iterable[dict]) -> None:
-    """
-    Handle if someone should be left out because they can not paired up
-
-    It sends a message to the channel it runs for so others know if someone had to be left out.
-    """
+    if left_out_message and left_out is not None:
+        user_data = client.get_user(left_out)
+        client.create_post(channel_id=channel_id, message=left_out_message.format(user=user_data))
 
-    user = get_user_data(client, group[0]["user_id"])
-    client.create_post(
-        channel_id=channel_id,
-        message=f"Ebben a körben {user['last_name']} {user['first_name']} nem került sorra sajnos.",
-    )
 
-
-def create_group(client: mattermost.MMApi, group: Iterable[dict]) -> None:
+def create_group(client: mattermost.MMApi, ids: Iterable[str]) -> None:
     """
     Creating a group-message for the group
-
-    Creates a new group-message between the group-members (and the bot itself),
-    and sends a short welcome message.
     """
 
-    ids = [m['user_id'] for m in group]
     groupchat = client.create_group_channel_with(other_user_ids_list=ids)
-    client.create_post(
-        channel_id=groupchat['id'],
-        message="Most ti kerültetek sorra. Jó beszélgetést és kávézást! :coffee_parrot:",
-    )
 
+    return groupchat['id']
 
-def make_match_in_channel(client: mattermost.MMApi, channel_id: str, members: Iterable[dict],
-                          groupsize: int=2) -> None:
+
+def make_matches_for_members(client: mattermost.MMApi, members: Iterable[dict],
+                             group_size: int = 2) -> Tuple[List[str], str]:
     """
     Making matches in a channel
     """
 
-    for group in random_nonrepeating_tuples(members, groupsize):
+    groups_created = []
+    left_out = None
+
+    for group in random_nonrepeating_tuples(members, group_size):
         if len(group) == 1:
-            handle_no_pair(client, channel_id, group)
+            left_out = group[0]['user_id']
         else:
-            create_group(client, group)
+            groups_created.append(create_group(client, [u['user_id'] for u in group]))
 
-
-def get_my_teams(client):
-    """
-    Get a list of the teams the bot associated with the client has access to.
-
-    Note: This is function is *not* available in the imported mattermost API binding lib.
-    """
-    # pylint: disable=protected-access
-    return client._get('/v4/users/me/teams')
+    return groups_created, left_out
 
 
 def members_in_channel_besides_me(client, channel_id):
@@ -107,13 +88,3 @@ def members_in_channel_besides_me(client, channel_id):
     """
     # pylint: disable=protected-access
     return [m for m in client.get_channel_members(channel_id) if m['user_id'] != client._my_user_id]
-
-
-def get_user_data(client, user_id):
-    """
-    Gets user data for the given user id using the given client.
-
-    Note: This is function is *not* properly implemented by the imported mattermost API binding lib.
-    """
-    # pylint: disable=protected-access
-    return client._get(f'/v4/users/{user_id}')

coffee_bot/run_for_all_channels.py

-import mattermost
-
-from .config import MATTERMOST_TOKEN, MATTERMOST_URL
-from .coffee_bot import matching
-
-
-if __name__ == "__main__":
-    client = mattermost.MMApi(MATTERMOST_URL)
-    client.login(bearer=MATTERMOST_TOKEN)
-
-    matching(client)

tests/test_matchmaking.py

@@ -4,10 +4,10 @@ import copy
 import mock
 import mattermost
 
-from coffee_bot import coffee_bot
+from coffee_bot import match_making
 
 
-class MatchmakingCase(unittest.TestCase):
+class MatchMakingCaseBase(unittest.TestCase):
     def setUp(self):
         self.my_user_id = "my-bots-user-id"
         self.client = mock.MagicMock(mattermost.MMApi)
@@ -19,77 +19,6 @@ class MatchmakingCase(unittest.TestCase):
         self.my_channel_membership = self._create_membership_object(
             self.my_user_id)
 
-    def test_exclude_myself(self):
-        members = self._generate_n_members(self.minimum_membership_requirement)
-        self._set_channel_members(members, add_myself=True)
-
-        self.assertIn(self.my_channel_membership, self.members)
-
-        members_except_me = coffee_bot.members_in_channel_besides_me(
-            self.client, "channel-id")
-
-        self.assertNotIn(
-            self.my_channel_membership, members_except_me)
-
-        self.assertListEqual(members_except_me, members)
-
-    @mock.patch('coffee_bot.coffee_bot.handle_no_pair')
-    @mock.patch('coffee_bot.coffee_bot.create_group')
-    def test_even_members(self, create_group, handle_no_pair):
-        # pylint: disable=invalid-name
-        n = (self.minimum_membership_requirement + 1) * 2
-        members = self._generate_n_members(n)
-
-        coffee_bot.make_match_in_channel(self.client, "channel-id", members)
-
-        self.assertEqual(create_group.call_count,
-                         self.minimum_membership_requirement + 1)
-        self.assertEqual(handle_no_pair.call_count, 0)
-
-    @mock.patch('coffee_bot.coffee_bot.handle_no_pair')
-    @mock.patch('coffee_bot.coffee_bot.create_group')
-    def test_odd_members(self, create_group, handle_no_pair):
-        # pylint: disable=invalid-name
-        n = (self.minimum_membership_requirement * 2) + 1
-        members = self._generate_n_members(n)
-
-        coffee_bot.make_match_in_channel(self.client, "channel-id", members)
-
-        self.assertEqual(create_group.call_count,
-                         self.minimum_membership_requirement)
-        self.assertEqual(handle_no_pair.call_count, 1)
-
-    def test_group_creation(self):
-        members = self._generate_n_members(2)
-        self.client.create_group_channel_with.return_value = {
-            "id": "group-chat-id"}
-
-        coffee_bot.create_group(self.client, members)
-
-        self.client.create_group_channel_with.assert_called_with(
-            other_user_ids_list=['user-id-0', 'user-id-1']
-        )
-        self.client.create_post.assert_called_with(
-            channel_id="group-chat-id",
-            message="Most ti kerültetek sorra. Jó beszélgetést és kávézást! :coffee_parrot:",
-        )
-
-    def test_handling_no_pairs(self):
-        user_id = "user-id-1"
-        member = self._create_membership_object(user_id)
-        user_data = {'id': user_id, 'username': "UserId1",
-                     "first_name": "User", "last_name": "Generated"}
-
-        with mock.patch('coffee_bot.coffee_bot.get_user_data', return_value=user_data) as get_user_data:
-            coffee_bot.handle_no_pair(self.client, "channel-id", (member,))
-
-        get_user_data.assert_called_with(self.client, user_id)
-
-        self.client.create_post.assert_called_with(
-            channel_id="channel-id",
-            message="Ebben a körben Generated User nem került sorra sajnos.",
-        )
-
     def _set_channel_members(self, members, add_myself=True):
         self.members = copy.deepcopy(members)
 
@@ -119,10 +48,157 @@ class MatchmakingCase(unittest.TestCase):
             "explicit_roles": "",
         }
 
-    def _generate_n_members(self, n, **kwargs):  # pylint: disable=invalid-name
+    @classmethod
+    def _generate_n_members(cls, n, **kwargs):  # pylint: disable=invalid-name
         members = []
         for i in range(0, n):
-            members.append(self._create_membership_object(
+            members.append(cls._create_membership_object(
                 f"user-id-{i}", **kwargs))
 
         return members
+
+
+class MatchMakingCase(MatchMakingCaseBase):
+    def test_exclude_myself(self):
+        members = self._generate_n_members(self.minimum_membership_requirement)
+        self._set_channel_members(members, add_myself=True)
+
+        self.assertIn(self.my_channel_membership, self.members)
+
+        members_except_me = match_making.members_in_channel_besides_me(
+            self.client, "channel-id")
+
+        self.assertNotIn(
+            self.my_channel_membership, members_except_me)
+
+        self.assertListEqual(members_except_me, members)
+
+    def test_group_creation(self):
+        self.client.create_group_channel_with.return_value = {
+            "id": "group-chat-id"}
+
+        group_id = match_making.create_group(self.client, ["user-id-0", "user-id-1"])
+
+        self.assertEqual(group_id, "group-chat-id")
+        self.client.create_group_channel_with.assert_called_with(
+            other_user_ids_list=['user-id-0', 'user-id-1']
+        )
+
+    @mock.patch('coffee_bot.match_making.create_group')
+    @mock.patch('coffee_bot.match_making.random.shuffle')
+    def test_making_matches_for_members_even(self, _shuffle, create_group):
+        members = self._generate_n_members(4)
+        create_group.side_effect = lambda c, ids: ";".join(ids)
+
+        groups_created, left_out = match_making.make_matches_for_members(
+            client=self.client,
+            members=members,
+            group_size=2,
+        )
+
+        self.assertIsNone(left_out, "No one should be left out in this setup")
+        self.assertListEqual(["user-id-0;user-id-1", "user-id-2;user-id-3"], groups_created)
+
+    @mock.patch('coffee_bot.match_making.create_group')
+    @mock.patch('coffee_bot.match_making.random.shuffle')
+    def test_making_matches_for_members_odd(self, _shuffle, create_group):
+        members = self._generate_n_members(5)
+        create_group.side_effect = lambda c, ids: ";".join(ids)
+
+        groups_created, left_out = match_making.make_matches_for_members(
+            client=self.client,
+            members=members,
+            group_size=2,
+        )
+
+        self.assertEqual(left_out, "user-id-4")
+        self.assertListEqual(["user-id-0;user-id-1", "user-id-2;user-id-3"], groups_created)
+
+
+@mock.patch('coffee_bot.match_making.make_matches_for_members')
+@mock.patch('coffee_bot.match_making.members_in_channel_besides_me')
+class MatchingInChannelCase(MatchMakingCaseBase):
+    def test_minimum_members_with_message(self, members_besides_me, make_matches_for_members):
+        members_besides_me.return_value = self._generate_n_members(2)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            too_few_message="Too few",
+            minimum_members=3,
+        )
+
+        self.client.create_post.assert_called_with(channel_id="channel-id", message="Too few")
+        make_matches_for_members.assert_not_called()
+
+    def test_minimum_members_without_message(self, members_besides_me, make_matches_for_members):
+        members_besides_me.return_value = self._generate_n_members(2)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            too_few_message=None,
+            minimum_members=3,
+        )
+
+        self.client.create_post.assert_not_called()
+        make_matches_for_members.assert_not_called()
+
+    def test_group_hail_with_message(self, members_besides_me, make_matches_for_members):
+        make_matches_for_members.return_value = (["group-id-1", "group-id-2"], None)
+
+        members_besides_me.return_value = self._generate_n_members(2)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            group_hail="Hail Group!",
+            minimum_members=0,
+        )
+
+        self.client.create_post.assert_has_calls([
+            mock.call(channel_id="group-id-1", message="Hail Group!"),
+            mock.call(channel_id="group-id-2", message="Hail Group!"),
+        ])
+
+    def test_group_hail_without_message(self, members_besides_me, make_matches_for_members):
+        make_matches_for_members.return_value = (["group-id-1", "group-id-2"], None)
+
+        members_besides_me.return_value = self._generate_n_members(2)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            group_hail=None,
+            minimum_members=0,
+        )
+
+        self.client.create_post.assert_not_called()
+
+    def test_left_out_with_message(self, members_besides_me, make_matches_for_members):
+        make_matches_for_members.return_value = ([], "user-id-0")
+        self.client.get_user.return_value = {'id': "user-id-0", 'username': "UserId0",
+                                             "first_name": "Firstname", "last_name": "LASTNAME"}
+
+        members_besides_me.return_value = self._generate_n_members(1)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            left_out_message="Couldn't match {user[first_name]} {user[last_name]} this time",
+            minimum_members=0,
+        )
+
+        self.client.create_post.assert_called_with(
+            channel_id="channel-id",
+            message="Couldn't match Firstname LASTNAME this time",
+        )
+
+    def test_left_out_without_message(self, members_besides_me, make_matches_for_members):
+        make_matches_for_members.return_value = ([], "user-id-0")
+
+        members_besides_me.return_value = self._generate_n_members(2)
+        match_making.matching_in_channel(
+            self.client,
+            channel_id="channel-id",
+            left_out_message=None,
+            minimum_members=0,
+        )
+
+        self.client.get_user.assert_not_called()
+        self.client.create_post.assert_not_called()

tests/test_random_tuple.py

 # pylint: disable=missing-class-docstring, missing-function-docstring
 import unittest
-from coffee_bot import coffee_bot
+from coffee_bot import match_making
 
 
 class RandomNonrepeatingTuplesCase(unittest.TestCase):
@@ -8,7 +8,7 @@ class RandomNonrepeatingTuplesCase(unittest.TestCase):
         population = ["alma", "korte", "cseresznye", "meggy"]
 
         # pylint: disable=unnecessary-comprehension
-        random_pairs = [p for p in coffee_bot.random_nonrepeating_tuples(population, 2)]
+        random_pairs = [p for p in match_making.random_nonrepeating_tuples(population, 2)]
 
         self.assertEqual(len(random_pairs), len(population)/2)
 
@@ -21,7 +21,7 @@ class RandomNonrepeatingTuplesCase(unittest.TestCase):
         population = ["alma", "korte", "cseresznye"]
 
         # pylint: disable=unnecessary-comprehension
-        random_pairs = [p for p in coffee_bot.random_nonrepeating_tuples(population, 2)]
+        random_pairs = [p for p in match_making.random_nonrepeating_tuples(population, 2)]
 
         self.assertEqual(len(random_pairs), (len(population)+1)/2)
 
@@ -48,6 +48,6 @@ class RandomNonrepeatingTuplesCase(unittest.TestCase):
         population = []
 
         # pylint: disable=unnecessary-comprehension
-        random_pairs = [p for p in coffee_bot.random_nonrepeating_tuples(population, 2)]
+        random_pairs = [p for p in match_making.random_nonrepeating_tuples(population, 2)]
 
         self.assertEqual(len(random_pairs), 0)