diff --git a/docs/devlog.md b/docs/devlog.md index 1140c41..9754aef 100644 --- a/docs/devlog.md +++ b/docs/devlog.md @@ -1,5 +1,78 @@ ## Development Log +## 2024-10-24 + +After finding some significant bugs in the latest release, decided to get more rigious with a test-driven-development approach. For this project that means: +1. Identify the missing or buggy behavior. +2. Code one or more test cases that check the expected behavior. +3. Fail those tests until... +4. Write valid working code to provide the correct behavior. +5. Add those tests to the automated test suite. +6. Run the test suite before each change to the `main` or any shared branch. + +Today, the first step will be building a tests around a minimal "sane" state after initialization to confirm that all expected *external* resources have been loaded correctly. Specifically, the loaded, parsed and cached data for: +- users +- teams +- priorities (the problem this time) +- trackers +- custom fields + +**Approach**: The last step of initialization before giving an "all ready" signal, the bot will run a sanity check to make sure that each of the resources mentioned above is available, non-zero and contain any referenced keys, like: +``` +INTAKE_TEAM = "ticket-intake" +INTAKE_TEAM_ID = 19 # FIXME +EPIC_PRIORITY_NAME = "EPIC" +BLOCKED_TEAM_NAME = "blocked" +CHANNEL_MAPPING +TEAM_MAPPING +``` + +Design: The top-level `Netbot` and any sub-components will implement a `sanity_check() -> dict[str,bool]` to describe it's expected states (with a true or false to indicate if that's working as expected). Sub-system results are aggreagated and returned (using dotted name strings: "netbot.redmine.user_mgr.teams"). Info log at bot startup will show passing state, or the bot sill fail to startup. + +Note: Looking over the date return for `/enumerations/issue_priorities.json`, there is no indication of the priority ordering other than the oder in the list (from low to high). Noting that Python 3.7 and later maintain insertion order, so a dict-iterator should work as well as a list iterator, *as long as the key-value pairs as inserted in the correct (reverse!) order* to put the higest priority first whenever iterating. +``` +{ + "issue_priorities": [ + { + "id": 1, + "name": "Low", + "is_default": false, + "active": true + }, + { + "id": 2, + "name": "Normal", + "is_default": true, + "active": true + }, + { + "id": 3, + "name": "High", + "is_default": false, + "active": true + }, + { + "id": 14, + "name": "EPIC", + "is_default": false, + "active": true + }, + { + "id": 4, + "name": "Urgent", + "is_default": false, + "active": true + }, + { + "id": 5, + "name": "Immediate", + "is_default": false, + "active": true + } + ] +} +``` + ### 2024-09-24 Starting work on ticket 1207, to add admin overrides for all actions that default to self. diff --git a/netbot/cog_tickets.py b/netbot/cog_tickets.py index d531d50..7ca00a8 100644 --- a/netbot/cog_tickets.py +++ b/netbot/cog_tickets.py @@ -31,16 +31,18 @@ def setup(bot:NetBot): def get_trackers(ctx: discord.AutocompleteContext): """Returns a list of trackers that begin with the characters entered so far.""" - trackers = ctx.bot.redmine.get_trackers() # this is expected to be cached IT'S NOT!!! + trackers = ctx.bot.redmine.ticket_mgr.get_trackers() # this is expected to be cached # .lower() is used to make the autocomplete match case-insensitive - return [tracker['name'] for tracker in trackers if tracker['name'].lower().startswith(ctx.value.lower())] + return [tracker for tracker in trackers.keys() if tracker.lower().startswith(ctx.value.lower())] def get_priorities(ctx: discord.AutocompleteContext): """Returns a list of priorities that begin with the characters entered so far.""" - priorities = ctx.bot.redmine.get_priorities() # this is expected to be cached + priorities = ctx.bot.redmine.ticket_mgr.get_priorities() # this is expected to be cached # .lower() is used to make the autocomplete match case-insensitive - return [priority.name for priority in priorities if priority.name.lower().startswith(ctx.value.lower())] + rtn = [priority for priority in priorities.keys() if priority.lower().startswith(ctx.value.lower())] + log.warning(f"PRIORITIES: {rtn}") + return rtn class PrioritySelect(discord.ui.Select): @@ -52,7 +54,7 @@ def __init__(self, bot_: discord.Bot): # Get the possible priorities options = [] - for priority in self.bot.redmine.get_priorities(): + for priority in self.bot.redmine.ticket_mgr.get_priorities(): options.append(discord.SelectOption(label=priority['name'], default=priority['is_default'])) # The placeholder is what will be shown when no option is selected. @@ -84,7 +86,7 @@ def __init__(self, bot_: discord.Bot): # Get the possible trackers options = [] - for tracker in self.bot.redmine.get_trackers(): + for tracker in self.bot.redmine.ticket_mgr.get_trackers(): options.append(discord.SelectOption(label=tracker['name'])) # The placeholder is what will be shown when no option is selected. @@ -118,7 +120,7 @@ def __init__(self, bot_: discord.Bot, ticket: Ticket): # Get the possible trackers options = [] - for tracker in self.bot.redmine.get_trackers(): + for tracker in self.bot.redmine.ticket_mgr.get_trackers(): options.append(discord.SelectOption(label=tracker['name'])) # The placeholder is what will be shown when no option is selected. @@ -585,19 +587,19 @@ async def tracker(self, ctx: discord.ApplicationContext, ticket_id:int, tracker: @ticket.command(name="priority", description="Update the tracker of a ticket") @option("ticket_id", description="ID of ticket to update", autocomplete=basic_autocomplete(default_ticket)) @option("priority", description="Priority to assign to ticket", autocomplete=get_priorities) - async def priority(self, ctx: discord.ApplicationContext, ticket_id:int, priority_str:str): + async def priority(self, ctx: discord.ApplicationContext, ticket_id:int, priority:str): user = self.redmine.user_mgr.find_discord_user(ctx.user.name) ticket = self.redmine.ticket_mgr.get(ticket_id) if ticket: # look up the priority - priority = self.bot.redmine.ticket_mgr.get_priority(priority_str) - if priority is None: - log.error(f"Unknown priority: {priority_str}") - await ctx.respond(f"Unknown priority: {priority_str}") + pri = self.bot.redmine.ticket_mgr.get_priority(priority) + if pri is None: + log.error(f"Unknown priority: {priority}") + await ctx.respond(f"Unknown priority: {priority}") return fields = { - "priority_id": priority.id, + "priority_id": pri.id, } updated = self.bot.redmine.ticket_mgr.update(ticket_id, fields, user.login) diff --git a/netbot/netbot.py b/netbot/netbot.py index 66dcb74..9433387 100755 --- a/netbot/netbot.py +++ b/netbot/netbot.py @@ -456,6 +456,9 @@ def main(): bot.load_extension("netbot.cog_scn") bot.load_extension("netbot.cog_tickets") + # sanity check! + client.sanity_check() + # run the bot bot.run_bot() diff --git a/redmine/redmine.py b/redmine/redmine.py index 11df573..6a729bc 100644 --- a/redmine/redmine.py +++ b/redmine/redmine.py @@ -36,6 +36,9 @@ def __init__(self, session:RedmineSession, user_mgr:UserManager, ticket_mgr:Tick self.user_mgr = user_mgr self.ticket_mgr = ticket_mgr + # sanity check + self.validate_sanity() # FATAL if not + @classmethod def from_session(cls, session:RedmineSession, default_project:int): @@ -55,7 +58,7 @@ def fromenv(cls): if token is None: raise RedmineException("Unable to load REDMINE_TOKEN") - default_project = os.getenv("DEFAULT_PROJECT_ID", default=SCN_PROJECT_ID) + default_project = int(os.getenv("DEFAULT_PROJECT_ID", default=str(SCN_PROJECT_ID))) return cls.from_session(RedmineSession(url, token), default_project) @@ -65,6 +68,18 @@ def reindex(self): self.user_mgr.reindex() # rebuild the user cache + def sanity_check(self) -> dict[str, bool]: + sanity = self.ticket_mgr.sanity_check() + return sanity + + def validate_sanity(self): + for subsystem, good in self.sanity_check().items(): + log.info(f"- {subsystem}: {good}") + if not good: + #log.critical(f"Subsystem {subsystem} not loading correctly.") + raise RedmineException(f"Subsystem {subsystem} not loading correctly.") + + def create_ticket(self, user:User, message:Message) -> Ticket: # NOTE to self re "projects": TicketManager.create supports a project ID # Need to find a way to pass it in. diff --git a/redmine/tickets.py b/redmine/tickets.py index a569300..5692d46 100644 --- a/redmine/tickets.py +++ b/redmine/tickets.py @@ -18,7 +18,7 @@ ISSUES_RESOURCE="/issues.json" ISSUE_RESOURCE="/issues/" DEFAULT_SORT = "status:desc,priority:desc,updated_on:desc" -SCN_PROJECT_ID = "1" # could lookup scn in projects +SCN_PROJECT_ID = 1 # could lookup scn in projects INTAKE_TEAM = "ticket-intake" INTAKE_TEAM_ID = 19 # FIXME EPIC_PRIORITY_NAME = "EPIC" @@ -30,12 +30,12 @@ class TicketManager(): """manage redmine tickets""" - def __init__(self, session: RedmineSession, default_project): + def __init__(self, session: RedmineSession, default_project:int): self.session: RedmineSession = session - self.priorities = [] + self.priorities = {} self.trackers = {} self.custom_fields = {} - self.default_project = default_project + self.default_project:int = default_project self.reindex() @@ -46,6 +46,17 @@ def reindex(self): self.custom_fields = self.load_custom_fields() + def sanity_check(self) -> dict[str, bool]: + subsystems: dict[str, bool] = {} + + subsystems['default_project'] = self.default_project > 0 + subsystems['priorities'] = self.get_priority(EPIC_PRIORITY_NAME) is not None + # todo trackers + # todo custom fields + + return subsystems + + def load_custom_fields(self) -> dict[str,NamedId]: # call redmine to get the ticket custom fields fields_response = self.session.get("/custom_fields.json") @@ -62,21 +73,26 @@ def get_custom_field(self, name:str) -> NamedId | None: return self.custom_fields.get(name, None) - def load_priorities(self) -> list[NamedId]: - """get all active priorities""" + def load_priorities(self) -> dict[str,NamedId]: + """load active priorities""" + + # Note: This relies on py3.7 insertion-ordered hashtables + priorities: dict[str,NamedId] = {} + resp = self.session.get("/enumerations/issue_priorities.json") - priorities = resp['issue_priorities'] # get specific dictionary in response - priorities = [NamedId(priority['id'], priority['name']) for priority in priorities] - # reverse the list, so higest is first - return reversed(priorities) + for priority in reversed(resp['issue_priorities']): + if priority.get('active', False): + priorities[priority['name']] = NamedId(priority['id'], priority['name']) + + return priorities def get_priority(self, name:str) -> NamedId | None: - # search - for priority in self.priorities: - if priority.name == name: - return priority - return None + return self.priorities.get(name, None) + + + def get_priorities(self) -> dict[str,NamedId]: + return self.priorities def load_trackers(self) -> dict[str,NamedId]: @@ -88,13 +104,17 @@ def load_trackers(self) -> dict[str,NamedId]: trackers[item['name']] = NamedId(id=item['id'], name=item['name']) return trackers else: - log.warning("No reackers to load") + log.warning("No trackers to load") def get_tracker(self, name:str) -> NamedId | None: return self.trackers.get(name, None) + def get_trackers(self) -> dict[str,NamedId]: + return self.trackers + + def create(self, user: User, message: Message, project_id: int = None, **params) -> Ticket: """create a redmine ticket""" # https://www.redmine.org/projects/redmine/wiki/Rest_Issues#Creating-an-issue diff --git a/tests/test_cog_tickets.py b/tests/test_cog_tickets.py index beea8a2..ede9d7b 100755 --- a/tests/test_cog_tickets.py +++ b/tests/test_cog_tickets.py @@ -4,12 +4,13 @@ import unittest import logging import re +from unittest.mock import MagicMock, patch import discord from dotenv import load_dotenv from netbot.netbot import NetBot -from netbot.cog_tickets import TicketsCog +from netbot.cog_tickets import TicketsCog, get_priorities, get_trackers from tests import test_utils @@ -241,6 +242,30 @@ async def test_create_invalid_discord_user(self): self.assertIn("/scn add", ctx.respond.call_args.args[0]) + async def test_get_trackers(self): + ctx = MagicMock(discord.AutocompleteContext) + ctx.bot = self.bot + ctx.value = "" + trackers = get_trackers(ctx) + self.assertTrue("Software-Dev" in trackers) + + ctx.value = "Ext" + trackers = get_trackers(ctx) + self.assertTrue("External-Comms-Intake" in trackers) + + + async def test_get_priorities(self): + ctx = MagicMock(discord.AutocompleteContext) + ctx.bot = self.bot + ctx.value = "" + priorities = get_priorities(ctx) + self.assertTrue("EPIC" in priorities) + + ctx.value = "Lo" + priorities = get_priorities(ctx) + self.assertTrue("Low" in priorities) + + if __name__ == '__main__': logging.basicConfig(level=logging.DEBUG, format="{asctime} {levelname:<8s} {name:<16} {message}", style='{') logging.getLogger("urllib3.connectionpool").setLevel(logging.INFO) diff --git a/tests/test_tickets.py b/tests/test_tickets.py index 654c6e5..bf11c33 100755 --- a/tests/test_tickets.py +++ b/tests/test_tickets.py @@ -71,7 +71,7 @@ def test_create_sub_ticket(self, mock_post:MagicMock): mock_post.assert_called_once() self.assertEqual(4242, check.parent.id) - # needs to pathc 10 calls to GET + # needs to patch 10 calls to GET # @patch('redmine.session.RedmineSession.get') # def test_epics(self, mock_get:MagicMock): # # setup the mock tickets @@ -145,6 +145,8 @@ def test_ticket_collaborate(self): def test_epics(self): check = self.tickets_mgr.get_epics() + #for e in check: + # print(e) self.assertEqual(10, len(check)) self.assertEqual(595, check[8].id) self.assertEqual(8, len(check[8].children))