Sync guard

apps/workflow/api/xero/reprocess_xero.py

@@ -47,6 +47,12 @@ def sync_xero_phone_methods(company: Company) -> list[str]:
     Returns the normalized numbers that were newly created, so the caller can
     dispatch a call rematch for them.
     """
+    # A merged company's numbers live on the company it was merged into. Its
+    # archived Xero contact keeps the phones (and is still fetched with
+    # include_archived=True), so syncing it would collide with the winner's
+    # rows under the one-number-one-company rule.
+    if company.merged_into_id:
+        return []
     phones = company.raw_json.get("_phones", []) if company.raw_json else []
     if not isinstance(phones, list):
         return []

apps/workflow/tests/test_sync_clients.py

@@ -345,6 +345,28 @@ def test_duplicate_phone_owner_crashes_sync_and_persists_app_error(self) -> None
         self.assertIn("+6421555123", app_error.message)
         self.assertIn("Existing Phone Owner", app_error.message)
 
+    def test_merged_company_phones_are_not_synced(self) -> None:
+        """A merged company's archived Xero contact keeps its phones, but they
+        now belong to the winner — syncing must skip, not collide."""
+        winner = Company.objects.create(
+            name="Winner", xero_last_modified=timezone.now()
+        )
+        ContactMethod.objects.create(
+            company=winner,
+            method_type=ContactMethod.MethodType.PHONE,
+            value="021 555 123",
+        )
+        merged = self._client_with_phone("Merged Loser")
+        merged.merged_into = winner
+        merged.save()
+        before = AppError.objects.count()
+
+        created = sync_xero_phone_methods(merged)  # must not raise
+
+        self.assertEqual(created, [])
+        self.assertEqual(ContactMethod.objects.filter(company=merged).count(), 0)
+        self.assertEqual(AppError.objects.count(), before)
+
     def test_resync_of_existing_number_is_grandfathered(self) -> None:
         """Re-syncing a company's own already-stored number must not raise."""
         owner = self._client_with_phone("Phone Owner")

frontend/src/components/job/JobFinishTab.vue

@@ -32,9 +32,7 @@
       class="bg-white rounded-lg border border-red-200 p-6 text-center"
     >
       <p class="text-red-700 font-medium">Could not load this job's financials.</p>
-      <p class="text-sm text-slate-600 mt-1">
-        Reload the page. Do not quote the customer a figure until it loads.
-      </p>
+      <p class="text-sm text-slate-600 mt-1">Reload the page.</p>
     </div>
 
     <template v-else-if="summary">
@@ -98,8 +96,11 @@
             data-automation-id="JobFinishTab-over-invoiced"
             class="mt-3 rounded-md bg-amber-50 border border-amber-200 p-3 text-sm text-amber-900"
           >
-            <strong>Over-invoiced by {{ formatCurrency(summary.over_invoiced_excl_gst) }}</strong>
-            (excl GST). Resolve it with a credit note in Xero.
+            <strong
+              >Job appears over-invoiced by
+              {{ formatCurrency(summary.over_invoiced_excl_gst) }}</strong
+            >
+            (excl GST). The job value may be out of date, or invoicing may need correcting.
           </div>
         </section>
 
@@ -118,7 +119,7 @@
         class="bg-white rounded-xl border border-slate-200 p-4"
       >
         <h3 class="text-base font-semibold text-gray-900">Completion checklist</h3>
-        <p class="text-xs text-slate-500 mb-3">Recorded against the job. Never blocks invoicing.</p>
+        <p class="text-xs text-slate-500 mb-3">Self-checklist.</p>
 
         <div class="space-y-2">
           <label
@@ -137,15 +138,6 @@
             <span class="text-slate-800">{{ item.label }}</span>
           </label>
         </div>
-
-        <p
-          v-if="props.pricingMethodology === 'time_materials'"
-          data-automation-id="JobFinishTab-tm-urgency"
-          class="mt-3 text-sm text-slate-600"
-        >
-          On a Time &amp; Materials job the time and materials are what the customer pays. Get them
-          right before invoicing.
-        </p>
       </section>
     </template>
 
@@ -375,14 +367,21 @@ const checklist = reactive<JobCompletionChecklist>({
 })
 const savingChecklistKey = ref<ChecklistItemKey | null>(null)
 
-const checklistItems: Array<{ key: ChecklistItemKey; label: string }> = [
-  { key: 'foreman_signed_off', label: 'Has the foreman signed off the job?' },
-  { key: 'timesheets_collected', label: 'Have you collected the timesheet entries?' },
-  { key: 'materials_checked', label: 'Have you checked the materials on the job?' },
+const allChecklistItems: Array<{ key: ChecklistItemKey; label: string }> = [
+  { key: 'foreman_signed_off', label: 'Has the supervisor signed off the job?' },
+  { key: 'timesheets_collected', label: "Have you collected today's timesheet entries?" },
+  { key: 'materials_checked', label: 'Are all materials used written correctly on the job sheet?' },
   { key: 'customer_called', label: 'Have you called the customer?' },
-  { key: 'released', label: 'Has the job been released?' },
+  { key: 'released', label: 'Has the job been handed over?' },
 ]
 
+// Timesheets only exist to invoice on a T&M job; a quoted job is never asked.
+const checklistItems = computed(() =>
+  props.pricingMethodology === 'time_materials'
+    ? allChecklistItems
+    : allChecklistItems.filter((item) => item.key !== 'timesheets_collected'),
+)
+
 const jobValueLabel = computed(() =>
   props.pricingMethodology === 'fixed_price'
     ? 'Quote total (excl GST)'

frontend/src/components/job/__tests__/JobFinishTab.checklist.test.ts

@@ -60,33 +60,23 @@ describe('JobFinishTab completion checklist', () => {
 
   afterEach(resetFinishTab)
 
-  it('asks the same five questions whatever the pricing methodology', async () => {
-    for (const methodology of ['fixed_price', 'time_materials']) {
-      const wrapper = mountTab(methodology)
-      await flushPromises()
-
-      for (const key of ITEMS) {
-        expect(item(wrapper, key).exists(), `${methodology}/${key}`).toBe(true)
-      }
-
-      resetFinishTab()
-    }
-  })
-
-  it('warns that time and materials are what a T&M customer pays', async () => {
+  it('asks all five questions on a T&M job', async () => {
     const wrapper = mountTab('time_materials')
     await flushPromises()
 
-    expect(wrapper.find('[data-automation-id="JobFinishTab-tm-urgency"]').text()).toContain(
-      'before invoicing',
-    )
+    for (const key of ITEMS) {
+      expect(item(wrapper, key).exists(), key).toBe(true)
+    }
   })
 
-  it('does not show the T&M urgency note on a quoted job', async () => {
+  it('does not ask about timesheets on a quoted job', async () => {
     const wrapper = mountTab('fixed_price')
     await flushPromises()
 
-    expect(wrapper.find('[data-automation-id="JobFinishTab-tm-urgency"]').exists()).toBe(false)
+    expect(item(wrapper, 'timesheets_collected').exists()).toBe(false)
+    for (const key of ITEMS.filter((k) => k !== 'timesheets_collected')) {
+      expect(item(wrapper, key).exists(), key).toBe(true)
+    }
   })
 
   it('reflects ticks already recorded on the job', async () => {