diff --git a/domain/abri/phone_selection.py b/domain/abri/phone_selection.py new file mode 100644 index 000000000..fe3588d5f --- /dev/null +++ b/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), + ) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index a07da387f..80f9cde77 100644 --- a/infrastructure/abri/abri_client.py +++ b/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(), diff --git a/tests/domain/abri/test_phone_selection.py b/tests/domain/abri/test_phone_selection.py new file mode 100644 index 000000000..70e2ec5a3 --- /dev/null +++ b/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"