Ver código fonte

Announce only minor releases, for an hour, with the clock on the user record (#418)

Bernt Christian Egeland 1 mês atrás
pai
commit
70a9a834bb

+ 3 - 0
prisma/migrations/20260922090000_update_banner_shown_at/migration.sql

@@ -0,0 +1,3 @@
+-- AlterTable
+ALTER TABLE "users" ADD COLUMN     "updateBannerVersion" TEXT,
+ADD COLUMN     "updateBannerShownAt" TIMESTAMP(3);

+ 5 - 0
prisma/schema/auth.prisma

@@ -24,6 +24,11 @@ model User {
   /// App version this user last saw/dismissed the update banner for.
   /// Null = never seeded (seeded silently on first load, no banner).
   lastSeenVersion  String?
+  /// The release the update banner is up for, and when it first appeared to
+  /// this user on any device. The hour it stays up runs from here, so a
+  /// second device does not restart them and a closed tab cannot stop them.
+  updateBannerVersion String?
+  updateBannerShownAt DateTime?
   /// Per-user dashboard grid layout (card positions/sizes/hidden set).
   /// Null = default layout. Shape validated in dashboardLayoutActions.
   dashboardLayout  Json?

+ 50 - 35
src/__tests__/components/update-banner.test.tsx

@@ -1,11 +1,12 @@
 /**
  * The "we deployed something" strip.
  *
- * The behaviour worth pinning is that it lets itself out. Almost nobody
- * presses the X, so without the six-hour clock the notice rides along until
- * the next release and stops being read at all. The clock is per browser, but
- * expiry writes the version server-side exactly as a real dismissal does, so
- * the banner does not come back on the user's other devices.
+ * Two things worth pinning. It only speaks for a minor or major release: a
+ * patch is recorded as seen and shows nothing, because a strip for every
+ * daily release was never read and never went away. And it lets itself out:
+ * the hour runs on the user record from the first sighting on any
+ * device, and expiry writes the version server-side exactly as a dismissal
+ * does, so the banner does not come back anywhere.
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
@@ -15,21 +16,30 @@ import { BannerSlotProvider } from '@/components/banner-slot'
 vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key }))
 
 const markVersionSeen = vi.fn()
+const markUpdateBannerShown = vi.fn()
 vi.mock('@/features/users/Actions/versionActions', () => ({
   markVersionSeen: (version: string) => markVersionSeen(version),
+  markUpdateBannerShown: (version: string) => markUpdateBannerShown(version),
 }))
 
 const { UpdateBanner } = await import('@/components/update-banner')
 
-const SIX_HOURS = 6 * 60 * 60 * 1000
+const ONE_HOUR = 60 * 60 * 1000
 
 /** The banner only renders inside the slot it competes for. */
-function show(lastSeenVersion: string | null = '1.0.0') {
+function show({
+  currentVersion = 'v1.1.0',
+  lastSeenVersion = 'v1.0.0' as string | null,
+  shownVersion = null as string | null,
+  shownAt = null as string | null,
+} = {}) {
   return render(
     <BannerSlotProvider>
       <UpdateBanner
-        currentVersion="1.1.0"
+        currentVersion={currentVersion}
         lastSeenVersion={lastSeenVersion}
+        shownVersion={shownVersion}
+        shownAt={shownAt}
         releaseNotesUrl="https://example.test/releases"
       />
     </BannerSlotProvider>
@@ -41,8 +51,8 @@ function shown() {
 }
 
 beforeEach(() => {
-  localStorage.clear()
   markVersionSeen.mockClear()
+  markUpdateBannerShown.mockClear()
   vi.useFakeTimers({ shouldAdvanceTime: true })
 })
 
@@ -51,9 +61,18 @@ afterEach(() => {
 })
 
 describe('the update banner', () => {
-  it('announces a version the account has not seen', () => {
+  it('announces a minor release the account has not seen, and starts its clock', () => {
     show()
     expect(shown()).not.toBeNull()
+    expect(markUpdateBannerShown).toHaveBeenCalledWith('v1.1.0')
+    expect(markVersionSeen).not.toHaveBeenCalled()
+  })
+
+  it('says nothing for a patch release, and records it as seen', () => {
+    show({ currentVersion: 'v1.0.1' })
+    expect(shown()).toBeNull()
+    expect(markVersionSeen).toHaveBeenCalledWith('v1.0.1')
+    expect(markUpdateBannerShown).not.toHaveBeenCalled()
   })
 
   it('stays gone once dismissed, and records the version', () => {
@@ -62,54 +81,50 @@ describe('the update banner', () => {
       screen.getByRole('button').click()
     })
     expect(shown()).toBeNull()
-    expect(markVersionSeen).toHaveBeenCalledWith('1.1.0')
+    expect(markVersionSeen).toHaveBeenCalledWith('v1.1.0')
   })
 
-  it('lets itself out six hours after it first appeared', () => {
+  it('lets itself out an hour after it first appeared', () => {
     show()
     expect(shown()).not.toBeNull()
 
     act(() => {
-      vi.advanceTimersByTime(SIX_HOURS)
+      vi.advanceTimersByTime(ONE_HOUR)
     })
 
     expect(shown()).toBeNull()
-    expect(markVersionSeen).toHaveBeenCalledWith('1.1.0')
+    expect(markVersionSeen).toHaveBeenCalledWith('v1.1.0')
   })
 
-  it('keeps the clock running across reloads rather than restarting it', () => {
-    show().unmount()
-
-    // Five hours later, in a fresh tab: one hour left, not six.
-    vi.setSystemTime(Date.now() + 5 * 60 * 60 * 1000)
-    show()
+  it('runs the clock from the first sighting on any device, not from this load', () => {
+    show({
+      shownVersion: 'v1.1.0',
+      shownAt: new Date(Date.now() - 50 * 60 * 1000).toISOString(),
+    })
     expect(shown()).not.toBeNull()
+    expect(markUpdateBannerShown).not.toHaveBeenCalled()
 
     act(() => {
-      vi.advanceTimersByTime(60 * 60 * 1000)
+      vi.advanceTimersByTime(10 * 60 * 1000)
     })
     expect(shown()).toBeNull()
   })
 
-  it('goes straight away when the six hours passed while the app was closed', () => {
-    show().unmount()
-
-    vi.setSystemTime(Date.now() + SIX_HOURS + 1000)
-    show()
-
+  it('goes straight away when the hour passed while the app was closed', () => {
+    show({ shownVersion: 'v1.1.0', shownAt: new Date(Date.now() - ONE_HOUR - 1000).toISOString() })
     expect(shown()).toBeNull()
-    expect(markVersionSeen).toHaveBeenCalledWith('1.1.0')
+    expect(markVersionSeen).toHaveBeenCalledWith('v1.1.0')
   })
 
-  it('gives the next release its own six hours', () => {
-    localStorage.setItem('update-banner-first-seen', `1.0.0|${Date.now() - SIX_HOURS - 1000}`)
-    show()
-    expect(shown()).not.toBeNull()
+  it('says nothing to an account seeing the app for the first time', () => {
+    show({ lastSeenVersion: null })
+    expect(shown()).toBeNull()
+    expect(markVersionSeen).toHaveBeenCalledWith('v1.1.0')
   })
 
-  it('says nothing to an account seeing the app for the first time', () => {
-    show(null)
+  it('says nothing in development', () => {
+    show({ currentVersion: 'development' })
     expect(shown()).toBeNull()
-    expect(markVersionSeen).toHaveBeenCalledWith('1.1.0')
+    expect(markVersionSeen).not.toHaveBeenCalled()
   })
 })

+ 105 - 0
src/__tests__/lib/update-banner-rule.test.ts

@@ -0,0 +1,105 @@
+/**
+ * Which releases get a banner, and for how long.
+ *
+ * The app ships most days. A strip for every patch was never read and never
+ * went away: nobody pressed the X, the clock ran per browser from each
+ * person's first sight, and the next release restarted them. So a patch is
+ * silent, a minor release is announced, and the hour runs once per
+ * person on the user record.
+ */
+
+import { describe, expect, it } from 'vitest'
+import {
+  decideUpdateBanner,
+  isAnnounced,
+  parseVersion,
+  UPDATE_BANNER_TTL_MS,
+} from '@/lib/update-banner-rule'
+
+describe('isAnnounced', () => {
+  it('announces a minor release', () => {
+    expect(isAnnounced('v1.2.54', 'v1.3.0')).toBe(true)
+  })
+
+  it('announces a major release', () => {
+    expect(isAnnounced('v1.9.3', 'v2.0.0')).toBe(true)
+  })
+
+  it('says nothing for a patch', () => {
+    expect(isAnnounced('v1.2.53', 'v1.2.54')).toBe(false)
+  })
+
+  it('says nothing for a rollback', () => {
+    expect(isAnnounced('v1.3.0', 'v1.2.54')).toBe(false)
+  })
+
+  it('says nothing when either side is not a release', () => {
+    expect(isAnnounced('v1.2.54', 'development')).toBe(false)
+    expect(isAnnounced('v1.2.54', 'demo-abc1234')).toBe(false)
+    expect(isAnnounced(null, 'v1.3.0')).toBe(false)
+  })
+
+  it('reads a tag with or without the v, and with a suffix', () => {
+    expect(parseVersion('1.3.0')).toEqual({ major: 1, minor: 3, patch: 0 })
+    expect(parseVersion('v1.3.0-rc1')).toEqual({ major: 1, minor: 3, patch: 0 })
+    expect(parseVersion('latest')).toBeNull()
+  })
+})
+
+describe('decideUpdateBanner', () => {
+  const now = Date.parse('2026-09-22T09:00:00Z')
+  const user = (over: Partial<Parameters<typeof decideUpdateBanner>[0]> = {}) => ({
+    lastSeenVersion: 'v1.2.54',
+    updateBannerVersion: null,
+    updateBannerShownAt: null,
+    ...over,
+  })
+
+  it('shows a minor release the first time, and asks for the clock to be started', () => {
+    expect(decideUpdateBanner(user(), 'v1.3.0', now)).toEqual({
+      kind: 'show',
+      expiresAt: now + UPDATE_BANNER_TTL_MS,
+      stamp: true,
+    })
+  })
+
+  it('keeps the clock that was started on another device', () => {
+    const shownAt = new Date(now - 20 * 60 * 1000)
+    expect(
+      decideUpdateBanner(
+        user({ updateBannerVersion: 'v1.3.0', updateBannerShownAt: shownAt }),
+        'v1.3.0',
+        now
+      )
+    ).toEqual({ kind: 'show', expiresAt: shownAt.getTime() + UPDATE_BANNER_TTL_MS, stamp: false })
+  })
+
+  it('is over an hour after it first appeared, wherever that was', () => {
+    const shownAt = new Date(now - UPDATE_BANNER_TTL_MS - 1000)
+    expect(
+      decideUpdateBanner(
+        user({ updateBannerVersion: 'v1.3.0', updateBannerShownAt: shownAt }),
+        'v1.3.0',
+        now
+      )
+    ).toEqual({ kind: 'expired' })
+  })
+
+  it('gives the next announced release its own hour', () => {
+    const old = new Date(now - 10 * UPDATE_BANNER_TTL_MS)
+    expect(
+      decideUpdateBanner(
+        user({ updateBannerVersion: 'v1.3.0', updateBannerShownAt: old }),
+        'v1.4.0',
+        now
+      )
+    ).toMatchObject({ kind: 'show', stamp: true })
+  })
+
+  it('is silent for a patch, and for a version already seen', () => {
+    expect(decideUpdateBanner(user(), 'v1.2.55', now)).toEqual({ kind: 'silent' })
+    expect(decideUpdateBanner(user({ lastSeenVersion: 'v1.3.0' }), 'v1.3.0', now)).toEqual({
+      kind: 'silent',
+    })
+  })
+})

+ 2 - 0
src/app/(authenticated)/layout.tsx

@@ -284,6 +284,8 @@ export default async function DashboardLayout({ children }: { children: React.Re
             <UpdateBanner
               currentVersion={process.env.APP_VERSION || 'development'}
               lastSeenVersion={data.lastSeenVersion}
+              shownVersion={data.updateBannerVersion}
+              shownAt={data.updateBannerShownAt?.toISOString() ?? null}
               releaseNotesUrl={
                 process.env.RELEASE_NOTES_URL || 'https://github.com/Torqvoice/torqvoice/releases'
               }

+ 39 - 65
src/components/update-banner.tsx

@@ -3,99 +3,73 @@
 import { useCallback, useEffect, useState } from 'react'
 import { useTranslations } from 'next-intl'
 import { X } from 'lucide-react'
-import { markVersionSeen } from '@/features/users/Actions/versionActions'
+import { markUpdateBannerShown, markVersionSeen } from '@/features/users/Actions/versionActions'
+import { decideUpdateBanner } from '@/lib/update-banner-rule'
 import { BANNER_PRIORITY, useBannerSlot } from './banner-slot'
 
 /**
- * How long the banner stays up when nobody touches it. Six hours is a full
- * working day of chances to read it; past that it has stopped being a notice
- * and become part of the header.
- */
-const AUTO_DISMISS_MS = 6 * 60 * 60 * 1000
-
-/** `<version>|<epoch ms>`: when this release's banner first appeared here. */
-const FIRST_SEEN_KEY = 'update-banner-first-seen'
-
-function firstSeenAt(version: string): number {
-  try {
-    const [seenVersion, at] = (localStorage.getItem(FIRST_SEEN_KEY) ?? '').split('|')
-    return seenVersion === version ? Number(at) || 0 : 0
-  } catch {
-    // localStorage unavailable; the clock restarts on this visit
-    return 0
-  }
-}
-
-function rememberFirstSeen(version: string, at: number) {
-  try {
-    localStorage.setItem(FIRST_SEEN_KEY, `${version}|${at}`)
-  } catch {
-    // localStorage unavailable; the banner then runs its six hours from now
-  }
-}
-
-/**
- * One-time "the app was updated" notice, shown when the running APP_VERSION
- * differs from the version stored on the user record. Dismissing (or opening
- * the release notes) stores the current version server-side, so the banner
- * appears exactly once per account per release, on any device.
+ * The "the app was updated" strip, shown when the running APP_VERSION is a
+ * minor or major release the account has not seen. A patch release says
+ * nothing: it is recorded as seen and the next comparison starts from it.
+ * The rule, and the hour the strip stays up, are in
+ * lib/update-banner-rule.ts; this only draws what it decides.
+ *
+ * Dismissing (or opening the release notes) stores the version on the user
+ * record, so the banner appears exactly once per account per release, on any
+ * device. So does letting it expire: the clock is on the user record too,
+ * started the first time this release's banner appeared anywhere.
  */
 export function UpdateBanner({
   currentVersion,
   lastSeenVersion,
+  shownVersion,
+  shownAt,
   releaseNotesUrl,
 }: {
   currentVersion: string
   lastSeenVersion: string | null
+  /** The release the banner was last stamped as shown for, and when (ISO). */
+  shownVersion: string | null
+  shownAt: string | null
   releaseNotesUrl: string
 }) {
   const t = useTranslations('common.updateBanner')
   const [dismissed, setDismissed] = useState(false)
 
   const neverSeeded = lastSeenVersion === null
-  const show =
-    !dismissed &&
-    !neverSeeded &&
-    currentVersion !== 'development' &&
-    lastSeenVersion !== currentVersion
+  const decision = neverSeeded
+    ? null
+    : decideUpdateBanner(
+        { lastSeenVersion, updateBannerVersion: shownVersion, updateBannerShownAt: shownAt },
+        currentVersion
+      )
+  const show = !dismissed && decision?.kind === 'show'
 
   const acknowledge = useCallback(() => {
     setDismissed(true)
     markVersionSeen(currentVersion)
   }, [currentVersion])
 
-  // First load ever for this account: seed silently so a brand-new user is
-  // not greeted with "what's new" for a version they never used.
   useEffect(() => {
-    if (neverSeeded && currentVersion !== 'development') {
-      markVersionSeen(currentVersion)
-    }
-  }, [neverSeeded, currentVersion])
-
-  // Hardly anyone presses the X, so the banner otherwise rides along until the
-  // next release. Six hours after it first appeared it acknowledges itself.
-  // The clock lives in this browser, but the acknowledgement is the same
-  // server-side write as the X, so it clears the banner on every device.
-  useEffect(() => {
-    if (!show) return
-
-    const now = Date.now()
-    let since = firstSeenAt(currentVersion)
-    if (!since) {
-      since = now
-      rememberFirstSeen(currentVersion, now)
-    }
-
-    const remaining = since + AUTO_DISMISS_MS - now
-    if (remaining <= 0) {
-      acknowledge()
+    if (currentVersion === 'development') return
+    // First load ever for this account: seed silently so a brand-new user is
+    // not greeted with "what's new" for a version they never used. A patch
+    // release, and a banner whose hour ran out while the app was closed,
+    // are recorded the same way, without anything on screen.
+    if (neverSeeded || decision?.kind === 'silent' || decision?.kind === 'expired') {
+      if (lastSeenVersion !== currentVersion) markVersionSeen(currentVersion)
       return
     }
+    if (decision?.kind === 'show' && decision.stamp) markUpdateBannerShown(currentVersion)
+  }, [neverSeeded, decision?.kind, currentVersion, lastSeenVersion]) // eslint-disable-line react-hooks/exhaustive-deps -- `decision.stamp` follows `kind`
 
-    // Also covers a tab left open across the deadline.
-    const timer = setTimeout(acknowledge, remaining)
+  // A tab left open across the deadline: the strip lets itself out, with the
+  // same write the X makes, so it clears on every device.
+  useEffect(() => {
+    if (!show || decision?.kind !== 'show') return
+    const timer = setTimeout(acknowledge, Math.max(0, decision.expiresAt - Date.now()))
     return () => clearTimeout(timer)
-  }, [show, currentVersion, acknowledge])
+  }, [show, decision, acknowledge])
 
   // Last in the queue. Interesting, never urgent, and it waits behind an
   // outage notice rather than sitting under one.

+ 20 - 0
src/features/users/Actions/versionActions.ts

@@ -22,3 +22,23 @@ export async function markVersionSeen(version: string) {
   })
   return { success: true }
 }
+
+/**
+ * Records when the update banner for this release first appeared to the
+ * current user. Only the first sighting counts: a second device, or a reload,
+ * finds the version already stamped and leaves the clock where it is. That is
+ * what makes the hour run once per person rather than once per tab.
+ */
+export async function markUpdateBannerShown(version: string) {
+  const session = await getCachedSession()
+  if (!session?.user?.id) return { success: false }
+
+  const clean = version.slice(0, 64)
+  if (!clean) return { success: false }
+
+  await db.user.updateMany({
+    where: { id: session.user.id, NOT: { updateBannerVersion: clean } },
+    data: { updateBannerVersion: clean, updateBannerShownAt: new Date() },
+  })
+  return { success: true }
+}

+ 11 - 1
src/lib/get-layout-data.ts

@@ -15,6 +15,8 @@ type AuthResult =
       isSuperAdmin: boolean
       emailVerified: boolean
       lastSeenVersion: string | null
+      updateBannerVersion: string | null
+      updateBannerShownAt: Date | null
       companyLogo: string | undefined
       dateFormat: string | undefined
       timeFormat: string | undefined
@@ -34,7 +36,13 @@ export async function getLayoutData(): Promise<AuthResult> {
     getCachedMembership(session.user.id),
     db.user.findUnique({
       where: { id: session.user.id },
-      select: { isSuperAdmin: true, emailVerified: true, lastSeenVersion: true },
+      select: {
+        isSuperAdmin: true,
+        emailVerified: true,
+        lastSeenVersion: true,
+        updateBannerVersion: true,
+        updateBannerShownAt: true,
+      },
     }),
   ])
 
@@ -88,6 +96,8 @@ export async function getLayoutData(): Promise<AuthResult> {
     isSuperAdmin,
     emailVerified: user?.emailVerified ?? false,
     lastSeenVersion: user?.lastSeenVersion ?? null,
+    updateBannerVersion: user?.updateBannerVersion ?? null,
+    updateBannerShownAt: user?.updateBannerShownAt ?? null,
     companyLogo: orgMap.get(SETTING_KEYS.COMPANY_LOGO) || undefined,
     dateFormat: orgMap.get(SETTING_KEYS.DATE_FORMAT) || undefined,
     timeFormat: orgMap.get(SETTING_KEYS.TIME_FORMAT) || undefined,

+ 79 - 0
src/lib/update-banner-rule.ts

@@ -0,0 +1,79 @@
+/**
+ * When a release is worth a banner, and for how long.
+ *
+ * Torqvoice ships most days, and a strip at the top of every screen for
+ * every patch stopped being read: nobody pressed the X, and the next release
+ * arrived before the notice had let itself out. So a patch (1.2.53 to 1.2.54)
+ * says nothing, and only a minor or major release (1.2.x to 1.3.0) is
+ * announced. Which is which is decided by the tag.
+ *
+ * The clock is on the user record, once per release across every device:
+ * one hour from when the banner first appeared to that person, wherever
+ * that was. Past that it is marked seen and does not come back.
+ */
+
+/** One hour: long enough to be read, short enough to never become furniture. */
+export const UPDATE_BANNER_TTL_MS = 60 * 60 * 1000
+
+/** Versions the app runs under that are not releases at all. */
+const NOT_A_RELEASE = new Set(['development', ''])
+
+interface Parsed {
+  major: number
+  minor: number
+  patch: number
+}
+
+/** `v1.2.54`, `1.2.54` or `1.2.54-rc1`. Anything else is not a release. */
+export function parseVersion(version: string | null | undefined): Parsed | null {
+  if (!version || NOT_A_RELEASE.has(version)) return null
+  const m = /^v?(\d+)\.(\d+)\.(\d+)/.exec(version.trim())
+  if (!m) return null
+  return { major: Number(m[1]), minor: Number(m[2]), patch: Number(m[3]) }
+}
+
+/**
+ * Whether going from the version the user last saw to the one running is
+ * worth telling them about. Only a step up in the major or minor part is; a
+ * patch, a rollback, or a version that is not a release is not.
+ *
+ * A user who has never been seeded is never announced to here: the first
+ * load seeds them silently, as it always has.
+ */
+export function isAnnounced(lastSeen: string | null, current: string): boolean {
+  const seen = parseVersion(lastSeen)
+  const now = parseVersion(current)
+  if (!seen || !now) return false
+  if (now.major !== seen.major) return now.major > seen.major
+  return now.minor > seen.minor
+}
+
+export type UpdateBannerDecision =
+  /** Nothing to show; write the version as seen so the next comparison starts here. */
+  | { kind: 'silent' }
+  /** Show it, and stamp when it first appeared if that has not happened yet. */
+  | { kind: 'show'; expiresAt: number; stamp: boolean }
+  /** Its hour is over: mark seen, show nothing. */
+  | { kind: 'expired' }
+
+export function decideUpdateBanner(
+  user: {
+    lastSeenVersion: string | null
+    updateBannerVersion: string | null
+    updateBannerShownAt: Date | string | null
+  },
+  current: string,
+  now = Date.now()
+): UpdateBannerDecision {
+  if (user.lastSeenVersion === current) return { kind: 'silent' }
+  if (!isAnnounced(user.lastSeenVersion, current)) return { kind: 'silent' }
+
+  const shownAt =
+    user.updateBannerVersion === current && user.updateBannerShownAt
+      ? new Date(user.updateBannerShownAt).getTime()
+      : null
+  if (shownAt === null) return { kind: 'show', expiresAt: now + UPDATE_BANNER_TTL_MS, stamp: true }
+  const expiresAt = shownAt + UPDATE_BANNER_TTL_MS
+  if (expiresAt <= now) return { kind: 'expired' }
+  return { kind: 'show', expiresAt, stamp: false }
+}