Repository: bennettoxford/bennettbot
Issue category: Feature Request

Issue:
Refactor Team REX Standup Rota
#674  introduced the concept of non-spreadsheet based `RotaReporter`s and some improvements to how staff slack/Github usernames are handled. The Team REX standup rota could be refactored to use these new structures, for clarity and such that users are notified on Slack when they are on duty in the Rota.


Hints:
Following on from #674, would like to also address if quick:
- re-name the dependabot rota as it's no longer using dependabot...
- add a note in `people.py` about the consequences of adding to or reordering `TEAM_REX`.

Patch:
diff --git a/workspace/dependabot/jobs.py b/workspace/dependabot/jobs.py
index ad92d5b4..602c491a 100644
--- a/workspace/dependabot/jobs.py
+++ b/workspace/dependabot/jobs.py
@@ -1,7 +1,7 @@
 from datetime import date, timedelta
 from itertools import cycle, islice
 
-from workspace.utils.people import TEAM_REX, People, get_formatted_slack_username
+from workspace.utils.people import TEAM_REX, People
 from workspace.utils.rota import RotaReporter
 
 
@@ -11,6 +11,7 @@ class DependabotRotaReporter(RotaReporter):
     The candidates are currently Team Rex (except Katie).
     If the candidate definition or Team changes this will affect
     the rota offset and the rota will restart at an arbitrary point.
+    Consider redesigning class to include an offset if that happens.
     """
 
     def get_rota(self) -> dict:
@@ -37,9 +38,9 @@ def get_rota_text_for_week(
     ) -> str:
         checker = rota[monday]
         if this_or_next == "this":
-            checker = get_formatted_slack_username(checker.value)
+            checker = checker.formatted_slack_username
         else:
-            checker = checker.name.title()
+            checker = checker.human_readable
         return f"To review dependabot PRs {this_or_next} week ({self.format_week(monday)}): {checker}"
 
 
diff --git a/workspace/report/generate_report.py b/workspace/report/generate_report.py
index d1d036ac..179ab890 100644
--- a/workspace/report/generate_report.py
+++ b/workspace/report/generate_report.py
@@ -5,10 +5,7 @@
 import requests
 
 from workspace.utils.blocks import get_basic_header_and_text_blocks
-from workspace.utils.people import (
-    get_formatted_slack_username,
-    get_person_from_github_username,
-)
+from workspace.utils.people import People
 
 
 URL = "https://api.github.com/graphql"
@@ -174,7 +171,7 @@ def get_status_and_summary(card):  # pragma: no cover
     title = card["content"]["title"]
     url = card["content"].get("bodyUrl")
     assignees = " / ".join(
-        get_formatted_slack_username(get_person_from_github_username(node["login"]))
+        People.by_github_username(node["login"]).formatted_slack_username
         for node in card["content"]["assignees"]["nodes"]
     )
 
diff --git a/workspace/standup/jobs.py b/workspace/standup/jobs.py
index 39ab7dd4..ba0144c3 100644
--- a/workspace/standup/jobs.py
+++ b/workspace/standup/jobs.py
@@ -3,12 +3,13 @@
 from datetime import date, datetime, timedelta
 
 from workspace.utils import blocks
+from workspace.utils.people import People
 
 
 PAIRS = {
-    "monday": ["Mike", "Jon"],
-    "wednesday": ["Mary", "Steve"],
-    "friday": ["Thomas", "Katie"],
+    "monday": [People.MIKE, People.JON],
+    "wednesday": [People.MARY, People.STEVE],
+    "friday": [People.THOMAS, People.KATIE],
 }
 
 
@@ -38,11 +39,12 @@ def weekly_rota(args):
     header = "Team Rex stand ups this week"
     days = "\n".join(
         [
-            f"{day_of_week.title()}: {PAIRS[day_of_week][primary]} (backup: {PAIRS[day_of_week][secondary]})"
+            f"{day_of_week.title()}: "
+            f"{PAIRS[day_of_week][primary].human_readable} "
+            f"(backup: {PAIRS[day_of_week][secondary].human_readable})"
             for day_of_week in PAIRS.keys()
         ]
     )
-
     return json.dumps(blocks.get_basic_header_and_text_blocks(header, days))
 
 
@@ -53,7 +55,11 @@ def daily_rota(args):
     primary = is_even_week(rota_date)
     secondary = 0 if primary else 1
     header = "Team Rex stand up"
-    body = f"{day_of_week.title()}: {PAIRS[day_of_week][primary]} (backup: {PAIRS[day_of_week][secondary]})"
+    body = (
+        f"{day_of_week.title()}: "
+        f"{PAIRS[day_of_week][primary].formatted_slack_username} "
+        f"(backup: {PAIRS[day_of_week][secondary].human_readable})"
+    )
 
     return json.dumps(blocks.get_basic_header_and_text_blocks(header, body))
 
diff --git a/workspace/utils/people.py b/workspace/utils/people.py
index b1aa3b2a..76009a7a 100644
--- a/workspace/utils/people.py
+++ b/workspace/utils/people.py
@@ -1,18 +1,56 @@
-from collections import namedtuple
-from enum import Enum
+from dataclasses import dataclass
+from typing import Optional
 
 
-Person = namedtuple("Person", ["github_username", "slack_username"])
+@dataclass(order=True)
+class Person:
+    github_username: str
+    slack_username: str
+    # Optional because this is mostly set in PersonCollection.__init__().
+    human_readable: Optional[str] = ""
 
+    @property
+    def formatted_slack_username(self) -> str:
+        return f"<@{self.slack_username}>"
 
-class People(Enum):
+
+class PersonCollection(type):
+    """Metaclass for collections of Person instances."""
+
+    def __init__(cls, name, bases, namespace):
+        # dict mapping github_username to People instances, for later lookups.
+        cls._by_github_username = {}
+
+        people_items = [
+            item for item in namespace.items() if isinstance(item[1], Person)
+        ]
+        for attribute_name, person in people_items:
+            cls._by_github_username[person.github_username] = person
+            # Populate human_readable based on attribute value if not set explicitly.
+            if not person.human_readable:
+                person.human_readable = attribute_name.title().replace("_", " ")
+
+        super().__init__(name, bases, namespace)
+
+    def __iter__(cls):
+        # Enable iteration by `for person in People` or similar to work.
+        return iter(
+            sorted(value for value in vars(cls).values() if isinstance(value, Person))
+        )
+
+
+class People(metaclass=PersonCollection):
     """Tech team members' GitHub and Slack usernames."""
 
-    # Find a Slack user's username by right-clicking on their name in the Slack app and clicking "Copy link".
+    # Find a Slack user's username by right-clicking on their name in the Slack
+    # app and clicking "Copy link".
 
+    # human_readable is mostly set by PersonCollection.__init__() but can be
+    # overriden as the third argument if required. This avoids the need to
+    # explicitly specify it for most people.
     ALICE = Person("alarthast", "U07KX6L3CMA")
     BECKY = Person("rebkwok", "U01SP5JLBFD")
-    BEN_BC = Person("benbc", "U01SPCP06Q1")
+    BEN_BC = Person("benbc", "U01SPCP06Q1", "Ben BC")
     CATHERINE = Person("CLStables", "U036A6LTR7D")
     DAVE = Person("evansd", "UAXE5V4RG")
     ELI = Person("eli-miriam", "U07LHEJ9TS4")
@@ -32,11 +70,20 @@ class People(Enum):
     TOM_P = Person("remlapmot", "U07L0L0SS6M")
     TOM_W = Person("madwort", "U019R5FJ7G8")
 
+    @classmethod
+    def by_github_username(cls, github_username):
+        default = Person(
+            human_readable=github_username,
+            github_username=github_username,
+            slack_username=github_username,
+        )
+        return cls._by_github_username.get(github_username, default)
 
-PEOPLE_BY_GITHUB_USERNAME = {
-    person.value.github_username: person.value for person in People
-}
 
+# Note that adding to or re-ordering this list will will affect the
+# DependabotRotaReporter ordering algorithm, restarting it at an arbitrary
+# point. If you need to change this list, consider redesigning that class to
+# include an offset.
 TEAM_REX = [
     People.JON,
     People.KATIE,
@@ -45,12 +92,3 @@ class People(Enum):
     People.MIKE,
     People.THOMAS,
 ]
-
-
-def get_person_from_github_username(github_username) -> Person:
-    default = Person(github_username=github_username, slack_username=github_username)
-    return PEOPLE_BY_GITHUB_USERNAME.get(github_username, default)
-
-
-def get_formatted_slack_username(person: Person) -> str:
-    return f"<@{person.slack_username}>"


Test patch:
diff --git a/tests/workspace/test_people.py b/tests/workspace/test_people.py
index db1b11f6..748591c9 100644
--- a/tests/workspace/test_people.py
+++ b/tests/workspace/test_people.py
@@ -1,24 +1,25 @@
-from workspace.utils.people import (
-    People,
-    get_formatted_slack_username,
-    get_person_from_github_username,
-)
+from workspace.utils.people import People
 
 
 def test_get_person_from_github_username():
-    result = get_person_from_github_username("lucyb")
+    result = People.by_github_username("lucyb")
 
-    assert result == People.LUCY.value
+    assert result == People.LUCY
 
 
 def test_get_person_from_github_username_returns_default():
-    result = get_person_from_github_username("TestUser")
+    result = People.by_github_username("TestUser")
 
+    assert result.human_readable == "TestUser"
     assert result.github_username == "TestUser"
     assert result.slack_username == "TestUser"
 
 
-def test_get_formatted_slack_username_returns_slack_user_id():
-    result = get_formatted_slack_username(People.LUCY.value)
+def test_formatted_slack_username():
+    result = People.LUCY.formatted_slack_username
 
     assert result == "<@U035FT48KEK>"
+
+
+def test_iteration():
+    assert isinstance([person for person in People], list)
diff --git a/tests/workspace/test_standup_rota.py b/tests/workspace/test_standup_rota.py
index e28d1b5c..61ffb627 100644
--- a/tests/workspace/test_standup_rota.py
+++ b/tests/workspace/test_standup_rota.py
@@ -58,7 +58,7 @@ def test_daily_rota_odd_week(freezer):
         },
         {
             "text": {
-                "text": "Monday: Jon (backup: Mike)",
+                "text": "Monday: <@U023ZG5H24R> (backup: Mike)",
                 "type": "mrkdwn",
             },
             "type": "section",
@@ -77,7 +77,7 @@ def test_daily_rota_even_week(freezer):
         },
         {
             "text": {
-                "text": "Wednesday: Mary (backup: Steve)",
+                "text": "Wednesday: <@U07LKQ06Q8L> (backup: Steve)",
                 "type": "mrkdwn",
             },
             "type": "section",