Ticket 1262: Added collaborators to ticket details and collaborate co…

…mmands. Displayed as Discord IDs when data is available.

docs/devlog.md

@@ -1,6 +1,32 @@
 # Netbot Development Log
 
-## 2024-10-29
+## 2024-10-31
+
+Updated `requirements.txt` using https://github.com/simion/pip-upgrader, but immediately noticed problems with:
+```
+TypeError: object MagicMock can't be used in 'await' expression
+```
+any time there was a test call into the discord framework, `pycord`.
+
+Rolled back to the version that worked `py-cord==2.4.1` and support libs: `aiohttp==3.8.6, async-timeout==4.0.3`
+
+With this change, all tests pass with 66% coverage.
+
+Back to ticket 1262.
+
+Added method to format a Discord ID from a redmine user record (using the compound discord id), used it to render the collaborators that have discord ids. (full name from redmine if not). Updated tests to check that collaborator is shown with valid value when avilable.
+
+NOTE: "watchers" (as they're called in the redmine API) are not in a standard ticket-get query. When the collaborator details are necessary, the call to `user_mgr.get()` must contain the parameter `include='watchers'`:
+```
+client = redmine.Client.from_env()
+ticket = client.ticket_mgr.get(ticket_id, include="watchers")
+# OR, as used for epics:
+ticket = client.ticket_mgr.get(ticket_id, include="watchers,children")
+```
+
+Tests look good. Committing.
+
+## 2024-10-30
 
 After deploying the latest, found a bug in the the description modal:
 ```
@@ -17,7 +43,6 @@ ValueError: title must be 45 characters or fewer
 Updated code. Added test case `test_description_modal_init` to test that the modal dialog is initialize correctly.
 
 
-
 ## 2024-10-29
 
 Response from the Redmine post:

netbot/cog_tickets.py

@@ -306,7 +306,7 @@ async def query(self, ctx: discord.ApplicationContext, term:str = ""):
     async def details(self, ctx: discord.ApplicationContext, ticket_id:int):
         """Update status on a ticket, using: unassign, resolve, progress"""
         #log.debug(f"found user mapping for {ctx.user.name}: {user}")
-        ticket = self.redmine.ticket_mgr.get(ticket_id, include="children")
+        ticket = self.redmine.ticket_mgr.get(ticket_id, include="children,watchers")
         if ticket:
             await self.bot.formatter.print_ticket(ticket, ctx)
         else:
@@ -332,7 +332,8 @@ async def collaborate(self, ctx: discord.ApplicationContext, ticket_id:int, memb
         ticket = self.redmine.ticket_mgr.get(ticket_id)
         if ticket:
             self.redmine.ticket_mgr.collaborate(ticket.id, user)
-            await self.bot.formatter.print_ticket(self.redmine.ticket_mgr.get(ticket.id), ctx)
+            updated = self.redmine.ticket_mgr.get(ticket.id, include="watchers")
+            await self.bot.formatter.print_ticket(updated, ctx)
         else:
             await ctx.respond(f"Ticket {ticket_id} not found.") # print error
 

netbot/formatting.py

@@ -52,10 +52,6 @@ def get_emoji(key:str) -> str:
     'EPIC': discord.Color.dark_gray(),
 }
 
-#EPIC_TAG = "[EPIC] "
-#def strip_epic_tag(subject:str) -> str:
-#     return subject[len(EPIC_TAG):] if subject.startswith(EPIC_TAG) else subject
-
 
 class DiscordFormatter():
     """
@@ -79,11 +75,6 @@ async def print_tickets(self, title:str, tickets:list[Ticket], ctx:discord.Appli
 
     async def print_ticket(self, ticket, ctx:discord.ApplicationContext):
         await ctx.respond(embed=self.ticket_embed(ctx, ticket))
-        #msg = self.format_ticket_details(ticket)
-        #if len(msg) > MAX_MESSAGE_LEN:
-        #    log.warning("message over {MAX_MESSAGE_LEN} chars. truncing.")
-        #    msg = msg[:MAX_MESSAGE_LEN]
-        #await ctx.respond(msg)
 
 
     def format_registered_users(self, users: list[User]) -> str:
@@ -284,13 +275,29 @@ def get_user_id(self, ctx: discord.ApplicationContext, ticket:Ticket) -> str:
         if ticket is None or ticket.assigned_to is None:
             return ""
 
-        user = ctx.bot.redmine.user_mgr.get(ticket.assigned_to.id)
+        user_str = self.format_discord_member(ctx, ticket.assigned_to.id)
+        if not user_str:
+            user_str = ticket.assigned_to.name
+
+        return user_str
+
+
+    def format_discord_member(self, ctx: discord.ApplicationContext, user_id:int) -> str:
+        user = ctx.bot.redmine.user_mgr.get(user_id) # call to cache
         if user and user.discord_id:
-            member = self.lookup_discord_user(ctx, user.discord_id)
-            if member:
-                return f"<@!{member.id}>"
+            return f"<@!{user.discord_id.id}>"
+        if user:
+            return user.name
+        return ""
 
-        return ticket.assigned
+
+    def format_collaborators(self, ctx: discord.ApplicationContext, ticket:Ticket) -> str:
+        if not ticket.watchers:
+            return ""
+        if len(ticket.watchers) > 1:
+            return ",".join([self.format_discord_member(ctx, watcher.id) for watcher in ticket.watchers])
+
+        return self.format_discord_member(ctx, ticket.watchers[0].id)
 
 
     def ticket_embed(self, ctx: discord.ApplicationContext, ticket:Ticket) -> discord.Embed:
@@ -314,6 +321,9 @@ def ticket_embed(self, ctx: discord.ApplicationContext, ticket:Ticket) -> discor
         if ticket.assigned_to:
             embed.add_field(name="Owner", value=self.get_user_id(ctx, ticket))
 
+        if ticket.watchers:
+            embed.add_field(name="Collaborators", value=self.format_collaborators(ctx, ticket))
+
         # list the sub-tickets
         if ticket.children:
             buff = ""

redmine/model.py

@@ -225,7 +225,7 @@ def parse_discord_custom_field(self) -> NamedId:
                     log.error(f"Unable to parse custom field for discord ID: {id_str}")
             else:
                 # no id. assume old style
-                return NamedId(-1, id_str)
+                return NamedId(0, id_str)
         else:
             return None
 

tests/test_cog_tickets.py

@@ -81,7 +81,7 @@ async def test_new_ticket(self):
                 self.assertTrue(field.value.endswith(" New"))
                 found = True
             if field.name == "Owner":
-                self.assertEqual(field.value, self.user.name)
+                self.assertIn(str(self.user.discord_id.id), field.value)
                 found = True
         self.assertTrue(found, "Owner field not found in embed")
 
@@ -97,7 +97,7 @@ async def test_new_ticket(self):
                 self.assertTrue(field.value.endswith(" In Progress"))
                 found = True
             if field.name == "Owner":
-                self.assertEqual(field.value, self.user.name)
+                self.assertIn(str(self.user.discord_id.id), field.value)
                 found = True
         self.assertTrue(found, "Owner field not found in embed")
 
@@ -113,7 +113,7 @@ async def test_new_ticket(self):
                 self.assertTrue(field.value.endswith(" Resolved"))
                 found = True
             if field.name == "Owner":
-                self.assertEqual(field.value, self.user.name)
+                self.assertIn(str(self.user.discord_id.id), field.value)
                 found = True
         self.assertTrue(found, "Owner field not found in embed")
 
@@ -148,6 +148,7 @@ async def test_ticket_unassign(self):
 
     async def test_ticket_collaborate(self):
         ticket = self.create_test_ticket()
+        discord_id = self.user.discord_id.id
 
         # add a collaborator
         ctx = self.build_context()
@@ -156,8 +157,13 @@ async def test_ticket_collaborate(self):
         self.assertIn(str(ticket.id), embed.title)
         self.assertIn(ticket.subject, embed.title)
 
-        # TODO Add check for list of collaborators,
-        # which is not currently displayed in embeds, see ticket 1262.
+        # Check for list of collaborators
+        found = False
+        for field in embed.fields:
+            if field.name == "Collaborators":
+                self.assertIn(str(discord_id), field.value)
+                found = True
+        self.assertTrue(found, "Collaborators field not found in embed")
 
         # delete ticket with redmine api, assert
         self.redmine.ticket_mgr.remove(ticket.id)

tests/test_imap.py

@@ -130,7 +130,7 @@ def test_new_account_from_email(self):
     def test_subject_search(self):
         # create a new ticket with unique subject
         tag = test_utils.tagstr()
-        user = self.redmine.user_mgr.get_by_name("admin") # FIXME: create_test_user in test_utils
+        user = self.redmine.user_mgr.get_by_name("test-user")
         self.assertIsNotNone(user)
         subject = f"Test {tag} {tag} {tag}"
         message = Message(user.mail, subject, f"to-{tag}@example.com", f"cc-{tag}@example.com")

tests/test_utils.py

@@ -82,26 +82,6 @@ def lookup_test_user(user_mgr:UserManager) -> User:
     return user
 
 
-def create_test_user(user_mgr:UserManager, tag:str):
-    # create new test user name: [email protected], login test-12345
-    first = "test-" + tag
-    last = "Testy"
-    #fullname = f"{first} {last}" ### <--
-    email = first + "@example.com"
-
-    # create new redmine user, using redmine api
-    user = user_mgr.create(email, first, last, None)
-
-    # create temp discord mapping with redmine api, assert
-    # create_discord_mapping will cache the new user
-    discord_id = random.randint(1000000,9999999)
-    discord_user = "discord-" + tag ### <--
-    user_mgr.create_discord_mapping(user, discord_id, discord_user)
-
-    # lookup based on login
-    return user_mgr.get_by_name(user.login)
-
-
 def mock_ticket(**kwargs) -> Ticket:
     #return json_ticket('test-ticket.json', **kwargs)
     return json_ticket('issues/595.json', **kwargs)
@@ -281,9 +261,9 @@ def build_context(self) -> ApplicationContext:
         ctx.bot.redmine = self.redmine
         ctx.user = mock.AsyncMock(discord.Member)
         ctx.user.name = self.user.discord_id.name
+        ctx.user.id = self.user.discord_id.id
         ctx.command = mock.AsyncMock(discord.ApplicationCommand)
         ctx.command.name = unittest.TestCase.id(self)
-        log.debug(f"created ctx with {self.user.discord_id}: {ctx}")
         return ctx