Skip to content

Commit 94eb598

Browse files
ksg98claude
andcommitted
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) <noreply@anthropic.com>
1 parent e0d4ff9 commit 94eb598

File tree

3 files changed

+146
-17
lines changed

3 files changed

+146
-17
lines changed
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { describe, expect, test, vi, beforeEach } from 'vitest'
2+
import { getVersionFromString, getVersionString, compareVersions } from './upgradeToast';
3+
4+
// --- Pure utility function tests ---
5+
6+
describe('getVersionFromString', () => {
7+
test('parses a valid semver string', () => {
8+
expect(getVersionFromString('v1.2.3')).toEqual({ major: 1, minor: 2, patch: 3 });
9+
});
10+
11+
test('returns null for invalid version strings', () => {
12+
expect(getVersionFromString('not-a-version')).toBeNull();
13+
expect(getVersionFromString('1.2.3')).toBeNull(); // missing v prefix
14+
expect(getVersionFromString('v1.2')).toBeNull(); // missing patch
15+
expect(getVersionFromString('v1.2.3-beta')).toBeNull(); // pre-release suffix
16+
});
17+
18+
test('parses zero versions', () => {
19+
expect(getVersionFromString('v0.0.0')).toEqual({ major: 0, minor: 0, patch: 0 });
20+
});
21+
});
22+
23+
describe('getVersionString', () => {
24+
test('formats a version object as a string', () => {
25+
expect(getVersionString({ major: 1, minor: 2, patch: 3 })).toBe('v1.2.3');
26+
});
27+
28+
test('formats zero version', () => {
29+
expect(getVersionString({ major: 0, minor: 0, patch: 0 })).toBe('v0.0.0');
30+
});
31+
});
32+
33+
describe('compareVersions', () => {
34+
test('returns 0 for equal versions', () => {
35+
const v = { major: 1, minor: 2, patch: 3 };
36+
expect(compareVersions(v, v)).toBe(0);
37+
});
38+
39+
test('compares by major version first', () => {
40+
const a = { major: 2, minor: 0, patch: 0 };
41+
const b = { major: 1, minor: 9, patch: 9 };
42+
expect(compareVersions(a, b)).toBeGreaterThan(0);
43+
expect(compareVersions(b, a)).toBeLessThan(0);
44+
});
45+
46+
test('compares by minor version when major is equal', () => {
47+
const a = { major: 1, minor: 3, patch: 0 };
48+
const b = { major: 1, minor: 2, patch: 9 };
49+
expect(compareVersions(a, b)).toBeGreaterThan(0);
50+
expect(compareVersions(b, a)).toBeLessThan(0);
51+
});
52+
53+
test('compares by patch version when major and minor are equal', () => {
54+
const a = { major: 1, minor: 2, patch: 4 };
55+
const b = { major: 1, minor: 2, patch: 3 };
56+
expect(compareVersions(a, b)).toBeGreaterThan(0);
57+
expect(compareVersions(b, a)).toBeLessThan(0);
58+
});
59+
});
60+
61+
// --- UpgradeToast isOwner gating test ---
62+
63+
// We mock the external dependencies to isolate the isOwner behavior.
64+
// The key assertion: when isOwner=false, fetch should NOT be called.
65+
66+
const mockToast = vi.fn();
67+
vi.mock('@/components/hooks/use-toast', () => ({
68+
useToast: () => ({ toast: mockToast }),
69+
}));
70+
71+
vi.mock('usehooks-ts', () => ({
72+
useLocalStorage: () => [new Date(0).toUTCString(), vi.fn()],
73+
}));
74+
75+
vi.mock('@tanstack/react-query', () => ({
76+
useQuery: () => ({ data: 'v1.0.0' }),
77+
}));
78+
79+
vi.mock('@/app/api/(client)/client', () => ({
80+
getVersion: vi.fn(),
81+
}));
82+
83+
describe('UpgradeToast isOwner gating', () => {
84+
beforeEach(() => {
85+
vi.restoreAllMocks();
86+
// Reset the global fetch mock before each test
87+
global.fetch = vi.fn();
88+
});
89+
90+
test('does not fetch or show toast when isOwner is false', async () => {
91+
// Dynamic import after mocks are set up
92+
const { UpgradeToast } = await import('./upgradeToast');
93+
const { render } = await import('@testing-library/react');
94+
95+
render(<UpgradeToast isOwner={false} />);
96+
97+
// fetch should not have been called because isOwner is false
98+
expect(global.fetch).not.toHaveBeenCalled();
99+
expect(mockToast).not.toHaveBeenCalled();
100+
});
101+
102+
test('fetches GitHub tags when isOwner is true', async () => {
103+
const mockResponse = {
104+
json: () => Promise.resolve([{ name: 'v2.0.0' }]),
105+
};
106+
global.fetch = vi.fn().mockResolvedValue(mockResponse);
107+
108+
const { UpgradeToast } = await import('./upgradeToast');
109+
const { render, waitFor } = await import('@testing-library/react');
110+
111+
render(<UpgradeToast isOwner={true} />);
112+
113+
await waitFor(() => {
114+
expect(global.fetch).toHaveBeenCalledWith(
115+
'https://api.github.com/repos/sourcebot-dev/sourcebot/tags'
116+
);
117+
});
118+
});
119+
});

packages/web/src/app/[domain]/components/upgradeToast.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ type Version = {
1717
patch: number;
1818
};
1919

20-
export const UpgradeToast = () => {
20+
interface UpgradeToastProps {
21+
isOwner: boolean;
22+
}
23+
24+
export const UpgradeToast = ({ isOwner }: UpgradeToastProps) => {
2125
const { toast } = useToast();
2226
const [ upgradeToastLastShownDate, setUpgradeToastLastShownDate ] = useLocalStorage<string>(
2327
"upgradeToastLastShownDate",
@@ -28,9 +32,14 @@ export const UpgradeToast = () => {
2832
queryKey: ["version"],
2933
queryFn: () => getVersion(),
3034
select: (data) => data.version,
35+
enabled: isOwner,
3136
})
3237

3338
useEffect(() => {
39+
if (!isOwner) {
40+
return;
41+
}
42+
3443
if (!versionString) {
3544
return;
3645
}
@@ -82,12 +91,12 @@ export const UpgradeToast = () => {
8291

8392
setUpgradeToastLastShownDate(new Date().toUTCString());
8493
});
85-
}, [setUpgradeToastLastShownDate, toast, upgradeToastLastShownDate, versionString]);
94+
}, [isOwner, setUpgradeToastLastShownDate, toast, upgradeToastLastShownDate, versionString]);
8695

8796
return null;
8897
}
8998

90-
const getVersionFromString = (version: string): Version | null => {
99+
export const getVersionFromString = (version: string): Version | null => {
91100
const match = version.match(SEMVER_REGEX);
92101
if (!match) {
93102
return null;
@@ -99,11 +108,11 @@ const getVersionFromString = (version: string): Version | null => {
99108
} satisfies Version;
100109
}
101110

102-
const getVersionString = (version: Version) => {
111+
export const getVersionString = (version: Version) => {
103112
return `v${version.major}.${version.minor}.${version.patch}`;
104113
}
105114

106-
const compareVersions = (a: Version, b: Version) => {
115+
export const compareVersions = (a: Version, b: Version) => {
107116
if (a.major !== b.major) {
108117
return a.major - b.major;
109118
}

packages/web/src/app/[domain]/layout.tsx

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { JoinOrganizationCard } from "@/app/components/joinOrganizationCard";
2323
import { LogoutEscapeHatch } from "@/app/components/logoutEscapeHatch";
2424
import { GitHubStarToast } from "./components/githubStarToast";
2525
import { UpgradeToast } from "./components/upgradeToast";
26+
import { OrgRole } from "@sourcebot/db";
2627
import { getLinkedAccounts } from "@/ee/features/sso/actions";
2728
import { PermissionSyncBanner } from "./components/permissionSyncBanner";
2829
import { getPermissionSyncStatus } from "../api/(server)/ee/permissionSyncStatus/api";
@@ -66,19 +67,19 @@ export default async function Layout(props: LayoutProps) {
6667
})();
6768

6869
// If the user is authenticated, we must check if they're a member of the org
69-
if (session) {
70-
const membership = await prisma.userToOrg.findUnique({
71-
where: {
72-
orgId_userId: {
73-
orgId: org.id,
74-
userId: session.user.id
75-
}
76-
},
77-
include: {
78-
user: true
70+
const membership = session ? await prisma.userToOrg.findUnique({
71+
where: {
72+
orgId_userId: {
73+
orgId: org.id,
74+
userId: session.user.id
7975
}
80-
});
76+
},
77+
include: {
78+
user: true
79+
}
80+
}) : null;
8181

82+
if (session) {
8283
// There's two reasons why a user might not be a member of an org:
8384
// 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
8485
// 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) {
197198
{children}
198199
<SyntaxReferenceGuide />
199200
<GitHubStarToast />
200-
{env.EXPERIMENT_ASK_GH_ENABLED !== 'true' && <UpgradeToast />}
201+
{env.EXPERIMENT_ASK_GH_ENABLED !== 'true' && <UpgradeToast isOwner={membership?.role === OrgRole.OWNER} />}
201202
</SyntaxGuideProvider>
202203
)
203204
}

0 commit comments

Comments
 (0)