diff --git a/netbot/cog_scn.py b/netbot/cog_scn.py index 6a4396c..88c7130 100644 --- a/netbot/cog_scn.py +++ b/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") diff --git a/netbot/cog_tickets.py b/netbot/cog_tickets.py index f614eb3..497a2c9 100644 --- a/netbot/cog_tickets.py +++ b/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 diff --git a/netbot/formatting.py b/netbot/formatting.py index f028c14..dd0b992 100644 --- a/netbot/formatting.py +++ b/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}" diff --git a/netbot/netbot.py b/netbot/netbot.py index c54da9c..d0673ea 100755 --- a/netbot/netbot.py +++ b/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: diff --git a/redmine/model.py b/redmine/model.py index dccfe0f..59f9808 100644 --- a/redmine/model.py +++ b/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: diff --git a/redmine/redmine.py b/redmine/redmine.py index 2e1d37c..11df573 100644 --- a/redmine/redmine.py +++ b/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" diff --git a/redmine/users.py b/redmine/users.py index 72503de..794bead 100644 --- a/redmine/users.py +++ b/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: diff --git a/tests/test_cog_scn.py b/tests/test_cog_scn.py index c1f721b..8d6fde6 100755 --- a/tests/test_cog_scn.py +++ b/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}" diff --git a/tests/test_netbot.py b/tests/test_netbot.py index 6915cd8..c74f721 100755 --- a/tests/test_netbot.py +++ b/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}" diff --git a/tests/test_utils.py b/tests/test_utils.py index a1bdbbe..7951334 100755 --- a/tests/test_utils.py +++ b/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}")