Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 90 additions & 1 deletion static/gsAdmin/components/changeBalanceAction.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import {OrganizationFixture} from 'sentry-fixture/organization';

import {SubscriptionFixture} from 'getsentry-test/fixtures/subscription';
import {renderGlobalModal, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import {
renderGlobalModal,
screen,
userEvent,
waitFor,
} from 'sentry-test/reactTestingLibrary';

import triggerChangeBalanceModal from 'admin/components/changeBalanceAction';

Expand Down Expand Up @@ -107,4 +112,88 @@ describe('BalanceChangeAction', () => {
})
);
});

it('prevents double submission', async () => {
const updateMock = MockApiClient.addMockResponse({
url: `/_admin/customers/${organization.slug}/balance-changes/`,
method: 'POST',
body: OrganizationFixture(),
});

triggerChangeBalanceModal({subscription, ...modalProps});

renderGlobalModal();
await userEvent.type(screen.getByRole('spinbutton', {name: 'Credit Amount'}), '10');

const submitButton = screen.getByRole('button', {name: 'Submit'});

// Rapidly click submit button multiple times
await userEvent.click(submitButton);
await userEvent.click(submitButton);
await userEvent.click(submitButton);

// Should only call API once (double-clicks prevented)
expect(updateMock).toHaveBeenCalledTimes(1);
});

it('disables form fields during submission', async () => {
// Mock with delay to keep isSubmitting true during test
Copy link
Contributor

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

MockApiClient.addMockResponse({
url: `/_admin/customers/${organization.slug}/balance-changes/`,
method: 'POST',
body: OrganizationFixture(),
asyncDelay: 100,
});

triggerChangeBalanceModal({subscription, ...modalProps});

renderGlobalModal();
const creditInput = screen.getByRole('spinbutton', {name: 'Credit Amount'});
await userEvent.type(creditInput, '10');

const submitButton = screen.getByRole('button', {name: 'Submit'});
await userEvent.click(submitButton);

// During submission, button should show "Submitting...", be disabled, and fields should be disabled
expect(submitButton).toHaveTextContent('Submitting...');
expect(submitButton).toBeDisabled();
expect(creditInput).toBeDisabled();
expect(screen.getByTestId('url-field')).toBeDisabled();
expect(screen.getByTestId('notes-field')).toBeDisabled();
});

it('re-enables form after error', async () => {
MockApiClient.addMockResponse({
url: `/_admin/customers/${organization.slug}/balance-changes/`,
method: 'POST',
statusCode: 400,
body: {detail: 'Invalid amount'},
asyncDelay: 10,
});

triggerChangeBalanceModal({subscription, ...modalProps});
renderGlobalModal();

// Pre-grab stable references to fields using findBy to wait for modal content
const creditInput = await screen.findByRole('spinbutton', {name: 'Credit Amount'});
const urlField = await screen.findByTestId('url-field');
const notesField = await screen.findByTestId('notes-field');
const submitButton = screen.getByRole('button', {name: /submit/i});

await userEvent.type(creditInput, '10');
await waitFor(() => expect(creditInput).toHaveValue(10));

await userEvent.click(submitButton);

// Wait for form to be re-enabled after error
// Don't rely on error message text as the Form component shows different messages
// depending on error response structure. All fields are controlled by isSubmitting
// state, so if one is enabled, all should be enabled.
await waitFor(() => expect(creditInput).toBeEnabled());

// Verify all fields and submit button are re-enabled
expect(urlField).toBeEnabled();
expect(notesField).toBeEnabled();
expect(submitButton).toBeEnabled();
});
});
15 changes: 14 additions & 1 deletion static/gsAdmin/components/changeBalanceAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function ChangeBalanceModal({
}: ModalProps) {
const [ticketUrl, setTicketUrl] = useState('');
const [notes, setNotes] = useState('');
const [isSubmitting, setIsSubmitting] = useState(false);
const api = useApi();

function coerceValue(value: number) {
Expand All @@ -47,6 +48,12 @@ function ChangeBalanceModal({
return;
}

// Prevent concurrent submissions
if (isSubmitting) {
return;
}

setIsSubmitting(true);
try {
await api.requestPromise(`/_admin/customers/${orgId}/balance-changes/`, {
method: 'POST',
Expand All @@ -60,6 +67,8 @@ function ChangeBalanceModal({
onSubmitError({
responseJSON: err.responseJSON,
});
} finally {
setIsSubmitting(false);
}
}

Expand All @@ -76,14 +85,16 @@ function ChangeBalanceModal({
<Form
onSubmit={onSubmit}
onCancel={closeModal}
Copy link
Contributor

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

submitLabel="Submit"
submitLabel={isSubmitting ? 'Submitting...' : 'Submit'}
submitDisabled={isSubmitting}
cancelLabel="Cancel"
footerClass="modal-footer"
>
<NumberField
label="Credit Amount"
name="creditAmount"
help="Add or remove credit, in dollars"
disabled={isSubmitting}
/>
<AuditFields>
<InputField
Expand All @@ -94,6 +105,7 @@ function ChangeBalanceModal({
inline={false}
stacked
flexibleControlStateSize
disabled={isSubmitting}
onChange={(ticketUrlInput: any) => setTicketUrl(ticketUrlInput)}
/>
<TextField
Expand All @@ -104,6 +116,7 @@ function ChangeBalanceModal({
stacked
flexibleControlStateSize
maxLength={500}
disabled={isSubmitting}
onChange={(notesInput: any) => setNotes(notesInput)}
/>
</AuditFields>
Expand Down
Loading