From d3fc0402d79df24ced897f4cbbcc558e0323c7d0 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Wed, 6 Mar 2024 21:18:24 -0500 Subject: [PATCH] Refactor ranking to treat orphans as members of a single low-rank block Previously, each orphan was added to its own block. The rank of these were never compared, and so it was possibly for a highly ranked orphan issue to never be overtaken by other highly ranked blocks. --- src/rules/team/rank.py | 57 ++++++++++++++++++++++++++++-------------- src/tests/conftest.py | 8 ++++-- src/tests/test_rank.py | 10 ++++++-- 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/rules/team/rank.py b/src/rules/team/rank.py index e82e23d..6645374 100644 --- a/src/rules/team/rank.py +++ b/src/rules/team/rank.py @@ -94,11 +94,23 @@ def parent_is_inprogress(self): return False return self.parent_issue.fields.status.statusCategory.name == "In Progress" + @property + def rank(self): + rank_field_id = self.issues[0].raw["Context"]["Field Ids"]["Rank"] + if self.parent_issue: + return getattr(self.parent_issue.fields, rank_field_id) + else: + return float("-inf") + def __str__(self) -> str: p_key = self.parent_issue.key if self.parent_issue else None i_keys = [i.key for i in self.issues] return f"{p_key}: {', '.join(i_keys)}" + def claims(self, issue) -> bool: + parent_issue = issue.raw["Context"]["Related Issues"]["Parent"] + return self.parent_issue == parent_issue + class Blocks(list): def __init__(self, issues: list[jira.resources.Issue]) -> None: @@ -109,19 +121,15 @@ def __init__(self, issues: list[jira.resources.Issue]) -> None: def add_issue(self, issue: jira.resources.Issue) -> None: """Add an issue to the right block""" block = None - parent_issue = issue.raw["Context"]["Related Issues"]["Parent"] - if parent_issue is None: - block = Block(None) + addBlock = True + for block in self.blocks: + if block.claims(issue): + addBlock = False + break + if addBlock: + parent_issue = issue.raw["Context"]["Related Issues"]["Parent"] + block = Block(parent_issue) self.blocks.append(block) - else: - addBlock = True - for block in self.blocks: - if block.parent_issue == parent_issue: - addBlock = False - break - if addBlock: - block = Block(parent_issue) - self.blocks.append(block) block.issues.append(issue) def get_issues(self) -> list[jira.resources.Issue]: @@ -141,28 +149,26 @@ def get_issues(self) -> list[jira.resources.Issue]: def sort(self): self._sort_by_project_rank() self._sort_by_status() + self._deprioritize_orphan_blocks() def _sort_by_project_rank(self): """Rerank blocks based on the block's project rank. Blocks are switch around, but a block can only be switched with a block of the same parent project. """ - rank_field_id = self.blocks[0].issues[0].raw["Context"]["Field Ids"]["Rank"] # For each project, generate a ranked list of issues per_project_ranking = {None: []} for block in self.blocks: parent_issue = block.parent_issue if parent_issue is None: - per_project_ranking[None].append(block) - continue - - project_key = parent_issue.fields.project.key + project_key = None + else: + project_key = parent_issue.fields.project.key project_ranking = per_project_ranking.get(project_key, []) - block_rank = getattr(parent_issue.fields, rank_field_id) for index, i_block in enumerate(project_ranking): - if block_rank < getattr(i_block.parent_issue.fields, rank_field_id): + if block.rank < i_block.rank: project_ranking.insert(index, block) break if block not in project_ranking: @@ -194,3 +200,16 @@ def _sort_by_status(self): new.append(block) self.blocks = inprogress + new + + def _deprioritize_orphan_blocks(self): + """Issues with no parent should fall to the bottom of the list.""" + orphans = [] + children = [] + + for block in self.blocks: + if not block.parent_issue: + orphans.append(block) + else: + children.append(block) + + self.blocks = children + orphans diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 1ce9c7b..3413ec2 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -19,16 +19,20 @@ def __init__(self, idx, project, parent, rank): self.fields.rank = rank def __repr__(self): - return f"<{type(self).__name__} {self.fields.project.key}-{self.idx}>" + return f"<{type(self).__name__} {self.fields.project.key}-{self.idx}({self.fields.rank})>" @pytest.fixture def issues(): project = "TESTPROJECT" + child0 = MockIssue("child0", project, None, 0) parent1 = MockIssue("parent1", project, None, 1) child1 = MockIssue("child1", project, parent1, 2) parent2 = MockIssue("parent2", project, None, 3) child2 = MockIssue("child2", project, parent2, 4) parent3 = MockIssue("parent3", project, None, 5) child3 = MockIssue("child3", project, parent3, 6) - return dict(child1=child1, child2=child2, child3=child3) + child4 = MockIssue("child4", project, None, 7) + return dict( + child0=child0, child1=child1, child2=child2, child3=child3, child4=child4 + ) diff --git a/src/tests/test_rank.py b/src/tests/test_rank.py index 038ae15..dd9eb5c 100644 --- a/src/tests/test_rank.py +++ b/src/tests/test_rank.py @@ -1,8 +1,12 @@ +import operator as op + import rules.team.rank def test_rank_idempotence(issues): - blocks = rules.team.rank.Blocks(list(issues.values())) + issues = list(sorted(issues.values(), key=op.attrgetter("fields.rank"))) + issues = [issue for issue in issues if issue.key not in ("child0", "child4")] + blocks = rules.team.rank.Blocks(issues) old_ranking = blocks.get_issues() blocks.sort() new_ranking = blocks.get_issues() @@ -10,7 +14,7 @@ def test_rank_idempotence(issues): def test_rank_single_move(issues): - issues["child3"].raw["Context"]["Related Issues"]["Parent"].fields.rank = 0 + issues["child3"].raw["Context"]["Related Issues"]["Parent"].fields.rank = -1 blocks = rules.team.rank.Blocks(list(issues.values())) old_ranking = blocks.get_issues() blocks.sort() @@ -22,3 +26,5 @@ def test_rank_single_move(issues): assert new_ranking[3].key == "child1" assert new_ranking[4].key == "parent2" assert new_ranking[5].key == "child2" + assert new_ranking[6].key == "child0" + assert new_ranking[7].key == "child4"