ref(dynamic-sampling): Migrate projectSampling to new form system#109360
ref(dynamic-sampling): Migrate projectSampling to new form system#109360priscilawebdev merged 19 commits intomasterfrom
Conversation
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
3006dd5 to
be5711c
Compare
…ystem Replace the custom formContext-based form in projectSampling with useScrapsForm from @sentry/scraps/form, completing the migration started for organizationSampling. - Delete projectSamplingForm.tsx and formContext.tsx — now dead code - Rewrite projectSampling.tsx using useScrapsForm with a z.record schema. A useEffect mirrors the old enableReInitialize behaviour, resetting the form whenever the server data changes. Per-project validity is checked inline in the AppField render prop (same logic as getProjectRateErrors) to keep the Apply Changes button disabled when any rate is invalid. - Rewrite projectsEditTable.tsx to receive projectRates, savedProjectRates, and onProjectRatesChange as explicit props instead of pulling from the form context via useFormField. Per-project validation errors are now computed locally via getProjectRateErrors, which keeps the validation logic co-located with the table that displays it. Co-Authored-By: Claude <noreply@anthropic.com>
a197f03 to
f92ccef
Compare
Use AppForm form prop, SubmitButton, canSubmit state, and remove FormWrapper to match the pattern established in organizationSampling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract sampleRateField from organizationSampling and reuse it in projectSamplingSchema via z.record(). This removes the duplicate manual validation (hasProjectRateErrors, getProjectRateErrors) and lets the form system handle submit-blocking through canSubmit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use projectRates as defaultValues instead of an empty object so fields show their current values immediately rather than rendering empty on first paint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Zod schema now handles per-value validation through the form system. Remove the redundant manual error computation that was running on every render regardless of submission state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pass the field error from the form to the table footer so users see feedback when Zod validation blocks submission. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the footer error with per-row validation errors next to each invalid field. Errors only appear after the form has been submitted and Zod validation has flagged the field as invalid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace manual validation checks with sampleRateField.safeParse() so per-row errors use the same Zod schema as the form-level validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the single projectRates record field with per-project
AppField instances. Each row now gets its own field via
form.AppField name={projectRates.${projectId}}, so the form system
handles per-field validation, error display, and canSubmit natively.
Removes the showErrors/getProjectRateError workaround. Bulk edit
uses form.setFieldValue per project instead of updating the record.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3a1b5a9 to
369a758
Compare
Move form awareness out of ProjectsTable into ProjectsEditTable. Read per-field errors from form.state.fieldMeta and pass them via the existing items array. ProjectsTable is now a pure display component again. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the form prop with plain data props (projectRates, projectErrors, onProjectRateChange) so the edit table is a pure presentational component with no form system imports or any types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The submit button should always be clickable so users can trigger validation and see errors. Only disable based on access permissions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover initial render, edit/reset flow, validation on submit, API payload, post-save state, error handling, and access control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d7ef56d to
a4381d4
Compare
Add an accessible label to each project's sample rate input so screen readers can identify which project the input belongs to. Update the test helper to query by this label instead of relying on positional spinbutton indexing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The bulk org rate edit was calling setFieldValue in a loop for each project, triggering N separate validations. Use a single atomic update for all project rates instead, matching the old behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When there are no span counts for the selected period, the division by zero produces NaN which renders as a blank number input. Default to 0% when totalSpanCount is zero. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
priscilawebdev
left a comment
There was a problem hiding this comment.
I did manual testing, and everything looks good
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| sampleRate: value[project.id]!, | ||
| error: error?.[project.id], | ||
| initialSampleRate: savedProjectRates[project.id]!, | ||
| sampleRate: projectRates[project.id]!, |
There was a problem hiding this comment.
Non-null assertions on potentially undefined project rates
Low Severity
savedProjectRates[project.id]! and projectRates[project.id]! use non-null assertions but can be undefined during the transition between data loading and the useEffect resetting the form. savedProjectRates is initialized via useState(projectRates) where projectRates is {} on first render, and is only populated after the useEffect fires — one render frame after isLoading becomes false. This causes inputs to receive undefined as their value prop, triggering React's uncontrolled-to-controlled warning (acknowledged and suppressed in the test). Using ?? '' instead of ! would provide a safe fallback and eliminate the transition.


Completes the form migration started in the previous PR by replacing the
custom
formContext-based form inprojectSamplingwithuseScrapsForm.What changed:
projectSampling.tsx— replacesuseFormState/FormProviderwithuseScrapsForm. AuseEffecton the query data mirrors the oldenableReInitialize: truebehaviour, resetting the form (andsavedProjectRates) whenever server data arrives or changes. Per-project rate validity is checked inline in theAppFieldrender prop to disable the Apply Changes button, sincez.record(z.string(), z.string())is used for the schema (type safety only — Zod'sZodEffectschained on record values isn't assignable toFormValidateOrFn).projectsEditTable.tsx— removes theuseFormField('projectRates')context consumer and instead receivesprojectRates,savedProjectRates, andonProjectRatesChangeas explicit props. Per-project validation errors are now computed locally viagetProjectRateErrors, 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.