all tests pass, including new mock test for complex discord id

netbot/cog_scn.py

@@ -99,7 +99,7 @@ def __init__(self, bot_: discord.Bot, users: list[User]) -> None:
 
         # Add buttons by rows
         for user in users:
-            self.add_item(ApproveButton(self.bot, label=user.discord_id))
+            self.add_item(ApproveButton(self.bot, label=user.discord))
 
 
     async def button_callback(self, button: discord.ui.Button, interaction: discord.Interaction):
@@ -172,19 +172,19 @@ def is_admin(self, user: discord.Member) -> bool:
     @option("member", description="Discord member collaborating with ticket", optional=True)
     async def add(self, ctx:discord.ApplicationContext, redmine_login:str, member:discord.Member=None):
         """add a Discord user to the Redmine ticketing integration"""
-        discord_name = ctx.user.name # by default, assume current user
+        discord_id = ctx.user # by default, assume current user
         if member:
             log.info(f"Overriding current user={ctx.user.name} with member={member.name}")
-            discord_name = member.name
+            discord_id = member
 
-        user = self.redmine.user_mgr.find(discord_name)
+        user = self.redmine.user_mgr.find(discord_id.name)
         if user:
-            await ctx.respond(f"Discord user: {discord_name} is already configured as redmine user: {user.login}")
+            await ctx.respond(f"Discord user: {discord_id.name} is already configured as redmine user: {user.login}")
         else:
             user = self.redmine.user_mgr.find(redmine_login)
             if user and self.is_admin(ctx.user):
-                self.redmine.user_mgr.create_discord_mapping(user, discord_name)
-                await ctx.respond(f"Discord user: {discord_name} has been paired with redmine user: {redmine_login}")
+                self.redmine.user_mgr.create_discord_mapping(user, discord_id.id, discord_id.name)
+                await ctx.respond(f"Discord user: {discord_id.name} has been paired with redmine user: {redmine_login}")
             else:
                 # case: unknown redmine_login -> new user request: register new user
                 modal = NewUserModal(self.redmine, redmine_login, title="Register new user")

netbot/cog_tickets.py

@@ -512,7 +512,7 @@ async def alert_ticket(self, ctx: discord.ApplicationContext, ticket_id:int):
                 await ctx.respond(f"ERROR: No thread for ticket ID: {ticket_id}, assign a fall-back") ## TODO
                 return
             msg = f"Ticket {ticket.id} is about will expire soon."
-            await thread.send(self.bot.formatter.format_ticket_alert(ticket.id, discord_ids, msg))
+            await thread.send(self.bot.formatter.format_ticket_alert(ticket, discord_ids, msg))
             await ctx.respond("Alert sent.")
         else:
             await ctx.respond(f"ERROR: Unkown ticket ID: {ticket_id}") ## TODO format error message

netbot/formatting.py

@@ -89,7 +89,7 @@ async def print_ticket(self, ticket, ctx:discord.ApplicationContext):
     def format_registered_users(self, users: list[User]) -> str:
         msg = ""
         for user in users:
-            msg = msg + f"{user.discord_id} -> {user.login} {user.name}, {user.mail}\n"
+            msg = msg + f"{user.discord} -> {user.login} {user.name}, {user.mail}\n"
         msg = msg.strip()
 
         if len(msg) > MAX_MESSAGE_LEN:
@@ -259,7 +259,8 @@ def format_expiration_notification(self, ticket:Ticket, discord_ids: list[str]):
 
 
     def format_ticket_alert(self, ticket: Ticket, discord_ids: list[str], msg: str) -> str:
-        ids_str = ["@" + id for id in discord_ids]
+        ids_str = ["<@" + id + ">" for id in discord_ids]
+        log.debug(f"ids_str={ids_str}, discord_ids={discord_ids}")
         return f"ALERT #{self.redmine_link(ticket)} {' '.join(ids_str)}: {msg}"
 
 

netbot/netbot.py

@@ -403,7 +403,7 @@ def find_ticket_thread(self, ticket_id:int) -> discord.Thread|None:
         return None # not found
 
 
-    def extract_ids_from_ticket(self, ticket: Ticket) -> list[str]:
+    def extract_ids_from_ticket(self, ticket: Ticket) -> set[str]:
         """Extract the Discord IDs from users interested in a ticket,
            using owner and collaborators"""
          # owner and watchers
@@ -412,15 +412,20 @@ def extract_ids_from_ticket(self, ticket: Ticket) -> list[str]:
             interested.append(ticket.assigned_to)
         interested.extend(ticket.watchers)
 
-        discord_ids: list[str] = []
+        log.debug(f"INTERESTED: {interested}")
+
+        discord_ids = set()
         for named in interested:
-            user = self.redmine.user_mgr.get(named.id)
+            user = self.redmine.user_mgr.cache.get(named.id) #expected to be cached
             if user:
-                discord_ids.append(user.discord_id)
+                if user.discord_id:
+                    discord_ids.add(user.discord_id)
+                else:
+                    log.info(f"No Discord ID for {named}")
             else:
                 log.info(f"ERROR: user ID {named} not found")
 
-        return []
+        return discord_ids
 
 
     def is_admin(self, user: discord.Member) -> bool:

redmine/model.py

@@ -99,6 +99,16 @@ def __str__(self) -> str:
         else:
             return str(self.id)
 
+    def as_token(self) -> str:
+        return f"{self.id}|{self.name}"
+
+    @classmethod
+    def from_token(cls, token: str):
+        vals = token.split("|")
+        discord_id = int(vals[0])
+        name = vals[1]
+        return cls(id=discord_id, name=name)
+
 
 @dataclass
 class TicketStatus(NamedId):
@@ -180,9 +190,11 @@ class User():
     status: int = 0
     custom_fields: list[CustomField]
 
+
     def __post_init__(self):
         self.custom_fields = [CustomField(**field) for field in self.custom_fields]
-        self.discord_id = self.get_custom_field(DISCORD_ID_FIELD)
+        self.discord_id = self.parse_discord_custom_field()
+
 
     def get_custom_field(self, name: str) -> str:
         for field in self.custom_fields:
@@ -191,6 +203,38 @@ def get_custom_field(self, name: str) -> str:
 
         return None
 
+
+    def set_custom_field(self, field_id: int, name: str, value: str) -> str:
+        for field in self.custom_fields:
+            if field.name == name:
+                old = field.value # old value
+                field.value = value
+                return old
+        # not found, add new
+        self.custom_fields.append(CustomField(field_id, name, value))
+        return None
+
+
+    def parse_discord_custom_field(self) -> NamedId:
+        id_str = self.get_custom_field(DISCORD_ID_FIELD)
+        if id_str and len(id_str) > 0:
+            if '|' in id_str:
+                try:
+                    return NamedId.from_token(id_str)
+                except Exception:
+                    log.error(f"Unable to parse custom field for discord ID: {id_str}")
+            else:
+                # no id. assume old style
+                return NamedId(-1, id_str)
+        else:
+            return None
+
+
+    @property
+    def discord(self) -> str:
+        if self.discord_id:
+            return self.discord_id.name
+
     @property
     def name(self) -> str:
         if self.firstname is None or len(self.firstname) < 2:
@@ -206,6 +250,9 @@ def set_field(self, fieldname:str, value):
         setattr(self, fieldname, value)
         log.debug(f"@{self.login}: {fieldname} <= {value}")
 
+    def asdict(self):
+        return dataclasses.asdict(self)
+
 
 @dataclass
 class UserResult:

redmine/redmine.py

@@ -17,7 +17,6 @@
 DEFAULT_SORT = "status:desc,priority:desc,updated_on:desc"
 TIMEOUT = 10 # seconds
 SYNC_FIELD_NAME = "syncdata"
-DISCORD_ID_FIELD = "Discord ID"
 BLOCKED_TEAM_NAME = "blocked"
 STATUS_REJECT = 5 # could to status lookup, based on "reject"
 

redmine/users.py

@@ -6,7 +6,7 @@
 import json
 
 
-from redmine.model import Team, User, UserResult
+from redmine.model import Team, User, UserResult, NamedId
 from redmine.session import RedmineSession, RedmineException
 
 
@@ -44,7 +44,7 @@ def cache_user(self, user: User) -> None:
         self.users[user.login] = user.id
         self.user_emails[user.mail] = user.id
         if user.discord_id:
-            self.discord_ids[user.discord_id] = user.id
+            self.discord_ids[user.discord_id.name] = user.id
 
 
     def cache_team(self, team: Team) -> None:
@@ -172,16 +172,13 @@ def update(self, user:User, fields:dict) -> User:
         data = {}
         data['user'] = fields
 
-        response = self.session.put(f"/users/{user.id}.json", json.dumps(data))
+        # put the updated date
+        self.session.put(f"/users/{user.id}.json", json.dumps(data))
 
-        # check status
-        if response:
-            # get and return the updated user
-            user = self.get(user.id)
-            log.debug(f"updated id={user.id}: user: {user}")
-            return user
-        else:
-            return None
+        # get and return the updated user
+        user = self.get(user.id)
+        log.debug(f"updated id={user.id}: user: {user}")
+        return user
 
 
     def get_by_name(self, username:str) -> User:
@@ -311,14 +308,18 @@ def remove(self, user: User):
             log.info(f"deleted user {user.id}")
 
 
-    def create_discord_mapping(self, user:User, discord_name:str) -> User:
+    def create_discord_mapping(self, user:User, discord_id: int, discord_name:str) -> User:
         field_id = 2 ## FIXME "Discord ID"search for me in cached custom fields
+        named = NamedId(id=discord_id, name=discord_name)
         fields = {
             "custom_fields": [
-                { "id": field_id, "value": discord_name } # cf_4, custom field syncdata
+                { "id": field_id, "value": named.as_token() }
             ]
         }
-        return self.update(user, fields)
+        updated = self.update(user, fields)
+        # cache updated user, based on now-or-changed discord id.
+        self.cache.cache_user(updated)
+        return updated
 
     # for testing
     def remove_discord_mapping(self, user:User) -> User:

tests/test_cog_scn.py

@@ -37,7 +37,7 @@ def setUp(self):
     async def test_add_self(self):
         # invoke "add" to add a discord mapping for the test user.
         # setup: remove existing mapping
-        discord_id = self.user.discord_id
+        discord_id = self.user.discord_id.name
         self.user_mgr.remove_discord_mapping(self.user)
         self.user_mgr.reindex_users() # to remove the discord ID from the cache
 
@@ -179,7 +179,7 @@ async def test_locked_during_sync_ticket(self):
         message = unittest.mock.AsyncMock(discord.Message)
         message.content = f"This is a new note about ticket #{ticket.id} for test {self.tag}"
         message.author = unittest.mock.AsyncMock(discord.Member)
-        message.author.name = self.user.discord_id
+        message.author.name = self.user.discord_id.name
 
         thread = unittest.mock.AsyncMock(discord.Thread)
         thread.name = f"Ticket #{ticket.id}: {ticket.subject}"

tests/test_netbot.py

@@ -47,7 +47,7 @@ async def test_synchronize_ticket(self):
         message = unittest.mock.AsyncMock(discord.Message)
         message.content = f"This is a new note about ticket #{ticket.id} for test {self.tag}"
         message.author = unittest.mock.AsyncMock(discord.Member)
-        message.author.name = self.user.discord_id
+        message.author.name = self.user.discord_id.name
 
         thread = unittest.mock.AsyncMock(discord.Thread)
         thread.name = f"Ticket #{ticket.id}"
@@ -83,7 +83,7 @@ async def test_sync_ticket_long_message(self):
         message = unittest.mock.AsyncMock(discord.Message)
         message.content = f"This is a new note about ticket #{ticket.id} for test {self.tag}"
         message.author = unittest.mock.AsyncMock(discord.Member)
-        message.author.name = self.user.discord_id
+        message.author.name = self.user.discord_id.name
 
         thread = unittest.mock.AsyncMock(discord.Thread)
         thread.name = f"Ticket #{ticket.id}"

tests/test_utils.py

@@ -57,6 +57,11 @@ def randstr(length:int=12) -> str:
     return ''.join(random.choices(string.ascii_uppercase + string.digits, k=length))
 
 
+def randint(maxval:int=9999999) -> int:
+    minval = 999 if maxval > 1000 else 0
+    return random.randint(minval, maxval)
+
+
 def load_json(filename:str):
     #with open(os.path.join(TEST_DATA, filename), 'r', encoding="utf-8") as file:
     with open(TEST_DATA + filename, 'r', encoding="utf-8") as file:
@@ -80,8 +85,9 @@ def create_test_user(user_mgr:UserManager, tag:str):
 
     # 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_user)
+    user_mgr.create_discord_mapping(user, discord_id, discord_user)
 
     # lookup based on login
     return user_mgr.get_by_name(user.login)
@@ -106,10 +112,11 @@ def json_ticket(file:str, **kwargs) -> Ticket:
 
 def mock_user(tag: str) -> User:
     return User(
-        id=5,
+        id=random.randint(1000, 9999),
         login=f'test-{tag}',
         mail=f'{tag}@example.org',
-        custom_fields=[ {"id": 2, "name":'Discord ID', "value":f'discord-{tag}'} ],
+        #custom_fields=[ {"id": 2, "name":'Discord ID', "value":f'discord-{tag}'} ],
+        custom_fields=[],
         admin=False,
         firstname='Test',
         lastname=tag,
@@ -235,7 +242,7 @@ def build_context(self) -> ApplicationContext:
         ctx = mock.AsyncMock(ApplicationContext)
         ctx.bot = mock.AsyncMock(NetBot)
         ctx.user = mock.AsyncMock(discord.Member)
-        ctx.user.name = self.user.discord_id
+        ctx.user.name = self.user.discord_id.name
         ctx.command = mock.AsyncMock(discord.ApplicationCommand)
         ctx.command.name = unittest.TestCase.id(self)
         log.debug(f"created ctx with {self.user.discord_id}: {ctx}")