From 94eb59874010fc2ee2ccf5a465029bccf89900b1 Mon Sep 17 00:00:00 2001 From: ksg98 Date: Sat, 21 Mar 2026 20:19:43 -0700 Subject: [PATCH] feat(web): conditionally show upgrade toast only to org owners Only org owners should see the upgrade notification toast. Non-owners and anonymous users no longer trigger the version check or see the popup. Fixes #815 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../[domain]/components/upgradeToast.test.tsx | 119 ++++++++++++++++++ .../app/[domain]/components/upgradeToast.tsx | 19 ++- packages/web/src/app/[domain]/layout.tsx | 25 ++-- 3 files changed, 146 insertions(+), 17 deletions(-) create mode 100644 packages/web/src/app/[domain]/components/upgradeToast.test.tsx diff --git a/packages/web/src/app/[domain]/components/upgradeToast.test.tsx b/packages/web/src/app/[domain]/components/upgradeToast.test.tsx new file mode 100644 index 000000000..c7b555d95 --- /dev/null +++ b/packages/web/src/app/[domain]/components/upgradeToast.test.tsx @@ -0,0 +1,119 @@ +import { describe, expect, test, vi, beforeEach } from 'vitest' +import { getVersionFromString, getVersionString, compareVersions } from './upgradeToast'; + +// --- Pure utility function tests --- + +describe('getVersionFromString', () => { + test('parses a valid semver string', () => { + expect(getVersionFromString('v1.2.3')).toEqual({ major: 1, minor: 2, patch: 3 }); + }); + + test('returns null for invalid version strings', () => { + expect(getVersionFromString('not-a-version')).toBeNull(); + expect(getVersionFromString('1.2.3')).toBeNull(); // missing v prefix + expect(getVersionFromString('v1.2')).toBeNull(); // missing patch + expect(getVersionFromString('v1.2.3-beta')).toBeNull(); // pre-release suffix + }); + + test('parses zero versions', () => { + expect(getVersionFromString('v0.0.0')).toEqual({ major: 0, minor: 0, patch: 0 }); + }); +}); + +describe('getVersionString', () => { + test('formats a version object as a string', () => { + expect(getVersionString({ major: 1, minor: 2, patch: 3 })).toBe('v1.2.3'); + }); + + test('formats zero version', () => { + expect(getVersionString({ major: 0, minor: 0, patch: 0 })).toBe('v0.0.0'); + }); +}); + +describe('compareVersions', () => { + test('returns 0 for equal versions', () => { + const v = { major: 1, minor: 2, patch: 3 }; + expect(compareVersions(v, v)).toBe(0); + }); + + test('compares by major version first', () => { + const a = { major: 2, minor: 0, patch: 0 }; + const b = { major: 1, minor: 9, patch: 9 }; + expect(compareVersions(a, b)).toBeGreaterThan(0); + expect(compareVersions(b, a)).toBeLessThan(0); + }); + + test('compares by minor version when major is equal', () => { + const a = { major: 1, minor: 3, patch: 0 }; + const b = { major: 1, minor: 2, patch: 9 }; + expect(compareVersions(a, b)).toBeGreaterThan(0); + expect(compareVersions(b, a)).toBeLessThan(0); + }); + + test('compares by patch version when major and minor are equal', () => { + const a = { major: 1, minor: 2, patch: 4 }; + const b = { major: 1, minor: 2, patch: 3 }; + expect(compareVersions(a, b)).toBeGreaterThan(0); + expect(compareVersions(b, a)).toBeLessThan(0); + }); +}); + +// --- UpgradeToast isOwner gating test --- + +// We mock the external dependencies to isolate the isOwner behavior. +// The key assertion: when isOwner=false, fetch should NOT be called. + +const mockToast = vi.fn(); +vi.mock('@/components/hooks/use-toast', () => ({ + useToast: () => ({ toast: mockToast }), +})); + +vi.mock('usehooks-ts', () => ({ + useLocalStorage: () => [new Date(0).toUTCString(), vi.fn()], +})); + +vi.mock('@tanstack/react-query', () => ({ + useQuery: () => ({ data: 'v1.0.0' }), +})); + +vi.mock('@/app/api/(client)/client', () => ({ + getVersion: vi.fn(), +})); + +describe('UpgradeToast isOwner gating', () => { + beforeEach(() => { + vi.restoreAllMocks(); + // Reset the global fetch mock before each test + global.fetch = vi.fn(); + }); + + test('does not fetch or show toast when isOwner is false', async () => { + // Dynamic import after mocks are set up + const { UpgradeToast } = await import('./upgradeToast'); + const { render } = await import('@testing-library/react'); + + render(); + + // fetch should not have been called because isOwner is false + expect(global.fetch).not.toHaveBeenCalled(); + expect(mockToast).not.toHaveBeenCalled(); + }); + + test('fetches GitHub tags when isOwner is true', async () => { + const mockResponse = { + json: () => Promise.resolve([{ name: 'v2.0.0' }]), + }; + global.fetch = vi.fn().mockResolvedValue(mockResponse); + + const { UpgradeToast } = await import('./upgradeToast'); + const { render, waitFor } = await import('@testing-library/react'); + + render(); + + await waitFor(() => { + expect(global.fetch).toHaveBeenCalledWith( + 'https://api.github.com/repos/sourcebot-dev/sourcebot/tags' + ); + }); + }); +}); diff --git a/packages/web/src/app/[domain]/components/upgradeToast.tsx b/packages/web/src/app/[domain]/components/upgradeToast.tsx index e3b8bf29c..fb615d529 100644 --- a/packages/web/src/app/[domain]/components/upgradeToast.tsx +++ b/packages/web/src/app/[domain]/components/upgradeToast.tsx @@ -17,7 +17,11 @@ type Version = { patch: number; }; -export const UpgradeToast = () => { +interface UpgradeToastProps { + isOwner: boolean; +} + +export const UpgradeToast = ({ isOwner }: UpgradeToastProps) => { const { toast } = useToast(); const [ upgradeToastLastShownDate, setUpgradeToastLastShownDate ] = useLocalStorage( "upgradeToastLastShownDate", @@ -28,9 +32,14 @@ export const UpgradeToast = () => { queryKey: ["version"], queryFn: () => getVersion(), select: (data) => data.version, + enabled: isOwner, }) useEffect(() => { + if (!isOwner) { + return; + } + if (!versionString) { return; } @@ -82,12 +91,12 @@ export const UpgradeToast = () => { setUpgradeToastLastShownDate(new Date().toUTCString()); }); - }, [setUpgradeToastLastShownDate, toast, upgradeToastLastShownDate, versionString]); + }, [isOwner, setUpgradeToastLastShownDate, toast, upgradeToastLastShownDate, versionString]); return null; } -const getVersionFromString = (version: string): Version | null => { +export const getVersionFromString = (version: string): Version | null => { const match = version.match(SEMVER_REGEX); if (!match) { return null; @@ -99,11 +108,11 @@ const getVersionFromString = (version: string): Version | null => { } satisfies Version; } -const getVersionString = (version: Version) => { +export const getVersionString = (version: Version) => { return `v${version.major}.${version.minor}.${version.patch}`; } -const compareVersions = (a: Version, b: Version) => { +export const compareVersions = (a: Version, b: Version) => { if (a.major !== b.major) { return a.major - b.major; } diff --git a/packages/web/src/app/[domain]/layout.tsx b/packages/web/src/app/[domain]/layout.tsx index 3a1f48b05..db274c587 100644 --- a/packages/web/src/app/[domain]/layout.tsx +++ b/packages/web/src/app/[domain]/layout.tsx @@ -23,6 +23,7 @@ import { JoinOrganizationCard } from "@/app/components/joinOrganizationCard"; import { LogoutEscapeHatch } from "@/app/components/logoutEscapeHatch"; import { GitHubStarToast } from "./components/githubStarToast"; import { UpgradeToast } from "./components/upgradeToast"; +import { OrgRole } from "@sourcebot/db"; import { getLinkedAccounts } from "@/ee/features/sso/actions"; import { PermissionSyncBanner } from "./components/permissionSyncBanner"; import { getPermissionSyncStatus } from "../api/(server)/ee/permissionSyncStatus/api"; @@ -66,19 +67,19 @@ export default async function Layout(props: LayoutProps) { })(); // If the user is authenticated, we must check if they're a member of the org - if (session) { - const membership = await prisma.userToOrg.findUnique({ - where: { - orgId_userId: { - orgId: org.id, - userId: session.user.id - } - }, - include: { - user: true + const membership = session ? await prisma.userToOrg.findUnique({ + where: { + orgId_userId: { + orgId: org.id, + userId: session.user.id } - }); + }, + include: { + user: true + } + }) : null; + if (session) { // There's two reasons why a user might not be a member of an org: // 1. The org doesn't require member approval, but the org was at max capacity when the user registered. In this case, we show them // the join organization card to allow them to join the org if seat capacity is freed up. This card handles checking if the org has available seats. @@ -197,7 +198,7 @@ export default async function Layout(props: LayoutProps) { {children} - {env.EXPERIMENT_ASK_GH_ENABLED !== 'true' && } + {env.EXPERIMENT_ASK_GH_ENABLED !== 'true' && } ) } \ No newline at end of file