KAN-281: Restore phone-number display on the job-settings tab (main)

.env.example

@@ -43,6 +43,8 @@ DJANGO_SITE_DOMAIN=your-domain.ngrok-free.app
 
 # Xero Integration
 XERO_DEFAULT_USER_ID=YOUR_XERO_USER_ID_FOR_TIME_ENTRIES
+# Suppress all Xero writes for this process (E2E/test backends only; reads stay live)
+XERO_READONLY=False
 XERO_SYNC_PROJECTS=False
 
 # JWT Configuration

.env.precommit

@@ -16,8 +16,8 @@ REDIS_HOST=localhost
 REDIS_PORT=6379
 XERO_CLIENT_ID=dummy
 XERO_CLIENT_SECRET=dummy
-XERO_REDIRECT_URI=http://localhost
 XERO_DEFAULT_USER_ID=dummy
+XERO_REDIRECT_URI=http://localhost
 EMAIL_HOST=localhost
 EMAIL_PORT=587
 EMAIL_USE_TLS=false
@@ -33,6 +33,7 @@ SKIP_VERSION_CHECK=True
 ALLOWED_HOSTS=localhost,127.0.0.1
 DJANGO_SITE_DOMAIN=localhost
 APP_DOMAIN=localhost
+XERO_READONLY=False
 XERO_SYNC_PROJECTS=False
 XERO_WEBHOOK_KEY=dummy-webhook-key
 CORS_ALLOWED_ORIGINS=http://localhost:3000

.vscode/tasks.json

@@ -120,7 +120,7 @@
       }
     },
     {
-      "label": "Start Dev Environment",
+      "label": "Start Hotfix Environment",
       "dependsOn": [
         "Frontend Dev Server",
         "Frontend Manual Dev Server",

CLAUDE.md

@@ -168,7 +168,7 @@ except Exception as exc:
 
 ## Environment Configuration
 
-See `.env.example` for required environment variables. Key integrations: Xero API, Dropbox, PostgreSQL. Frontend tooling reads `APP_DOMAIN` from the backend `.env` at `../.env` and derives URLs from it (see ADR 0008's Consequences). Deploy uses `scripts/server/deploy.sh` (per-instance `<client>-<env>`); it also runs on boot via systemd so a cold machine catches up to `main`.
+See `.env.example` for required environment variables. Key integrations: Xero API, Dropbox, PostgreSQL. Frontend tooling reads `APP_DOMAIN` from the backend `.env` at `../.env` and derives URLs from it (see ADR 0008's Consequences). Deploy uses `scripts/server/deploy.sh` (per-instance `<client>-<env>`); it also runs on boot via systemd so a cold machine catches up to `production`. Servers only ever run the `production` branch — `main` is the integration branch and is never deployed (ADR 0029).
 
 ## Migration Management
 
@@ -189,6 +189,8 @@ See ADR 0020. Backend owns data, calculations, and external systems; frontend ow
 
 `npm run test:e2e` (from `frontend/`) runs Playwright tests against live HTTP endpoints. The suite takes ~20–25 min.
 
+**CRITICAL: The backend serving `APP_DOMAIN` must run with `XERO_READONLY=True`.** The flag swaps in a provider that suppresses all Xero writes (contacts, invoices, quotes, attachments, history notes) while reads and token refresh stay live; without it the suite writes real entities into the connected Xero tenant. Global-setup enforces this via `/api/xero/ping/` (`xero_readonly` field) and aborts otherwise. The flag is process-scoped: any celery worker/beat sharing the DB must also run with `XERO_READONLY=True`, or the hourly `xero_regular_sync_task` will push local `[TEST]` stock to Xero — global-setup cannot verify a worker's environment.
+
 **CRITICAL: The global teardown (`global-teardown.ts`) MUST always run to completion.** It restores the database from backup, saves/reinjects Xero tokens, removes the lock file, and runs integrity checks. If the bash process is killed (timeout, SIGTERM, etc.) the teardown never executes and the database is left polluted with `[TEST]` data.
 
 - **Never set a bash timeout on the E2E command.** A timeout (or SIGTERM) kills the node process before Playwright calls `globalTeardown`, leaving the DB polluted with `[TEST]` data and a stale lock file. The teardown is NOT a signal handler — it only fires on normal exit.

apps/accounting/tests/test_payroll_reconciliation_service.py

@@ -0,0 +1,75 @@
+"""Unit tests for payroll reconciliation date-range alignment.
+
+``get_aligned_date_range`` is the oracle behind the report's date inputs:
+it snaps arbitrary dates to Monday-start/Sunday-end pay weeks, and clamps
+the start to ``CompanyDefaults.xero_payroll_start_date`` so the report
+never covers weeks before Xero payroll history exists. The E2E suite
+deliberately does not re-derive this logic (it varies with per-instance
+data); these tests own it with controlled CompanyDefaults.
+"""
+
+from datetime import date
+
+from apps.accounting.services import PayrollReconciliationService
+from apps.testing import BaseTestCase
+from apps.workflow.models import CompanyDefaults
+
+
+class GetAlignedDateRangeTests(BaseTestCase):
+    def _set_payroll_start(self, payroll_start: date | None) -> None:
+        defaults = CompanyDefaults.get_solo()
+        CompanyDefaults.objects.filter(pk=defaults.pk).update(
+            xero_payroll_start_date=payroll_start
+        )
+
+    def _aligned(self, start: date, end: date) -> tuple[date, date]:
+        result = PayrollReconciliationService.get_aligned_date_range(start, end)
+        return result["aligned_start"], result["aligned_end"]
+
+    def test_midweek_dates_snap_to_monday_and_sunday(self) -> None:
+        self._set_payroll_start(None)
+        # Tuesday 2025-04-01 -> Monday 2025-03-31; Tuesday 2026-03-31 -> Sunday 2026-04-05
+        aligned_start, aligned_end = self._aligned(date(2025, 4, 1), date(2026, 3, 31))
+        self.assertEqual(aligned_start, date(2025, 3, 31))
+        self.assertEqual(aligned_end, date(2026, 4, 5))
+
+    def test_already_aligned_dates_are_unchanged(self) -> None:
+        self._set_payroll_start(None)
+        # Monday 2025-03-31 and Sunday 2026-04-05 are already week boundaries
+        aligned_start, aligned_end = self._aligned(date(2025, 3, 31), date(2026, 4, 5))
+        self.assertEqual(aligned_start, date(2025, 3, 31))
+        self.assertEqual(aligned_end, date(2026, 4, 5))
+
+    def test_single_day_expands_to_its_full_week(self) -> None:
+        self._set_payroll_start(None)
+        # Thursday 2025-04-03 -> the whole Mon-Sun week containing it
+        aligned_start, aligned_end = self._aligned(date(2025, 4, 3), date(2025, 4, 3))
+        self.assertEqual(aligned_start, date(2025, 3, 31))
+        self.assertEqual(aligned_end, date(2025, 4, 6))
+
+    def test_start_clamps_to_payroll_start_before_snapping(self) -> None:
+        # Friday 2025-08-01: requests starting earlier clamp to it, then
+        # snap to the Monday of its week.
+        self._set_payroll_start(date(2025, 8, 1))
+        aligned_start, aligned_end = self._aligned(date(2025, 4, 1), date(2026, 3, 31))
+        self.assertEqual(aligned_start, date(2025, 7, 28))
+        self.assertEqual(aligned_end, date(2026, 4, 5))
+
+    def test_start_after_payroll_start_is_not_clamped(self) -> None:
+        self._set_payroll_start(date(2025, 8, 1))
+        # Tuesday 2025-09-02 is after the payroll start -> normal Monday snap
+        aligned_start, _ = self._aligned(date(2025, 9, 2), date(2026, 3, 31))
+        self.assertEqual(aligned_start, date(2025, 9, 1))
+
+    def test_payroll_start_on_a_monday_clamps_exactly_to_it(self) -> None:
+        self._set_payroll_start(date(2025, 8, 4))  # a Monday
+        aligned_start, _ = self._aligned(date(2025, 4, 1), date(2026, 3, 31))
+        self.assertEqual(aligned_start, date(2025, 8, 4))
+
+    def test_end_date_is_never_clamped_by_payroll_start(self) -> None:
+        # The clamp applies to the start only; an end before payroll start
+        # still snaps to its own week's Sunday (yielding an empty range,
+        # which the report handles, rather than a silently rewritten end).
+        self._set_payroll_start(date(2025, 8, 1))
+        _, aligned_end = self._aligned(date(2025, 4, 1), date(2025, 5, 1))
+        self.assertEqual(aligned_end, date(2025, 5, 4))

apps/client/__init__.py

@@ -35,12 +35,14 @@
             ClientSerializer,
             ClientUpdateResponseSerializer,
             ClientUpdateSerializer,
+            JobContactBaseSerializer,
             JobContactResponseSerializer,
             JobContactUpdateSerializer,
             StandardErrorSerializer,
             SupplierPickupAddressSerializer,
             SupplierSearchAliasCreateSerializer,
             SupplierSearchAliasSerializer,
+            set_primary_phone,
         )
 except (ImportError, RuntimeError):
     # Django not ready or circular import, skip conditional imports
@@ -68,6 +70,7 @@
     "ClientSerializer",
     "ClientUpdateResponseSerializer",
     "ClientUpdateSerializer",
+    "JobContactBaseSerializer",
     "JobContactResponseSerializer",
     "JobContactUpdateSerializer",
     "PhoneAssignmentConflictError",
@@ -79,4 +82,5 @@
     "SupplierSearchAliasCreateSerializer",
     "SupplierSearchAliasSerializer",
     "date_to_datetime",
+    "set_primary_phone",
 ]

apps/client/models.py

@@ -3,6 +3,7 @@
 import uuid
 from collections.abc import Iterable
 from decimal import Decimal
+from typing import Final, Literal
 
 from django.contrib.postgres.indexes import GinIndex
 from django.contrib.postgres.search import SearchVector
@@ -16,20 +17,6 @@
 logger = logging.getLogger(__name__)
 
 
-def _include_auto_now_update_field(kwargs, field_name: str) -> None:
-    update_fields = kwargs.get("update_fields")
-    if update_fields is None:
-        return
-
-    update_field_names = (
-        [update_fields] if isinstance(update_fields, str) else list(update_fields)
-    )
-    if not update_field_names or field_name in update_field_names:
-        return
-
-    kwargs["update_fields"] = [*update_field_names, field_name]
-
-
 def _augment_update_fields(
     update_fields: Iterable[str] | None, field_name: str
 ) -> list[str] | None:
@@ -172,11 +159,23 @@ class Meta:
     def __str__(self):
         return self.name
 
-    def save(self, *args, **kwargs):
-        _include_auto_now_update_field(kwargs, "django_updated_at")
-        super().save(*args, **kwargs)
+    def save(
+        self,
+        *,
+        force_insert: bool | tuple[ModelBase, ...] = False,
+        force_update: bool = False,
+        using: str | None = None,
+        update_fields: Iterable[str] | None = None,
+    ) -> None:
+        update_fields = _augment_update_fields(update_fields, "django_updated_at")
+        super().save(
+            force_insert=force_insert,
+            force_update=force_update,
+            using=using,
+            update_fields=update_fields,
+        )
 
-    def validate_for_xero(self):
+    def validate_for_xero(self) -> bool:
         """
         Validate if the client data is sufficient to sync to Xero.
         Only name is required by Xero.
@@ -249,19 +248,19 @@ def get_client_for_xero(self):
     def primary_phone_value(self) -> str:
         """The client's own primary phone number, or "" when it has none.
 
-        Uses the same ordering as every other primary-phone consumer:
-        primary first, then label/value as a stable tie-break.
+        Single-object flows only (Xero sync, PO PDFs). Queryset consumers must
+        use ClientContactMethod.primary_phone_annotation instead.
         """
         method = (
             self.contact_methods.filter(
                 method_type=ClientContactMethod.MethodType.PHONE
             )
-            .order_by("-is_primary", "label", "value")
+            .order_by(*PRIMARY_PHONE_ORDERING)
             .first()
         )
         return method.value if method else ""
 
-    def get_final_client(self):
+    def get_final_client(self) -> "Client":
         """
         Follow the merge chain to get the final client.
         If this client was merged into another, return that client
@@ -403,6 +402,13 @@ def __init__(self, conflict: "ClientContactMethod | None") -> None:
         super().__init__(message)
 
 
+# The primary-phone ordering rule: primary first, then label/value as a
+# stable tie-break. Single source — every primary-phone consumer must use it.
+PRIMARY_PHONE_ORDERING: Final[tuple[str, str, str]] = ("-is_primary", "label", "value")
+
+PhoneOwner = Literal["client", "contact"]
+
+
 class ClientContactMethod(models.Model):
     """Phone or email address owned by a client or one of its contacts."""
 
@@ -488,6 +494,26 @@ def __str__(self) -> str:
         owner = self.contact or self.client
         return f"{self.method_type}: {self.value} ({owner})"
 
+    @classmethod
+    def primary_phone_annotation(cls, *, owner: PhoneOwner, outer_ref: str) -> Coalesce:
+        """Queryset annotation: the owner's primary phone value, "" when it has none.
+
+        ``outer_ref`` names the outer queryset's column holding the owner's id
+        (e.g. "pk" on a Client queryset, "client_id" on a Job queryset).
+        """
+        candidates = (
+            cls.objects.filter(
+                method_type=cls.MethodType.PHONE, **{owner: models.OuterRef(outer_ref)}
+            )
+            .order_by(*PRIMARY_PHONE_ORDERING)
+            .values("value")[:1]
+        )
+        return Coalesce(
+            models.Subquery(candidates),
+            models.Value(""),
+            output_field=models.CharField(),
+        )
+
     def save(
         self,
         *,