Phone-number selection policy lives in the domain layer 🟪

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

domain/abri/phone_selection.py

@@ -0,0 +1,64 @@
+from dataclasses import dataclass
+from typing import List, Literal, Optional, Sequence, Tuple
+
+PhoneKind = Literal["mobile", "telephone"]
+
+
+@dataclass(frozen=True)
+class PhoneEntry:
+    """One phone number as Abri lists it for a tenant, in document order."""
+
+    kind: PhoneKind
+    number: str
+    priority: Optional[str] = None
+
+
+@dataclass(frozen=True)
+class SelectedPhoneNumbers:
+    """The numbers a contact carries: best of each kind plus the runner-up."""
+
+    mobile: Optional[str]
+    telephone: Optional[str]
+    secondary: Optional[str]
+
+
+# TODO(#1474): confirm the priority ordering with Abri — this assumes the
+# lowest numeric priority marks the preferred number, and that entries with
+# a missing or non-numeric priority rank last (ties keep document order).
+def _priority_sort_key(entry: PhoneEntry) -> Tuple[int, int]:
+    """Sort key: lowest numeric priority first; unusable priorities last."""
+    try:
+        return (0, int((entry.priority or "").strip()))
+    except ValueError:
+        return (1, 0)
+
+
+def _best(entries: List[PhoneEntry]) -> Optional[PhoneEntry]:
+    ranked = sorted(entries, key=_priority_sort_key)  # stable: ties keep document order
+    return ranked[0] if ranked else None
+
+
+def _number(entry: Optional[PhoneEntry]) -> Optional[str]:
+    return entry.number.strip() if entry is not None else None
+
+
+def select_phone_numbers(entries: Sequence[PhoneEntry]) -> SelectedPhoneNumbers:
+    """Best mobile, best landline, then the best of the leftovers of both kinds.
+
+    All three picks use the same rule: lowest numeric priority wins, entries
+    without a usable priority sort last, ties break by document order, blank
+    numbers are ignored.
+    """
+    usable = [entry for entry in entries if entry.number.strip()]
+
+    best_mobile = _best([e for e in usable if e.kind == "mobile"])
+    best_telephone = _best([e for e in usable if e.kind == "telephone"])
+    runner_up = _best(
+        [e for e in usable if e is not best_mobile and e is not best_telephone]
+    )
+
+    return SelectedPhoneNumbers(
+        mobile=_number(best_mobile),
+        telephone=_number(best_telephone),
+        secondary=_number(runner_up),
+    )

infrastructure/abri/abri_client.py

@@ -1,7 +1,6 @@
 import xml.etree.ElementTree as ET
-from dataclasses import dataclass
 from datetime import date
-from typing import List, Optional, Tuple, Union
+from typing import Dict, List, Tuple, Union
 
 import requests
 
@@ -18,6 +17,7 @@ from domain.abri.models import (
     TenancyData,
     Tenant,
 )
+from domain.abri.phone_selection import PhoneEntry, PhoneKind, select_phone_numbers
 from infrastructure.abri.config import AbriConfig
 from infrastructure.abri.envelope import serialise_relay_request
 from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError
@@ -31,54 +31,21 @@ def _format_appointment_date(appointment_date: date) -> str:
     return appointment_date.strftime("%d/%m/%Y")
 
 
-def _phone_priority(entry: ET.Element) -> Tuple[int, int]:
-    """Sort key: lowest numeric priority first; unusable priorities last."""
-    try:
-        return (0, int((entry.get("priority") or "").strip()))
-    except ValueError:
-        return (1, 0)
+# Wire tags -> the domain's phone kinds; selection policy lives in the domain.
+_PHONE_KINDS: Dict[str, PhoneKind] = {"Mobile": "mobile", "Telephone": "telephone"}
 
 
-def _best_phone_number(entries: List[ET.Element]) -> Optional[ET.Element]:
-    ranked = sorted(entries, key=_phone_priority)  # stable: ties keep document order
-    return ranked[0] if ranked else None
-
-
-@dataclass(frozen=True)
-class _SelectedPhoneNumbers:
-    mobile: Optional[str]
-    telephone: Optional[str]
-    secondary: Optional[str]
-
-
-def _select_phone_numbers(tenant_element: ET.Element) -> _SelectedPhoneNumbers:
-    """Best mobile, best landline, then the best of the leftovers of both kinds.
-
-    All three picks use the same rule: lowest numeric priority wins, entries
-    without a usable priority sort last, ties break by document order, blank
-    numbers are ignored.
-    """
-    usable = [
-        entry
-        for entry in tenant_element
-        if entry.tag in ("Mobile", "Telephone") and (entry.text or "").strip()
+def _phone_entries(tenant_element: ET.Element) -> List[PhoneEntry]:
+    return [
+        PhoneEntry(
+            kind=_PHONE_KINDS[child.tag],
+            number=child.text or "",
+            priority=child.get("priority"),
+        )
+        for child in tenant_element
+        if child.tag in _PHONE_KINDS
     ]
 
-    best_mobile = _best_phone_number([e for e in usable if e.tag == "Mobile"])
-    best_telephone = _best_phone_number([e for e in usable if e.tag == "Telephone"])
-    runner_up = _best_phone_number(
-        [e for e in usable if e is not best_mobile and e is not best_telephone]
-    )
-
-    def _text(entry: Optional[ET.Element]) -> Optional[str]:
-        return (entry.text or "").strip() if entry is not None else None
-
-    return _SelectedPhoneNumbers(
-        mobile=_text(best_mobile),
-        telephone=_text(best_telephone),
-        secondary=_text(runner_up),
-    )
-
 
 class AbriClient:
     def __init__(self, config: AbriConfig) -> None:
@@ -251,7 +218,7 @@ class AbriClient:
     def _parse_tenant(element: ET.Element) -> Tenant:
         # Data minimisation: person_title, main-contact and the per-person
         # reference (the element text) are deliberately not read.
-        numbers = _select_phone_numbers(element)
+        numbers = select_phone_numbers(_phone_entries(element))
         return Tenant(
             forenames=(element.get("forenames") or "").strip(),
             surname=(element.get("surname") or "").strip(),

tests/domain/abri/test_phone_selection.py

@@ -0,0 +1,140 @@
+from typing import Optional
+
+from domain.abri.phone_selection import (
+    PhoneEntry,
+    SelectedPhoneNumbers,
+    select_phone_numbers,
+)
+
+
+def _mobile(number: str, priority: Optional[str] = None) -> PhoneEntry:
+    return PhoneEntry(kind="mobile", number=number, priority=priority)
+
+
+def _telephone(number: str, priority: Optional[str] = None) -> PhoneEntry:
+    return PhoneEntry(kind="telephone", number=number, priority=priority)
+
+
+def test_no_entries_selects_nothing() -> None:
+    assert select_phone_numbers([]) == SelectedPhoneNumbers(
+        mobile=None, telephone=None, secondary=None
+    )
+
+
+def test_lowest_numeric_priority_wins_within_a_kind() -> None:
+    # Arrange
+    entries = [
+        _mobile("07700900020", priority="20"),
+        _mobile("07700900005", priority="5"),
+        _mobile("07700900010", priority="10"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected.mobile == "07700900005"
+
+
+def test_entries_without_a_usable_priority_sort_last() -> None:
+    # Arrange
+    entries = [
+        _mobile("07700900001"),  # missing priority
+        _mobile("07700900002", priority="abc"),  # non-numeric priority
+        _mobile("07700900003", priority="99"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected.mobile == "07700900003"
+
+
+def test_priority_ties_break_by_document_order() -> None:
+    # Arrange
+    entries = [
+        _telephone("01632960001", priority="7"),
+        _telephone("01632960002", priority="7"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected.telephone == "01632960001"
+
+
+def test_blank_numbers_are_ignored_even_at_best_priority() -> None:
+    # Arrange
+    entries = [
+        _mobile("   ", priority="1"),
+        _mobile("07700900002", priority="2"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected == SelectedPhoneNumbers(
+        mobile="07700900002", telephone=None, secondary=None
+    )
+
+
+def test_secondary_is_the_best_of_the_pooled_leftovers_of_both_kinds() -> None:
+    # Arrange
+    entries = [
+        _mobile("07700900005", priority="5"),  # best mobile
+        _mobile("07700900020", priority="20"),  # leftover
+        _telephone("01632960003", priority="3"),  # best landline
+        _telephone("01632960007", priority="7"),  # best leftover: wins secondary
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected == SelectedPhoneNumbers(
+        mobile="07700900005", telephone="01632960003", secondary="01632960007"
+    )
+
+
+def test_no_secondary_when_the_best_picks_use_every_number() -> None:
+    # Arrange
+    entries = [
+        _mobile("07700900005", priority="5"),
+        _telephone("01632960003", priority="3"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected.secondary is None
+
+
+def test_a_single_kind_still_yields_a_secondary_from_its_own_leftovers() -> None:
+    # Arrange
+    entries = [
+        _telephone("01632960001", priority="1"),
+        _telephone("01632960002", priority="2"),
+    ]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected == SelectedPhoneNumbers(
+        mobile=None, telephone="01632960001", secondary="01632960002"
+    )
+
+
+def test_selected_numbers_are_stripped_of_surrounding_whitespace() -> None:
+    # Arrange
+    entries = [_mobile(" 07700900123 ", priority="1")]
+
+    # Act
+    selected = select_phone_numbers(entries)
+
+    # Assert
+    assert selected.mobile == "07700900123"