Skip to content

Commit f8bf6f7

Browse files
cezudascezudas
authored
Update frontend to respect benchmark permissions (#2106)
Fixes OPS-3874.
1 parent 250d5a1 commit f8bf6f7

4 files changed

Lines changed: 172 additions & 16 deletions

File tree

‎packages/react-ui/src/app/features/home/components/useBenchmarkBannerState.test.ts‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@ describe('useBenchmarkBannerState', () => {
3838
mockListBenchmarks.mockResolvedValue([]);
3939
});
4040

41-
it('returns isEnabled false and default variation when flag is disabled', () => {
42-
mockUseShowBenchmarkBanner.mockReturnValue(false);
41+
it('returns isEnabled false and default variation when banner should not be shown', () => {
42+
mockUseShowBenchmarkBanner.mockReturnValue({
43+
showBanner: false,
44+
isPending: false,
45+
});
4346
setupQueryMock(undefined);
4447

4548
const { result } = renderHook(() => useBenchmarkBannerState());
@@ -49,8 +52,11 @@ describe('useBenchmarkBannerState', () => {
4952
expect(result.current.provider).toBe(undefined);
5053
});
5154

52-
it('does not call listBenchmarks when flag is disabled', () => {
53-
mockUseShowBenchmarkBanner.mockReturnValue(false);
55+
it('does not call listBenchmarks when banner should not be shown', () => {
56+
mockUseShowBenchmarkBanner.mockReturnValue({
57+
showBanner: false,
58+
isPending: false,
59+
});
5460
mockUseQuery.mockImplementation(({ enabled }) => {
5561
if (!enabled) return { data: undefined };
5662
benchmarkApi.listBenchmarks();
@@ -62,17 +68,23 @@ describe('useBenchmarkBannerState', () => {
6268
expect(mockListBenchmarks).not.toHaveBeenCalled();
6369
});
6470

65-
it('calls listBenchmarks when flag is enabled', () => {
66-
mockUseShowBenchmarkBanner.mockReturnValue(true);
71+
it('calls listBenchmarks when banner should be shown', () => {
72+
mockUseShowBenchmarkBanner.mockReturnValue({
73+
showBanner: true,
74+
isPending: false,
75+
});
6776
setupQueryMock([]);
6877

6978
renderHook(() => useBenchmarkBannerState());
7079

7180
expect(mockListBenchmarks).toHaveBeenCalledTimes(1);
7281
});
7382

74-
it('returns default variation when flag is enabled but no benchmarks exist', () => {
75-
mockUseShowBenchmarkBanner.mockReturnValue(true);
83+
it('returns default variation when banner is shown but no benchmarks exist', () => {
84+
mockUseShowBenchmarkBanner.mockReturnValue({
85+
showBanner: true,
86+
isPending: false,
87+
});
7688
setupQueryMock([]);
7789

7890
const { result } = renderHook(() => useBenchmarkBannerState());
@@ -82,8 +94,11 @@ describe('useBenchmarkBannerState', () => {
8294
expect(result.current.provider).toBe(undefined);
8395
});
8496

85-
it('returns default variation when flag is enabled but benchmark is RUNNING', () => {
86-
mockUseShowBenchmarkBanner.mockReturnValue(true);
97+
it('returns default variation when banner is shown but benchmark is RUNNING', () => {
98+
mockUseShowBenchmarkBanner.mockReturnValue({
99+
showBanner: true,
100+
isPending: false,
101+
});
87102
setupQueryMock([
88103
{ benchmarkId: 'bm-1', provider: 'aws', status: BenchmarkStatus.RUNNING },
89104
]);
@@ -94,7 +109,10 @@ describe('useBenchmarkBannerState', () => {
94109
});
95110

96111
it('returns default variation when benchmark is CREATED', () => {
97-
mockUseShowBenchmarkBanner.mockReturnValue(true);
112+
mockUseShowBenchmarkBanner.mockReturnValue({
113+
showBanner: true,
114+
isPending: false,
115+
});
98116
setupQueryMock([
99117
{ benchmarkId: 'bm-1', provider: 'aws', status: BenchmarkStatus.CREATED },
100118
]);
@@ -105,7 +123,10 @@ describe('useBenchmarkBannerState', () => {
105123
});
106124

107125
it('returns default variation when benchmark has FAILED status', () => {
108-
mockUseShowBenchmarkBanner.mockReturnValue(true);
126+
mockUseShowBenchmarkBanner.mockReturnValue({
127+
showBanner: true,
128+
isPending: false,
129+
});
109130
setupQueryMock([
110131
{ benchmarkId: 'bm-1', provider: 'aws', status: BenchmarkStatus.FAILED },
111132
]);
@@ -116,7 +137,10 @@ describe('useBenchmarkBannerState', () => {
116137
});
117138

118139
it('returns report variation and aws provider when aws benchmark has SUCCEEDED', () => {
119-
mockUseShowBenchmarkBanner.mockReturnValue(true);
140+
mockUseShowBenchmarkBanner.mockReturnValue({
141+
showBanner: true,
142+
isPending: false,
143+
});
120144
setupQueryMock([
121145
{
122146
benchmarkId: 'bm-1',
@@ -132,7 +156,10 @@ describe('useBenchmarkBannerState', () => {
132156
});
133157

134158
it('returns report variation when there are multiple benchmarks and one has SUCCEEDED', () => {
135-
mockUseShowBenchmarkBanner.mockReturnValue(true);
159+
mockUseShowBenchmarkBanner.mockReturnValue({
160+
showBanner: true,
161+
isPending: false,
162+
});
136163
setupQueryMock([
137164
{ benchmarkId: 'bm-1', provider: 'aws', status: BenchmarkStatus.RUNNING },
138165
{

‎packages/react-ui/src/app/features/home/components/useBenchmarkBannerState.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ type BenchmarkBannerState = {
1111
};
1212

1313
export const useBenchmarkBannerState = (): BenchmarkBannerState => {
14-
const isEnabled = useShowBenchmarkBanner();
14+
const { showBanner: isEnabled } = useShowBenchmarkBanner();
1515

1616
const { data: benchmarks } = useQuery({
1717
queryKey: [QueryKeys.benchmarks],

‎packages/react-ui/src/app/features/home/components/useShowBenchmarkBanner.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { flagsHooks } from '@/app/common/hooks/flags-hooks';
2+
import { userHooks } from '@/app/common/hooks/user-hooks';
3+
import { renderHook } from '@testing-library/react';
4+
import { useShowBenchmarkBanner } from './useShowBenchmarkBanner';
5+
6+
jest.mock('@/app/common/hooks/flags-hooks', () => ({
7+
flagsHooks: {
8+
useFlag: jest.fn(),
9+
},
10+
}));
11+
12+
jest.mock('@/app/common/hooks/user-hooks', () => ({
13+
userHooks: {
14+
useUserMeta: jest.fn(),
15+
},
16+
}));
17+
18+
const mockUseFlag = flagsHooks.useFlag as jest.Mock;
19+
const mockUseUserMeta = userHooks.useUserMeta as jest.Mock;
20+
21+
describe('useShowBenchmarkBanner', () => {
22+
beforeEach(() => {
23+
jest.clearAllMocks();
24+
});
25+
26+
it('should return showBanner false when flag is disabled', () => {
27+
mockUseFlag.mockReturnValue({ data: false });
28+
mockUseUserMeta.mockReturnValue({
29+
userMeta: {
30+
projectPermissions: {
31+
benchmark: true,
32+
},
33+
},
34+
isPending: false,
35+
});
36+
37+
const { result } = renderHook(() => useShowBenchmarkBanner());
38+
39+
expect(result.current.showBanner).toBe(false);
40+
expect(result.current.isPending).toBe(false);
41+
});
42+
43+
it('should return showBanner false when user has no benchmark access', () => {
44+
mockUseFlag.mockReturnValue({ data: true });
45+
mockUseUserMeta.mockReturnValue({
46+
userMeta: {
47+
projectPermissions: {
48+
benchmark: false,
49+
},
50+
},
51+
isPending: false,
52+
});
53+
54+
const { result } = renderHook(() => useShowBenchmarkBanner());
55+
56+
expect(result.current.showBanner).toBe(false);
57+
expect(result.current.isPending).toBe(false);
58+
});
59+
60+
it('should return showBanner false when userMeta is undefined', () => {
61+
mockUseFlag.mockReturnValue({ data: true });
62+
mockUseUserMeta.mockReturnValue({
63+
userMeta: undefined,
64+
isPending: false,
65+
});
66+
67+
const { result } = renderHook(() => useShowBenchmarkBanner());
68+
69+
expect(result.current.showBanner).toBe(false);
70+
expect(result.current.isPending).toBe(false);
71+
});
72+
73+
it('should return showBanner true when flag is enabled and user has benchmark access', () => {
74+
mockUseFlag.mockReturnValue({ data: true });
75+
mockUseUserMeta.mockReturnValue({
76+
userMeta: {
77+
projectPermissions: {
78+
benchmark: true,
79+
},
80+
},
81+
isPending: false,
82+
});
83+
84+
const { result } = renderHook(() => useShowBenchmarkBanner());
85+
86+
expect(result.current.showBanner).toBe(true);
87+
expect(result.current.isPending).toBe(false);
88+
});
89+
90+
it('should handle isPending from userMeta', () => {
91+
mockUseFlag.mockReturnValue({ data: true });
92+
mockUseUserMeta.mockReturnValue({
93+
userMeta: undefined,
94+
isPending: true,
95+
});
96+
97+
const { result } = renderHook(() => useShowBenchmarkBanner());
98+
99+
expect(result.current.showBanner).toBe(false);
100+
expect(result.current.isPending).toBe(true);
101+
});
102+
103+
it('should handle undefined flag value', () => {
104+
mockUseFlag.mockReturnValue({ data: undefined });
105+
mockUseUserMeta.mockReturnValue({
106+
userMeta: {
107+
projectPermissions: {
108+
benchmark: true,
109+
},
110+
},
111+
isPending: false,
112+
});
113+
114+
const { result } = renderHook(() => useShowBenchmarkBanner());
115+
116+
expect(result.current.showBanner).toBe(false);
117+
});
118+
});

‎packages/react-ui/src/app/features/home/components/useShowBenchmarkBanner.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
import { flagsHooks } from '@/app/common/hooks/flags-hooks';
2+
import { userHooks } from '@/app/common/hooks/user-hooks';
23
import { FlagId } from '@openops/shared';
4+
import { useMemo } from 'react';
35

46
export const useShowBenchmarkBanner = () => {
57
const { data: isFinOpsBenchmarkEnabled } = flagsHooks.useFlag<
68
boolean | undefined
79
>(FlagId.FINOPS_BENCHMARK_ENABLED);
810

9-
return isFinOpsBenchmarkEnabled ?? false;
11+
const { userMeta, isPending } = userHooks.useUserMeta();
12+
const hasBenchmarkAccess = userMeta?.projectPermissions?.benchmark ?? false;
13+
14+
return useMemo(
15+
() => ({
16+
showBanner: Boolean(isFinOpsBenchmarkEnabled && hasBenchmarkAccess),
17+
isPending,
18+
}),
19+
[isFinOpsBenchmarkEnabled, hasBenchmarkAccess, isPending],
20+
);
1021
};

0 commit comments

Comments
 (0)