Parcourir la source

Draw the eye on a photo as what the customer can see (#417)

* Draw the eye on a photo as what the customer can see

* Answer a live change from the technician app even when it is your own
Bernt Christian Egeland il y a 1 semaine
Parent
commit
4c70674097

+ 62 - 0
src/__tests__/features/realtime/own-echo.test.ts

@@ -0,0 +1,62 @@
+/**
+ * Which live changes a page answers, and which it treats as its own echo.
+ *
+ * The page that made a change has already shown the result, so it must not
+ * re-read on hearing about it. That rule once swallowed a real change: the
+ * technician app finishing a job, signed in as the same person who had the
+ * work order open at the desk. The check read the user, the phone holds no
+ * presence in the room, and the desk sat on the old status until a reload.
+ * These pin what counts as an echo, so that does not come back.
+ */
+
+import { describe, expect, it } from 'vitest'
+import { isOwnEcho } from '@/features/realtime/hooks'
+import type { PresenceUser, RecordChange } from '@/lib/realtime/events'
+
+const me = { userId: 'u-1', name: 'Christian', color: '#000' }
+
+const change = (by: RecordChange['by']): RecordChange => ({
+  kind: 'serviceRecord',
+  id: 'rec-1',
+  organizationId: 'org-1',
+  action: 'updated',
+  by,
+  at: Date.now(),
+})
+
+const here = (devices: number): PresenceUser[] => [
+  { userId: 'u-1', name: 'Christian', color: '#000', devices },
+]
+
+describe('isOwnEcho', () => {
+  it('answers a change the technician app made, whoever is signed in on the phone', () => {
+    expect(isOwnEcho(change({ userId: 'u-1', name: null, source: 'app' }), me, here(1))).toBe(false)
+  })
+
+  it('answers a change the system made', () => {
+    expect(isOwnEcho(change({ userId: null, name: null, source: 'system' }), me, [])).toBe(false)
+  })
+
+  it('answers a colleague on the web', () => {
+    expect(isOwnEcho(change({ userId: 'u-2', name: 'Ellen', source: 'web' }), me, here(1))).toBe(
+      false
+    )
+  })
+
+  it('ignores its own web save when this is the only page open', () => {
+    expect(isOwnEcho(change({ userId: 'u-1', name: null, source: 'web' }), me, here(1))).toBe(true)
+    expect(isOwnEcho(change({ userId: 'u-1', name: null, source: 'web' }), me, undefined)).toBe(
+      true
+    )
+  })
+
+  it('answers its own web save when the same person also has the record open elsewhere', () => {
+    expect(isOwnEcho(change({ userId: 'u-1', name: null, source: 'web' }), me, here(2))).toBe(false)
+  })
+
+  it('answers everything when nobody is signed in to the socket', () => {
+    expect(isOwnEcho(change({ userId: 'u-1', name: null, source: 'web' }), null, here(1))).toBe(
+      false
+    )
+  })
+})

+ 33 - 9
src/features/realtime/hooks.ts

@@ -50,6 +50,33 @@ export function useRecordChanges(
   }, [realtime, kind, id])
 }
 
+/**
+ * Whether a change is this page's own save coming back, which it must not
+ * answer: the page has already shown the result, and re-reading would fight
+ * the form somebody is typing in.
+ *
+ * Only a change from another web page of the same person can be that. One
+ * from the technician app is never an echo of a browser, whoever is signed
+ * in on the phone, and a one-person shop is signed in on both: for a while
+ * the desk ignored its own phone finishing a job, because the check read the
+ * user and not the source, and the phone holds no presence to say it is a
+ * second device.
+ *
+ * Between two of the person's own web pages, presence decides: a save on the
+ * laptop has to reach their own tablet in the bay, and the room's `devices`
+ * count says whether there is one.
+ */
+export function isOwnEcho(
+  change: RecordChange,
+  me: Me | null | undefined,
+  presence: PresenceUser[] | undefined
+): boolean {
+  if (change.by.source !== 'web') return false
+  if (!change.by.userId || !me || change.by.userId !== me.userId) return false
+  const mine = presence?.find((user) => user.userId === me.userId)
+  return !mine || mine.devices < 2
+}
+
 /**
  * The record kept current on screen: re-read the page's own server data when
  * it changes somewhere else.
@@ -101,15 +128,12 @@ export function useLiveRecord(
     id,
     useCallback(
       (change) => {
-        const me = realtime?.getState().me
-        if (change?.by.userId && change.by.userId === me?.userId && id) {
-          // Unless this person has the record open somewhere else as well: a
-          // save on the laptop has to reach their own tablet in the bay. The
-          // room's presence says so, as `devices`, when the page shows chips.
-          const mine = realtime
-            ?.presenceOf(recordRoom(kind, id))
-            .find((user) => user.userId === me.userId)
-          if (!mine || mine.devices < 2) return
+        if (
+          change &&
+          id &&
+          isOwnEcho(change, realtime?.getState().me, realtime?.presenceOf(recordRoom(kind, id)))
+        ) {
+          return
         }
         onChange.current?.(change)
         governor.current?.request()

+ 5 - 2
src/features/vehicles/Components/service-page/modern/MediaGrid.tsx

@@ -308,10 +308,13 @@ export function MediaGrid({ kind, serviceRecordId, files, max, customerId }: Med
                   }
                   className={tileAction}
                 >
+                  {/* The icon is the state, the tooltip is the action: an open
+                      eye means the customer can see this. Drawn the other way
+                      round it read as "hidden" on every visible photo. */}
                   {file.includeInInvoice ? (
-                    <EyeOff className="h-3.5 w-3.5" />
-                  ) : (
                     <Eye className="h-3.5 w-3.5" />
+                  ) : (
+                    <EyeOff className="h-3.5 w-3.5" />
                   )}
                 </button>
                 <button