Skip to content
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

fix: [M3-6540] - Surface general errors in the Object Storage Bucket Create Drawer #9067

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import * as React from 'react';
import { CreateBucketDrawer } from './CreateBucketDrawer';
import { waitFor } from '@testing-library/react';
import { renderWithTheme } from 'src/utilities/testHelpers';
import userEvent from '@testing-library/user-event';
import { rest, server } from 'src/mocks/testServer';
import { makeResourcePage } from 'src/mocks/serverHandlers';
import {
accountSettingsFactory,
objectStorageClusterFactory,
regionFactory,
} from 'src/factories';

const props = {
isOpen: true,
onClose: jest.fn(),
};

jest.mock('src/components/EnhancedSelect/Select');

describe('CreateBucketDrawer', () => {
it('Should show a general error notice if the API returns one', async () => {
server.use(
rest.post('*/object-storage/buckets', (req, res, ctx) => {
return res(
ctx.status(500),
ctx.json({ errors: [{ reason: 'Object Storage is offline!' }] })
);
}),
rest.get('*/regions', async (req, res, ctx) => {
return res(
ctx.json(
makeResourcePage(
regionFactory.buildList(1, { id: 'us-east', label: 'Newark, NJ' })
)
)
);
}),
rest.get('*object-storage/clusters', (req, res, ctx) => {
return res(
ctx.json(
makeResourcePage(
objectStorageClusterFactory.buildList(1, {
region: 'us-east',
id: 'us-east-1',
})
)
)
);
}),
rest.get('*/account/settings', (req, res, ctx) => {
return res(
ctx.json(accountSettingsFactory.build({ object_storage: 'active' }))
);
})
);

const {
getByTestId,
getByLabelText,
getByPlaceholderText,
findByText,
} = renderWithTheme(<CreateBucketDrawer {...props} />);

userEvent.type(getByLabelText('Label'), 'my-test-bucket');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't those be wrapped in an act()?

Copy link
Member Author

@bnussman-akamai bnussman-akamai May 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I wrap userEvent.type(getByLabelText('Label'), 'my-test-bucket'); in an act, the underlying input does not update properly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something's odd - lemme try locally

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where I'm stuck. Something about the mock error isn't correct but I don't know what it is.

Screenshot 2023-05-01 at 5 37 48 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am not getting that error by doing something like this:

const labelField = getByTestId('textfield-input');
const saveButton = getByTestId('create-bucket-button');
await act(async () => {
  userEvent.type(labelField, 'my-test-bucket');
  await waitFor(() =>
    userEvent.selectOptions(
      getByPlaceholderText('Select a Region'),
      'Newark, NJ (us-east-1)'
    )
  );
  userEvent.click(saveButton);
});

await findByText('something bad happened!');

but it does not find the element. am confused as to what's going on here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind this is erroneous code ^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I've never seen anything like this. Feels like an Axios interceptor issue or something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, yeah. Seems like the error is an actual Error, not in the APIError shape like we'd expect. This is probably caused by the Axios interceptors not being applied to transform the error to the expected shape. I don't know if this is a new problem or an existing one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it fixed in 18edd23. Axios interceptors were not setup...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah thanks, nice fix - strange that this is the only test relying on those


// We must waitFor because we need to load region and cluster data from the API
await waitFor(() =>
userEvent.selectOptions(
getByPlaceholderText('Select a Region'),
'Newark, NJ (us-east-1)'
)
);

const saveButton = getByTestId('create-bucket-button');

userEvent.click(saveButton);

await findByText('Object Storage is offline!');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const CreateBucketDrawer = (props: Props) => {
error,
reset,
} = useCreateBucketMutation();

const { data: agreements } = useAccountAgreements();
const { mutateAsync: updateAccountAgreements } = useMutateAccountAgreements();
const { data: accountSettings } = useAccountSettings();
Expand Down Expand Up @@ -121,6 +122,7 @@ export const CreateBucketDrawer = (props: Props) => {
data-qa-permissions-notice
/>
)}
{Boolean(errorMap.none) && <Notice error text={errorMap.none} />}
<TextField
data-qa-cluster-label
label="Label"
Expand All @@ -134,7 +136,7 @@ export const CreateBucketDrawer = (props: Props) => {
/>
<ClusterSelect
data-qa-cluster-select
error={formik.touched.cluster ? errorMap.cluster : undefined}
error={errorMap.cluster}
onBlur={formik.handleBlur}
onChange={(value) => formik.setFieldValue('cluster', value)}
selectedCluster={formik.values.cluster}
Expand All @@ -153,7 +155,12 @@ export const CreateBucketDrawer = (props: Props) => {
<Button buttonType="secondary" onClick={onClose}>
Cancel
</Button>
<Button buttonType="primary" type="submit" loading={isLoading}>
<Button
buttonType="primary"
type="submit"
loading={isLoading}
data-testid="create-bucket-button"
>
Create Bucket
</Button>
</ActionsPanel>
Expand Down
3 changes: 3 additions & 0 deletions packages/manager/src/mocks/serverHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,9 @@ export const handlers = [
const buckets = objectStorageBucketFactory.buildList(10);
return res(ctx.json(makeResourcePage(buckets)));
}),
rest.post('*/object-storage/buckets', (req, res, ctx) => {
return res(ctx.json(objectStorageBucketFactory.build()));
}),
rest.get('*object-storage/clusters', (req, res, ctx) => {
const clusters = objectStorageClusterFactory.buildList(3);
return res(ctx.json(makeResourcePage(clusters)));
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/queries/objectStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
getObjectURL,
ObjectStorageObjectURLOptions,
ObjectStorageObjectURL,
} from '@linode/api-v4/lib/object-storage';
} from '@linode/api-v4';
import { queryKey as accountSettingsQueryKey } from './accountSettings';

export interface BucketError {
Expand Down
8 changes: 8 additions & 0 deletions packages/manager/src/utilities/testHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import thunk from 'redux-thunk';
import { FlagSet } from 'src/featureFlags';
import LinodeThemeWrapper from 'src/LinodeThemeWrapper';
import { queryClientFactory } from 'src/queries/base';
import { setupInterceptors } from 'src/request';
import {
storeFactory,
ApplicationState,
Expand Down Expand Up @@ -55,6 +56,13 @@ export const wrapWithTheme = (ui: any, options: Options = {}) => {
const storeToPass = customStore
? baseStore(customStore)
: storeFactory(queryClient);

// we have to call setupInterceptors so that our API error normalization works as expected
// I'm sorry that it makes us pass it the "ApplicationStore"
setupInterceptors(
configureStore<ApplicationState>([thunk])(defaultState)
);

return (
<Provider store={storeToPass}>
<QueryClientProvider client={passedQueryClient || queryClient}>
Expand Down