diff --git a/apps/workflow/api/xero/reprocess_xero.py b/apps/workflow/api/xero/reprocess_xero.py index 7e4701f3e..2d9d8f472 100644 --- a/apps/workflow/api/xero/reprocess_xero.py +++ b/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 [] diff --git a/apps/workflow/tests/test_sync_clients.py b/apps/workflow/tests/test_sync_clients.py index fba52e6ab..6cbfdc218 100644 --- a/apps/workflow/tests/test_sync_clients.py +++ b/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") diff --git a/frontend/src/components/job/JobFinishTab.vue b/frontend/src/components/job/JobFinishTab.vue index 6dcd10913..aef5e9b25 100644 --- a/frontend/src/components/job/JobFinishTab.vue +++ b/frontend/src/components/job/JobFinishTab.vue @@ -32,9 +32,7 @@ class="bg-white rounded-lg border border-red-200 p-6 text-center" >

Could not load this job's financials.

-

- Reload the page. Do not quote the customer a figure until it loads. -

+

Reload the page.

@@ -375,14 +367,21 @@ const checklist = reactive({ }) const savingChecklistKey = ref(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)' diff --git a/frontend/src/components/job/__tests__/JobFinishTab.checklist.test.ts b/frontend/src/components/job/__tests__/JobFinishTab.checklist.test.ts index c91bb1450..499392bac 100644 --- a/frontend/src/components/job/__tests__/JobFinishTab.checklist.test.ts +++ b/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 () => {