Skip to content

Commit 7502de1

Browse files
JonasBaclaudepriscilawebdev
authored
ref(dynamic-sampling): Migrate projectSampling to new form system (#109360)
Completes the form migration started in the previous PR by replacing the custom `formContext`-based form in `projectSampling` with `useScrapsForm`. **What changed:** - `projectSampling.tsx` — replaces `useFormState`/`FormProvider` with `useScrapsForm`. A `useEffect` on the query data mirrors the old `enableReInitialize: true` behaviour, resetting the form (and `savedProjectRates`) whenever server data arrives or changes. Per-project rate validity is checked inline in the `AppField` render prop to disable the Apply Changes button, since `z.record(z.string(), z.string())` is used for the schema (type safety only — Zod's `ZodEffects` chained on record values isn't assignable to `FormValidateOrFn`). - `projectsEditTable.tsx` — removes the `useFormField('projectRates')` context consumer and instead receives `projectRates`, `savedProjectRates`, and `onProjectRatesChange` as explicit props. Per-project validation errors are now computed locally via `getProjectRateErrors`, keeping the validation logic co-located with the table that displays it. - `utils/projectSamplingForm.tsx` — deleted. - `utils/formContext.tsx` — deleted; this was the shared custom form context, now fully unused. Stacks on top of #109356. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Priscila Oliveira <priscila.oliveira@sentry.io>
1 parent 5d5951a commit 7502de1

File tree

7 files changed

+396
-391
lines changed

7 files changed

+396
-391
lines changed

static/app/views/settings/dynamicSampling/organizationSampling.tsx

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,20 @@ const UNSAVED_CHANGES_MESSAGE = t(
2626
'You have unsaved changes, are you sure you want to leave?'
2727
);
2828

29+
export const sampleRateField = z
30+
.string()
31+
.min(1, t('Please enter a valid number'))
32+
.refine(val => !isNaN(Number(val)), {message: t('Please enter a valid number')})
33+
.refine(
34+
val => {
35+
const n = Number(val);
36+
return n >= 0 && n <= 100;
37+
},
38+
{message: t('Must be between 0% and 100%')}
39+
);
40+
2941
export const targetSampleRateSchema = z.object({
30-
targetSampleRate: z
31-
.string()
32-
.min(1, t('Please enter a valid number'))
33-
.refine(val => !isNaN(Number(val)), {message: t('Please enter a valid number')})
34-
.refine(
35-
val => {
36-
const n = Number(val);
37-
return n >= 0 && n <= 100;
38-
},
39-
{message: t('Must be between 0% and 100%')}
40-
),
42+
targetSampleRate: sampleRateField,
4143
});
4244

4345
export function OrganizationSampling() {
@@ -80,8 +82,8 @@ export function OrganizationSampling() {
8082

8183
return (
8284
<form.AppForm form={form}>
83-
<form.Subscribe selector={s => ({isDirty: s.isDirty, canSubmit: s.canSubmit})}>
84-
{({isDirty, canSubmit}) => (
85+
<form.Subscribe selector={s => ({isDirty: s.isDirty})}>
86+
{({isDirty}) => (
8587
<Fragment>
8688
<OnRouteLeave
8789
message={UNSAVED_CHANGES_MESSAGE}
@@ -121,10 +123,7 @@ export function OrganizationSampling() {
121123
'You do not have permission to update these settings.'
122124
)}
123125
>
124-
<form.SubmitButton
125-
disabled={!hasAccess || !canSubmit}
126-
formNoValidate
127-
>
126+
<form.SubmitButton disabled={!hasAccess} formNoValidate>
128127
{t('Apply Changes')}
129128
</form.SubmitButton>
130129
</Tooltip>
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
import {OrganizationFixture} from 'sentry-fixture/organization';
2+
import {ProjectFixture} from 'sentry-fixture/project';
3+
4+
import {act, render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
5+
6+
import {ProjectsStore} from 'sentry/stores/projectsStore';
7+
8+
import {ProjectSampling} from './projectSampling';
9+
10+
jest.mock('@tanstack/react-virtual', () => ({
11+
useVirtualizer: jest.fn(({count}: {count: number}) => ({
12+
getVirtualItems: jest.fn(() =>
13+
Array.from({length: count}, (_, index) => ({
14+
key: index,
15+
index,
16+
start: index * 63,
17+
size: 63,
18+
}))
19+
),
20+
getTotalSize: jest.fn(() => count * 63),
21+
measure: jest.fn(),
22+
})),
23+
}));
24+
25+
describe('ProjectSampling', () => {
26+
const project = ProjectFixture({id: '1', slug: 'project-slug'});
27+
const organization = OrganizationFixture({
28+
slug: 'org-slug',
29+
access: ['org:write'],
30+
samplingMode: 'project',
31+
});
32+
33+
beforeEach(() => {
34+
MockApiClient.clearMockResponses();
35+
act(() => ProjectsStore.loadInitialData([project]));
36+
37+
MockApiClient.addMockResponse({
38+
url: '/organizations/org-slug/sampling/project-root-counts/',
39+
body: {
40+
data: [
41+
[
42+
{
43+
by: {project: 'project-slug', target_project_id: '1'},
44+
totals: 1000,
45+
series: [],
46+
},
47+
],
48+
],
49+
end: '',
50+
intervals: [],
51+
start: '',
52+
},
53+
});
54+
55+
MockApiClient.addMockResponse({
56+
url: '/organizations/org-slug/sampling/project-rates/',
57+
body: [{id: 1, sampleRate: 0.5}],
58+
});
59+
});
60+
61+
async function waitForProjectRateInput() {
62+
return screen.findByRole('spinbutton', {
63+
name: 'Sample rate for project-slug',
64+
});
65+
}
66+
67+
it('renders project rate inputs with initial values', async () => {
68+
// The input briefly transitions from uncontrolled to controlled as form
69+
// state initializes with the fetched project rates.
70+
jest.spyOn(console, 'error').mockImplementation();
71+
72+
render(<ProjectSampling />, {organization});
73+
74+
const input = await waitForProjectRateInput();
75+
expect(input).toHaveValue(50);
76+
});
77+
78+
it('enables Reset button after changing a project rate', async () => {
79+
render(<ProjectSampling />, {organization});
80+
81+
const input = await waitForProjectRateInput();
82+
expect(screen.getByRole('button', {name: 'Reset'})).toBeDisabled();
83+
84+
await userEvent.clear(input);
85+
await userEvent.type(input, '30');
86+
87+
expect(screen.getByRole('button', {name: 'Reset'})).toBeEnabled();
88+
});
89+
90+
it('resets the input back to the saved value when Reset is clicked', async () => {
91+
render(<ProjectSampling />, {organization});
92+
93+
const input = await waitForProjectRateInput();
94+
await userEvent.clear(input);
95+
await userEvent.type(input, '30');
96+
97+
await userEvent.click(screen.getByRole('button', {name: 'Reset'}));
98+
99+
expect(input).toHaveValue(50);
100+
});
101+
102+
it('shows validation error for empty value on submit', async () => {
103+
render(<ProjectSampling />, {organization});
104+
105+
const input = await waitForProjectRateInput();
106+
await userEvent.clear(input);
107+
await userEvent.click(screen.getByRole('button', {name: 'Apply Changes'}));
108+
109+
expect(await screen.findByText('Please enter a valid number')).toBeInTheDocument();
110+
});
111+
112+
it('calls the API with the correct payload on save', async () => {
113+
const putMock = MockApiClient.addMockResponse({
114+
url: '/organizations/org-slug/sampling/project-rates/',
115+
method: 'PUT',
116+
body: [{id: 1, sampleRate: 0.3}],
117+
});
118+
119+
render(<ProjectSampling />, {organization});
120+
121+
const input = await waitForProjectRateInput();
122+
await userEvent.clear(input);
123+
await userEvent.type(input, '30');
124+
await userEvent.click(screen.getByRole('button', {name: 'Apply Changes'}));
125+
126+
await waitFor(() => {
127+
expect(putMock).toHaveBeenCalledWith(
128+
'/organizations/org-slug/sampling/project-rates/',
129+
expect.objectContaining({data: [{id: 1, sampleRate: 0.3}]})
130+
);
131+
});
132+
});
133+
134+
it('resets form to clean state after a successful save', async () => {
135+
MockApiClient.addMockResponse({
136+
url: '/organizations/org-slug/sampling/project-rates/',
137+
method: 'PUT',
138+
body: [{id: 1, sampleRate: 0.3}],
139+
});
140+
141+
render(<ProjectSampling />, {organization});
142+
143+
const input = await waitForProjectRateInput();
144+
await userEvent.clear(input);
145+
await userEvent.type(input, '30');
146+
await userEvent.click(screen.getByRole('button', {name: 'Apply Changes'}));
147+
148+
await waitFor(() =>
149+
expect(screen.getByRole('button', {name: 'Reset'})).toBeDisabled()
150+
);
151+
});
152+
153+
it('keeps form dirty after an API error', async () => {
154+
MockApiClient.addMockResponse({
155+
url: '/organizations/org-slug/sampling/project-rates/',
156+
method: 'PUT',
157+
statusCode: 500,
158+
body: {detail: 'Internal Server Error'},
159+
});
160+
161+
render(<ProjectSampling />, {organization});
162+
163+
const input = await waitForProjectRateInput();
164+
await userEvent.clear(input);
165+
await userEvent.type(input, '30');
166+
await userEvent.click(screen.getByRole('button', {name: 'Apply Changes'}));
167+
168+
await waitFor(() =>
169+
expect(screen.getByRole('button', {name: 'Reset'})).toBeEnabled()
170+
);
171+
});
172+
173+
it('updates project rates atomically via bulk org rate edit', async () => {
174+
const putMock = MockApiClient.addMockResponse({
175+
url: '/organizations/org-slug/sampling/project-rates/',
176+
method: 'PUT',
177+
body: [{id: 1, sampleRate: 0.8}],
178+
});
179+
180+
render(<ProjectSampling />, {organization});
181+
182+
await waitForProjectRateInput();
183+
184+
// Activate bulk edit mode
185+
await userEvent.click(
186+
screen.getByRole('button', {name: 'Proportionally scale project rates'})
187+
);
188+
189+
// Type a new org rate — this should update all project rates in one atomic call
190+
const orgRateInput = screen.getAllByRole('spinbutton')[0]!;
191+
await userEvent.clear(orgRateInput);
192+
await userEvent.type(orgRateInput, '80');
193+
194+
// The project rate should have been scaled
195+
const projectInput = screen.getByRole('spinbutton', {
196+
name: 'Sample rate for project-slug',
197+
});
198+
expect(projectInput).toHaveValue(80);
199+
200+
// Submit and verify the API call
201+
await userEvent.click(screen.getByRole('button', {name: 'Apply Changes'}));
202+
203+
await waitFor(() => {
204+
expect(putMock).toHaveBeenCalledWith(
205+
'/organizations/org-slug/sampling/project-rates/',
206+
expect.objectContaining({data: [{id: 1, sampleRate: 0.8}]})
207+
);
208+
});
209+
});
210+
211+
it('disables Apply Changes for users without org:write access', async () => {
212+
const orgWithoutAccess = OrganizationFixture({
213+
access: [],
214+
samplingMode: 'project',
215+
});
216+
217+
render(<ProjectSampling />, {organization: orgWithoutAccess});
218+
219+
await waitForProjectRateInput();
220+
expect(screen.getByRole('button', {name: 'Apply Changes'})).toBeDisabled();
221+
});
222+
});

0 commit comments

Comments
 (0)