Răsfoiți Sursa

Stop a status report claiming a channel that never sent (#259)

sendSmsToCustomer and sendTelegramToCustomer are withAuth actions, so a
refusal comes back as a returned value rather than a thrown error. This
function awaited all three senders and read none of their results, so a
message the provider rejected still counted: the row said sent, sentVia
named the channel, and the customer heard nothing. No error surfaced
anywhere, which is why it could sit there indefinitely.

Each channel is now recorded only if it reported success, and a report with
nothing to show for it fails outright rather than being filed as sent. A
ticked channel with no address on file counts as a failure too, because
doing nothing quietly is the same lie in a softer voice.

The dialog had the matching half of the bug: its success toast listed the
boxes that were ticked, not the channels that carried the message. It now
reports what came back, and warns separately when part of a send failed.
Bernt Christian Egeland 1 lună în urmă
părinte
comite
eb92dde7fd

+ 2 - 1
messages/de/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Überspringen",
     "send": "Senden",
     "sent": "Statusbericht gesendet",
-    "failed": "Statusbericht konnte nicht gesendet werden"
+    "failed": "Statusbericht konnte nicht gesendet werden",
+    "notSent": "Nicht gesendet"
   },
   "view": {
     "title": "Fahrzeug-Statusbericht",

+ 2 - 1
messages/en/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Skip",
     "send": "Send",
     "sent": "Status report sent",
-    "failed": "Failed to send status report"
+    "failed": "Failed to send status report",
+    "notSent": "Not sent"
   },
   "view": {
     "title": "Vehicle Status Report",

+ 2 - 1
messages/es/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Omitir",
     "send": "Enviar",
     "sent": "Informe de estado enviado",
-    "failed": "Error al enviar el informe de estado"
+    "failed": "Error al enviar el informe de estado",
+    "notSent": "No enviado"
   },
   "view": {
     "title": "Informe de estado del vehículo",

+ 2 - 1
messages/fr/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Ignorer",
     "send": "Envoyer",
     "sent": "Rapport d'état envoyé",
-    "failed": "Échec de l'envoi du rapport d'état"
+    "failed": "Échec de l'envoi du rapport d'état",
+    "notSent": "Non envoyé"
   },
   "view": {
     "title": "Rapport d'état du véhicule",

+ 2 - 1
messages/it/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Salta",
     "send": "Invia",
     "sent": "Rapporto di stato inviato",
-    "failed": "Invio del rapporto di stato non riuscito"
+    "failed": "Invio del rapporto di stato non riuscito",
+    "notSent": "Non inviato"
   },
   "view": {
     "title": "Rapporto di stato del veicolo",

+ 2 - 1
messages/lt/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Praleisti",
     "send": "Siųsti",
     "sent": "Būsenos ataskaita išsiųsta",
-    "failed": "Nepavyko išsiųsti būsenos ataskaitos"
+    "failed": "Nepavyko išsiųsti būsenos ataskaitos",
+    "notSent": "Neišsiųsta"
   },
   "view": {
     "title": "Transporto priemonės būsenos ataskaita",

+ 2 - 1
messages/nb/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Hopp over",
     "send": "Send",
     "sent": "Statusrapport sendt",
-    "failed": "Kunne ikke sende statusrapport"
+    "failed": "Kunne ikke sende statusrapport",
+    "notSent": "Ikke sendt"
   },
   "view": {
     "title": "Kjøretøyets statusrapport",

+ 2 - 1
messages/nl/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Overslaan",
     "send": "Verzenden",
     "sent": "Statusrapport verzonden",
-    "failed": "Statusrapport verzenden mislukt"
+    "failed": "Statusrapport verzenden mislukt",
+    "notSent": "Niet verzonden"
   },
   "view": {
     "title": "Statusrapport voertuig",

+ 2 - 1
messages/pl/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Pomiń",
     "send": "Wyślij",
     "sent": "Raport statusu wysłany",
-    "failed": "Nie udało się wysłać raportu statusu"
+    "failed": "Nie udało się wysłać raportu statusu",
+    "notSent": "Nie wysłano"
   },
   "view": {
     "title": "Raport statusu pojazdu",

+ 2 - 1
messages/pt-BR/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Pular",
     "send": "Enviar",
     "sent": "Relatório de status enviado",
-    "failed": "Falha ao enviar o relatório de status"
+    "failed": "Falha ao enviar o relatório de status",
+    "notSent": "Não enviado"
   },
   "view": {
     "title": "Relatório de Status do Veículo",

+ 2 - 1
messages/ru/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Пропустить",
     "send": "Отправить",
     "sent": "Отчёт о состоянии отправлен",
-    "failed": "Не удалось отправить отчёт о состоянии"
+    "failed": "Не удалось отправить отчёт о состоянии",
+    "notSent": "Не отправлено"
   },
   "view": {
     "title": "Отчёт о состоянии автомобиля",

+ 2 - 1
messages/tr/statusReport.json

@@ -37,7 +37,8 @@
     "skip": "Atla",
     "send": "Gönder",
     "sent": "Durum raporu gönderildi",
-    "failed": "Durum raporu gönderilemedi"
+    "failed": "Durum raporu gönderilemedi",
+    "notSent": "Gönderilmedi"
   },
   "view": {
     "title": "Araç Durum Raporu",

+ 152 - 0
src/__tests__/features/status-report-send.test.ts

@@ -0,0 +1,152 @@
+/**
+ * A status report must only claim the channels that carried it.
+ *
+ * The three senders are withAuth actions, so a refusal arrives as a returned
+ * `{ success: false }` rather than as a thrown error. The original code
+ * awaited them and ignored the result, so a text message the provider rejected
+ * still counted: the row said sent, sentVia said sms, and the customer never
+ * heard anything. Nothing in the app disagreed with that, which is why it
+ * could sit there indefinitely.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+
+const statusReport = {
+  findFirst: vi.fn(),
+  update: vi.fn().mockResolvedValue({}),
+}
+vi.mock('@/lib/db', () => ({ db: { statusReport } }))
+
+// withAuth is the thing that turns a throw into a returned error, so the test
+// needs the real behaviour rather than a pass-through.
+vi.mock('@/lib/with-auth', () => ({
+  withAuth: async (fn: (ctx: { organizationId: string; userId: string }) => Promise<unknown>) => {
+    try {
+      return { success: true, data: await fn({ organizationId: 'org', userId: 'user' }) }
+    } catch (error) {
+      return { success: false, error: (error as Error).message }
+    }
+  },
+}))
+
+vi.mock('next-intl/server', () => ({
+  getTranslations: async () => (key: string) => key,
+}))
+
+const sendNotificationEmail = vi.fn()
+const sendSmsToCustomer = vi.fn()
+const sendTelegramToCustomer = vi.fn()
+vi.mock('@/features/email/Actions/emailActions', () => ({
+  sendNotificationEmail: (i: unknown) => sendNotificationEmail(i),
+}))
+vi.mock('@/features/sms/Actions/smsActions', () => ({
+  sendSmsToCustomer: (i: unknown) => sendSmsToCustomer(i),
+}))
+vi.mock('@/features/telegram/Actions/telegramActions', () => ({
+  sendTelegramToCustomer: (i: unknown) => sendTelegramToCustomer(i),
+}))
+
+const { sendStatusReport } = await import(
+  '@/features/status-reports/Actions/sendStatusReport'
+)
+
+const CUSTOMER = {
+  id: 'cust',
+  name: 'Ola',
+  email: 'ola@example.com',
+  phone: '+4700000000',
+  telegramChatId: 'chat',
+}
+
+type Customer = {
+  id: string
+  name: string
+  email: string | null
+  phone: string | null
+  telegramChatId: string | null
+}
+
+function report(customer: Customer | null = CUSTOMER) {
+  statusReport.findFirst.mockResolvedValue({
+    id: 'rep',
+    publicToken: 'tok',
+    serviceRecord: {
+      title: 'Job',
+      customer,
+      vehicle: { year: 2020, make: 'Ford', model: 'Focus', customer },
+    },
+  })
+}
+
+const ALL = { sms: true, email: true, telegram: true }
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  statusReport.update.mockResolvedValue({})
+  sendNotificationEmail.mockResolvedValue({ success: true })
+  sendSmsToCustomer.mockResolvedValue({ success: true })
+  sendTelegramToCustomer.mockResolvedValue({ success: true })
+  report()
+})
+
+describe('what the report records', () => {
+  it('lists every channel that went out', async () => {
+    const res = await sendStatusReport({ statusReportId: 'rep', channels: ALL })
+    expect(res.success).toBe(true)
+    expect(res.data?.channels.sort()).toEqual(['email', 'sms', 'telegram'])
+    expect(statusReport.update).toHaveBeenCalledWith(
+      expect.objectContaining({ data: expect.objectContaining({ sentVia: 'email,sms,telegram' }) })
+    )
+  })
+
+  it('leaves out a channel the provider refused', async () => {
+    // The reported bug. SMS comes back unsuccessful, and the row used to say
+    // it was sent anyway.
+    sendSmsToCustomer.mockResolvedValue({ success: false, error: 'no credit' })
+
+    const res = await sendStatusReport({ statusReportId: 'rep', channels: ALL })
+    expect(res.success).toBe(true)
+    expect(res.data?.channels).not.toContain('sms')
+    expect(statusReport.update).toHaveBeenCalledWith(
+      expect.objectContaining({ data: expect.objectContaining({ sentVia: 'email,telegram' }) })
+    )
+  })
+
+  it('reports the refusal rather than swallowing it', async () => {
+    sendSmsToCustomer.mockResolvedValue({ success: false, error: 'no credit' })
+    const res = await sendStatusReport({ statusReportId: 'rep', channels: ALL })
+    expect(res.data?.failures).toContainEqual({ channel: 'sms', error: 'no credit' })
+  })
+
+  it('fails outright when nothing got through', async () => {
+    // Nothing reached the customer, so nothing should say it did.
+    sendNotificationEmail.mockResolvedValue({ success: false, error: 'smtp down' })
+    sendSmsToCustomer.mockResolvedValue({ success: false, error: 'no credit' })
+    sendTelegramToCustomer.mockResolvedValue({ success: false, error: 'blocked' })
+
+    const res = await sendStatusReport({ statusReportId: 'rep', channels: ALL })
+    expect(res.success).toBe(false)
+    expect(res.error).toContain('no credit')
+    expect(statusReport.update).not.toHaveBeenCalled()
+  })
+
+  it('counts a missing address as a failure, not a quiet skip', async () => {
+    // Someone ticked SMS for a customer with no number. Doing nothing and
+    // saying "sent" is the same lie in a quieter voice.
+    report({ ...CUSTOMER, phone: null })
+    const res = await sendStatusReport({ statusReportId: 'rep', channels: ALL })
+    expect(res.data?.channels).not.toContain('sms')
+    expect(res.data?.failures.map((f) => f.channel)).toContain('sms')
+    expect(sendSmsToCustomer).not.toHaveBeenCalled()
+  })
+
+  it('does not send on a channel that was not asked for', async () => {
+    await sendStatusReport({
+      statusReportId: 'rep',
+      channels: { sms: true, email: false, telegram: false },
+    })
+    expect(sendSmsToCustomer).toHaveBeenCalled()
+    expect(sendNotificationEmail).not.toHaveBeenCalled()
+    expect(sendTelegramToCustomer).not.toHaveBeenCalled()
+  })
+})

+ 69 - 25
src/features/status-reports/Actions/sendStatusReport.ts

@@ -65,37 +65,76 @@ export async function sendStatusReport(input: unknown) {
         : t("defaultMessage", { name: customer.name, vehicle: vehicleName, url: publicUrl });
 
       const sentChannels: string[] = [];
+      const failures: { channel: string; error: string }[] = [];
 
-      if (data.channels.email && customer.email) {
-        await sendNotificationEmail({
-          recipientEmail: customer.email,
-          subject: t("emailSubject", { vehicle: vehicleName }),
-          body: messageBody,
-        });
-        sentChannels.push("email");
+      /**
+       * Records a channel as sent only if it was.
+       *
+       * The three senders are withAuth actions, so a refusal comes back as
+       * `{ success: false }` rather than as a thrown error. Nothing here used
+       * to read that, so a text message the provider rejected still counted,
+       * and the report was filed as sent over a channel that sent nothing.
+       *
+       * A missing address counts as a failure too. Someone ticked the box, and
+       * silently doing nothing is the same lie in a quieter voice.
+       */
+      const attempt = async (
+        channel: string,
+        address: string | null | undefined,
+        send: () => Promise<{ success: boolean; error?: string }>,
+      ) => {
+        if (!address) {
+          failures.push({ channel, error: "no address on file" });
+          return;
+        }
+        const result = await send();
+        if (result.success) sentChannels.push(channel);
+        else failures.push({ channel, error: result.error ?? "send failed" });
+      };
+
+      if (data.channels.email) {
+        await attempt("email", customer.email, () =>
+          sendNotificationEmail({
+            recipientEmail: customer.email as string,
+            subject: t("emailSubject", { vehicle: vehicleName }),
+            body: messageBody,
+          }),
+        );
+      }
+
+      if (data.channels.sms) {
+        await attempt("sms", customer.phone, () =>
+          sendSmsToCustomer({
+            customerId: customer.id,
+            body: messageBody,
+            relatedEntityType: "status_report",
+            relatedEntityId: report.id,
+          }),
+        );
       }
 
-      if (data.channels.sms && customer.phone) {
-        await sendSmsToCustomer({
-          customerId: customer.id,
-          body: messageBody,
-          relatedEntityType: "status_report",
-          relatedEntityId: report.id,
-        });
-        sentChannels.push("sms");
+      if (data.channels.telegram) {
+        await attempt("telegram", customer.telegramChatId, () =>
+          sendTelegramToCustomer({
+            customerId: customer.id,
+            body: messageBody,
+            relatedEntityType: "status_report",
+            relatedEntityId: report.id,
+          }),
+        );
       }
 
-      if (data.channels.telegram && customer.telegramChatId) {
-        await sendTelegramToCustomer({
-          customerId: customer.id,
-          body: messageBody,
-          relatedEntityType: "status_report",
-          relatedEntityId: report.id,
-        });
-        sentChannels.push("telegram");
+      // Nothing left the building, so nothing is recorded and the caller hears
+      // about it. Marking a report sent when every channel failed is the
+      // failure this whole function exists to avoid.
+      if (sentChannels.length === 0) {
+        throw new Error(
+          failures.map((f) => `${f.channel}: ${f.error}`).join("; ") || "No channel to send on",
+        );
       }
 
-      // Update the report status
+      // Only the channels that actually carried it. A partial send is still a
+      // send, and the row should say which half worked.
       await db.statusReport.update({
         where: { id: report.id },
         data: {
@@ -105,7 +144,12 @@ export async function sendStatusReport(input: unknown) {
         },
       });
 
-      return { sent: true, channels: sentChannels, statusReportId: data.statusReportId };
+      return {
+        sent: true,
+        channels: sentChannels,
+        failures,
+        statusReportId: data.statusReportId,
+      };
     },
     {
       requiredPermissions: [

+ 13 - 5
src/features/status-reports/Components/SendStatusReportDialog.tsx

@@ -78,11 +78,19 @@ export function SendStatusReportDialog({
       });
 
       if (res.success) {
-        const channels: string[] = [];
-        if (sendSms) channels.push(t("sms"));
-        if (sendEmail) channels.push(t("email"));
-        if (sendTelegram) channels.push(t("telegram"));
-        toast.success(`${t("sent")}: ${channels.join(", ")}`);
+        // What the server got out, not what was ticked here. Those were the
+        // same thing only as long as nothing could fail.
+        const label = (channel: string) =>
+          channel === "sms" ? t("sms") : channel === "email" ? t("email") : t("telegram");
+        const sent = (res.data?.channels ?? []).map(label);
+        const failed = (res.data?.failures ?? []).map((f) => label(f.channel));
+
+        toast.success(`${t("sent")}: ${sent.join(", ")}`);
+        // A partial send is still worth flagging: the customer got one message
+        // and the desk should know the other never arrived.
+        if (failed.length > 0) {
+          toast.warning(`${t("notSent")}: ${failed.join(", ")}`);
+        }
         onOpenChange(false);
       } else {
         toast.error(res.error || t("failed"));