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

@@ -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

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
+    )

src/tests/test_rank.py

@@ -1,16 +1,20 @@
+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()
     assert new_ranking == old_ranking
 
 
 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"