-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(billing): Disable submission form button when adding/removing credits #100807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #100807 +/- ##
========================================
Coverage 80.90% 80.90%
========================================
Files 8821 8821
Lines 389819 389819
Branches 24795 24795
========================================
Hits 315365 315365
Misses 74092 74092
Partials 362 362 |
4b1b42a to
f5f5346
Compare
f5f5346 to
a9c4e27
Compare
a9c4e27 to
4c9afa9
Compare
019d205 to
a97aaf6
Compare
2b0c610 to
944440a
Compare
944440a to
121d7ef
Compare
| }); | ||
|
|
||
| it('disables form fields during submission', async () => { | ||
| // Mock with delay to keep isSubmitting true during test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might not need many of these comments imo
| </p> | ||
| <Form | ||
| onSubmit={onSubmit} | ||
| onCancel={closeModal} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a submitDisabled prop too if we want to add that prop here as well
| submitDisabled: false, |
ajay-sentry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing blocking, just a couple thoughts
Add submitDisabled prop to Form component to properly disable the submit button during form submission, utilizing the built-in Form API. Also update tests to verify submit button disabled state.
121d7ef to
7c8833b
Compare
The PR I merged recently #100807 introduced a flaky frontend test: ``` Summary of all failing tests FAIL static/gsAdmin/components/changeBalanceAction.spec.tsx ● BalanceChangeAction › re-enables form after error Unable to perform pointer interaction as the element inherits `pointer-events: none`: DIV <-- This element declared `pointer-events: none` DIV DIV SECTION FORM DIV BUTTON(label=Submit) <-- Asserted pointer events here 184 | await waitFor(() => expect(creditInput).toHaveValue(10)); 185 | > 186 | await userEvent.click(submitButton); | ^ 187 | 188 | // Wait for form to be re-enabled after error 189 | // Don't rely on error message text as the Form component shows different messages at Object.assertPointerEvents (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/utils/pointer/cssPointerEvents.js:47:15) at Object.enter (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/pointer.js:52:34) at PointerHost.move (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/index.js:53:85) at pointerAction (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:59:39) at Object.pointer (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:27:15) at Object.asyncWrapper (node_modules/.pnpm/@testing-library+react@16.2.0_@testing-library+dom@10.4.0_@types+react-dom@19.2.0_@type_011f94990cdc27509fa142ae9e3c3bf5/node_modules/@testing-library/react/dist/pure.js:88:22) at Object.<anonymous> (static/gsAdmin/components/changeBalanceAction.spec.tsx:186:5) Test Suites: 1 failed, 407 passed, 408 total Tests: 1 failed, 1 skipped, 3040 passed, 3042 total Snapshots: 2 passed, 2 total Time: 266.043 s Ran all test suites. Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished? ELIFECYCLE Command failed with exit code 1. Error: Process completed with exit code 1. ``` This PR should hopefully address this flakiness by: 1. waiting for button to be enabled 2. and by disabling the pointer-events check (similarly done in #95798)
The PR I merged recently #100807 introduced a flaky frontend test: ``` Summary of all failing tests FAIL static/gsAdmin/components/changeBalanceAction.spec.tsx ● BalanceChangeAction › re-enables form after error Unable to perform pointer interaction as the element inherits `pointer-events: none`: DIV <-- This element declared `pointer-events: none` DIV DIV SECTION FORM DIV BUTTON(label=Submit) <-- Asserted pointer events here 184 | await waitFor(() => expect(creditInput).toHaveValue(10)); 185 | > 186 | await userEvent.click(submitButton); | ^ 187 | 188 | // Wait for form to be re-enabled after error 189 | // Don't rely on error message text as the Form component shows different messages at Object.assertPointerEvents (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/utils/pointer/cssPointerEvents.js:47:15) at Object.enter (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/pointer.js:52:34) at PointerHost.move (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/index.js:53:85) at pointerAction (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:59:39) at Object.pointer (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:27:15) at Object.asyncWrapper (node_modules/.pnpm/@testing-library+react@16.2.0_@testing-library+dom@10.4.0_@types+react-dom@19.2.0_@type_011f94990cdc27509fa142ae9e3c3bf5/node_modules/@testing-library/react/dist/pure.js:88:22) at Object.<anonymous> (static/gsAdmin/components/changeBalanceAction.spec.tsx:186:5) Test Suites: 1 failed, 407 passed, 408 total Tests: 1 failed, 1 skipped, 3040 passed, 3042 total Snapshots: 2 passed, 2 total Time: 266.043 s Ran all test suites. Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished? ELIFECYCLE Command failed with exit code 1. Error: Process completed with exit code 1. ``` This PR should hopefully address this flakiness by: 1. waiting for button to be enabled 2. and by disabling the pointer-events check (similarly done in #95798)
…edits (#100807) Closes https://linear.app/getsentry/issue/BIL-232/credit-to-a-subscription-can-sometimes-get-duplicated-via-admin Disable submission form button when adding/removing credits. This should prevent accidental double-clicks to add multiple credits.
The PR I merged recently #100807 introduced a flaky frontend test: ``` Summary of all failing tests FAIL static/gsAdmin/components/changeBalanceAction.spec.tsx ● BalanceChangeAction › re-enables form after error Unable to perform pointer interaction as the element inherits `pointer-events: none`: DIV <-- This element declared `pointer-events: none` DIV DIV SECTION FORM DIV BUTTON(label=Submit) <-- Asserted pointer events here 184 | await waitFor(() => expect(creditInput).toHaveValue(10)); 185 | > 186 | await userEvent.click(submitButton); | ^ 187 | 188 | // Wait for form to be re-enabled after error 189 | // Don't rely on error message text as the Form component shows different messages at Object.assertPointerEvents (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/utils/pointer/cssPointerEvents.js:47:15) at Object.enter (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/pointer.js:52:34) at PointerHost.move (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/system/pointer/index.js:53:85) at pointerAction (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:59:39) at Object.pointer (node_modules/.pnpm/@testing-library+user-event@14.6.1_@testing-library+dom@10.4.0/node_modules/@testing-library/user-event/dist/cjs/pointer/index.js:27:15) at Object.asyncWrapper (node_modules/.pnpm/@testing-library+react@16.2.0_@testing-library+dom@10.4.0_@types+react-dom@19.2.0_@type_011f94990cdc27509fa142ae9e3c3bf5/node_modules/@testing-library/react/dist/pure.js:88:22) at Object.<anonymous> (static/gsAdmin/components/changeBalanceAction.spec.tsx:186:5) Test Suites: 1 failed, 407 passed, 408 total Tests: 1 failed, 1 skipped, 3040 passed, 3042 total Snapshots: 2 passed, 2 total Time: 266.043 s Ran all test suites. Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished? ELIFECYCLE Command failed with exit code 1. Error: Process completed with exit code 1. ``` This PR should hopefully address this flakiness by: 1. waiting for button to be enabled 2. and by disabling the pointer-events check (similarly done in #95798)
Closes https://linear.app/getsentry/issue/BIL-232/credit-to-a-subscription-can-sometimes-get-duplicated-via-admin
Disable submission form button when adding/removing credits. This should prevent accidental double-clicks to add multiple credits.