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: remove unnecessary toast notification on adding component #1490

Merged
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 2 additions & 0 deletions src/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ describe('Custom Hooks', () => {

fireEvent.scroll(window);

// Called on scroll once and then due to content being less than screen height
// and hasNextPage being true.
expect(fetchNextPage).toHaveBeenCalledTimes(2);
});

Expand Down
22 changes: 6 additions & 16 deletions src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import {
waitFor,
within,
} from '../testUtils';
import { executeThunk } from '../utils';
import initializeStore from '../store';
import { getApiWaffleFlagsUrl } from '../data/api';
import { fetchWaffleFlags } from '../data/thunks';
import mockResult from './__mocks__/library-search.json';
import mockEmptyResult from '../search-modal/__mocks__/empty-search-result.json';
import {
Expand All @@ -27,6 +23,7 @@ import { getStudioHomeApiUrl } from '../studio-home/data/api';
import { mockBroadcastChannel } from '../generic/data/api.mock';
import { LibraryLayout } from '.';
import { getLibraryCollectionsApiUrl } from './data/api';
import { getApiWaffleFlagsUrl } from '../data/api';

mockGetCollectionMetadata.applyMock();
mockContentSearchConfig.applyMock();
Expand Down Expand Up @@ -54,17 +51,12 @@ const returnEmptyResult = (_url, req) => {

const path = '/library/:libraryId/*';
const libraryTitle = mockContentLibrary.libraryData.title;
let store;

describe('<LibraryAuthoringPage />', () => {
beforeEach(async () => {
const { axiosMock } = initializeMocks();
store = initializeStore();
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);
axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});

// The Meilisearch client-side API uses fetch, not Axios.
fetchMock.mockReset();
Expand Down Expand Up @@ -689,17 +681,15 @@ describe('<LibraryAuthoringPage />', () => {

it('Shows an error if libraries V2 is disabled', async () => {
const { axiosMock } = initializeMocks();
axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, {
...studioHomeMock,
libraries_v2_enabled: false,
});
axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);

render(<LibraryLayout />, { path, params: { libraryId: mockContentLibrary.libraryId } });
await waitFor(() => { expect(axiosMock.history.get.length).toBe(4); });
expect(screen.getByRole('alert')).toHaveTextContent('This page cannot be shown: Libraries v2 are disabled.');
expect(await screen.findByRole('alert')).toHaveTextContent(
'This page cannot be shown: Libraries v2 are disabled.',
);
});
});
89 changes: 77 additions & 12 deletions src/library-authoring/add-content/AddContentContainer.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import MockAdapter from 'axios-mock-adapter/types';
import { snakeCaseObject } from '@edx/frontend-platform';
import {
fireEvent,
render as baseRender,
Expand All @@ -6,13 +8,21 @@ import {
initializeMocks,
} from '../../testUtils';
import { mockContentLibrary } from '../data/api.mocks';
import { getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl } from '../data/api';
import {
getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl,
} from '../data/api';
import { mockBroadcastChannel, mockClipboardEmpty, mockClipboardHtml } from '../../generic/data/api.mock';
import { LibraryProvider } from '../common/context';
import AddContentContainer from './AddContentContainer';
import { ComponentEditorModal } from '../components/ComponentEditorModal';
import editorCmsApi from '../../editors/data/services/cms/api';
import { ToastActionData } from '../../generic/toast-context';

mockBroadcastChannel();

// Mocks for ComponentEditorModal to work in tests.
jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' }));

const { libraryId } = mockContentLibrary;
const render = (collectionId?: string) => {
const params: { libraryId: string, collectionId?: string } = { libraryId };
Expand All @@ -26,15 +36,27 @@ const render = (collectionId?: string) => {
<LibraryProvider
libraryId={libraryId}
collectionId={collectionId}
>{ children }
>
{ children }
<ComponentEditorModal />
</LibraryProvider>
),
});
};
let axiosMock: MockAdapter;
let mockShowToast: (message: string, action?: ToastActionData | undefined) => void;

describe('<AddContentContainer />', () => {
beforeEach(() => {
const mocks = initializeMocks();
axiosMock = mocks.axiosMock;
mockShowToast = mocks.mockShowToast;
axiosMock.onGet(getContentLibraryApiUrl(libraryId)).reply(200, {});
});
afterEach(() => {
jest.restoreAllMocks();
});
it('should render content buttons', () => {
initializeMocks();
mockClipboardEmpty.applyMock();
render();
expect(screen.queryByRole('button', { name: /collection/i })).toBeInTheDocument();
Expand All @@ -48,7 +70,6 @@ describe('<AddContentContainer />', () => {
});

it('should create a content', async () => {
const { axiosMock } = initializeMocks();
mockClipboardEmpty.applyMock();
const url = getCreateLibraryBlockUrl(libraryId);
axiosMock.onPost(url).reply(200);
Expand All @@ -62,15 +83,15 @@ describe('<AddContentContainer />', () => {
await waitFor(() => expect(axiosMock.history.patch.length).toEqual(0));
});

it('should create a content in a collection', async () => {
const { axiosMock } = initializeMocks();
it('should create a content in a collection for non-editable blocks', async () => {
mockClipboardEmpty.applyMock();
const collectionId = 'some-collection-id';
const url = getCreateLibraryBlockUrl(libraryId);
const collectionComponentUrl = getLibraryCollectionComponentApiUrl(
libraryId,
collectionId,
);
// having id of block which is not video, html or problem will not trigger editor.
axiosMock.onPost(url).reply(200, { id: 'some-component-id' });
axiosMock.onPatch(collectionComponentUrl).reply(200);

Expand All @@ -84,8 +105,57 @@ describe('<AddContentContainer />', () => {
await waitFor(() => expect(axiosMock.history.patch[0].url).toEqual(collectionComponentUrl));
});

it('should create a content in a collection for editable blocks', async () => {
mockClipboardEmpty.applyMock();
const collectionId = 'some-collection-id';
const url = getCreateLibraryBlockUrl(libraryId);
const collectionComponentUrl = getLibraryCollectionComponentApiUrl(
libraryId,
collectionId,
);
// Mocks for ComponentEditorModal to work in tests.
jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line
{ data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } }
));
jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({
status: 200,
data: {
ancestors: [{
id: 'block-v1:Org+TS100+24+type@vertical+block@parent',
display_name: 'You-Knit? The Test Unit',
category: 'vertical',
has_children: true,
}],
},
}));

axiosMock.onPost(url).reply(200, {
id: 'lb:OpenedX:CSPROB2:html:1a5efd56-4ee5-4df0-b466-44f08fbbf567',
});
const fieldsHtml = {
displayName: 'Introduction to Testing',
data: '<p>This is a text component which uses <strong>HTML</strong>.</p>',
metadata: { displayName: 'Introduction to Testing' },
};
jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => (
{ status: 200, data: snakeCaseObject(fieldsHtml) }
));
axiosMock.onPatch(collectionComponentUrl).reply(200);

render(collectionId);

const textButton = screen.getByRole('button', { name: /text/i });
fireEvent.click(textButton);

// Component should be linked to Collection on closing editor.
const closeButton = await screen.findByRole('button', { name: 'Exit the editor' });
fireEvent.click(closeButton);
await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url));
await waitFor(() => expect(axiosMock.history.patch.length).toEqual(1));
await waitFor(() => expect(axiosMock.history.patch[0].url).toEqual(collectionComponentUrl));
});

it('should render paste button if clipboard contains pastable xblock', async () => {
initializeMocks();
// Simulate having an HTML block in the clipboard:
const getClipboardSpy = mockClipboardHtml.applyMock();
render();
Expand All @@ -94,7 +164,6 @@ describe('<AddContentContainer />', () => {
});

it('should paste content', async () => {
const { axiosMock } = initializeMocks();
// Simulate having an HTML block in the clipboard:
const getClipboardSpy = mockClipboardHtml.applyMock();

Expand All @@ -112,7 +181,6 @@ describe('<AddContentContainer />', () => {
});

it('should paste content inside a collection', async () => {
const { axiosMock } = initializeMocks();
// Simulate having an HTML block in the clipboard:
const getClipboardSpy = mockClipboardHtml.applyMock();

Expand All @@ -138,7 +206,6 @@ describe('<AddContentContainer />', () => {
});

it('should show error toast on linking failure', async () => {
const { axiosMock, mockShowToast } = initializeMocks();
// Simulate having an HTML block in the clipboard:
const getClipboardSpy = mockClipboardHtml.applyMock();

Expand All @@ -165,7 +232,6 @@ describe('<AddContentContainer />', () => {
});

it('should stop user from pasting unsupported blocks and show toast', async () => {
const { axiosMock, mockShowToast } = initializeMocks();
// Simulate having an HTML block in the clipboard:
mockClipboardHtml.applyMock('openassessment');

Expand Down Expand Up @@ -214,7 +280,6 @@ describe('<AddContentContainer />', () => {
])('$label', async ({
mockUrl, mockResponse, buttonName, expectedError,
}) => {
const { axiosMock, mockShowToast } = initializeMocks();
axiosMock.onPost(mockUrl).reply(400, mockResponse);

// Simulate having an HTML block in the clipboard:
Expand Down
14 changes: 5 additions & 9 deletions src/library-authoring/add-content/AddContentContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ const AddContentContainer = () => {
}

const linkComponent = (usageKey: string) => {
updateComponentsMutation.mutateAsync([usageKey]).then(() => {
showToast(intl.formatMessage(messages.successAssociateComponentMessage));
}).catch(() => {
updateComponentsMutation.mutateAsync([usageKey]).catch(() => {
showToast(intl.formatMessage(messages.errorAssociateComponentMessage));
});
};
Expand Down Expand Up @@ -199,13 +197,14 @@ const AddContentContainer = () => {
blockType,
definitionId: `${uuid4()}`,
}).then((data) => {
linkComponent(data.id);
const hasEditor = canEditComponent(data.id);
if (hasEditor) {
openComponentEditor(data.id);
// linkComponent on editor close.
openComponentEditor(data.id, () => linkComponent(data.id));
} else {
// We can't start editing this right away so just show a toast message:
showToast(intl.formatMessage(messages.successCreateMessage));
linkComponent(data.id);
}
}).catch((error) => {
showToast(parseErrorMsg(
Expand All @@ -228,14 +227,11 @@ const AddContentContainer = () => {
}
};

/* istanbul ignore next */
if (pasteClipboardMutation.isLoading) {
showToast(intl.formatMessage(messages.pastingClipboardMessage));
}

if (updateComponentsMutation.isLoading) {
showToast(intl.formatMessage(messages.linkingComponentMessage));
}

return (
<Stack direction="vertical">
{collectionId ? (
Expand Down
11 changes: 1 addition & 10 deletions src/library-authoring/add-content/AddContentWorkflow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@ import {
mockCreateLibraryBlock,
mockXBlockFields,
} from '../data/api.mocks';
import initializeStore from '../../store';
import { executeThunk } from '../../utils';
import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock';
import { mockContentSearchConfig, mockSearchResult } from '../../search-manager/data/api.mock';
import { studioHomeMock } from '../../studio-home/__mocks__';
import { getStudioHomeApiUrl } from '../../studio-home/data/api';
import { getApiWaffleFlagsUrl } from '../../data/api';
import { fetchWaffleFlags } from '../../data/thunks';
import LibraryLayout from '../LibraryLayout';

mockContentSearchConfig.applyMock();
Expand All @@ -51,17 +48,11 @@ const renderOpts = {
routerProps: { initialEntries: [`/library/${libraryId}/components`] },
};

let store;

describe('AddContentWorkflow test', () => {
beforeEach(async () => {
const { axiosMock } = initializeMocks();
store = initializeStore();
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);
axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
});

it('can create an HTML component', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import {
screen,
initializeMocks,
} from '../../testUtils';
import initializeStore from '../../store';
import { executeThunk } from '../../utils';
import { studioHomeMock } from '../../studio-home/__mocks__';
import { getStudioHomeApiUrl } from '../../studio-home/data/api';
import mockResult from '../__mocks__/library-search.json';
Expand All @@ -20,7 +18,6 @@ import {
} from '../data/api.mocks';
import { PickLibraryContentModal } from './PickLibraryContentModal';
import { getApiWaffleFlagsUrl } from '../../data/api';
import { fetchWaffleFlags } from '../../data/thunks';

mockContentSearchConfig.applyMock();
mockContentLibrary.applyMock();
Expand All @@ -31,7 +28,6 @@ const { libraryId } = mockContentLibrary;

const onClose = jest.fn();
let mockShowToast: (message: string) => void;
let store;

const render = () => baseRender(<PickLibraryContentModal isOpen onClose={onClose} />, {
path: '/library/:libraryId/collection/:collectionId/*',
Expand All @@ -50,13 +46,9 @@ const render = () => baseRender(<PickLibraryContentModal isOpen onClose={onClose
describe('<PickLibraryContentModal />', () => {
beforeEach(async () => {
const mocks = initializeMocks();
store = initializeStore();
mockShowToast = mocks.mockShowToast;
mocks.axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
mocks.axiosMock
.onGet(getApiWaffleFlagsUrl())
.reply(200, {});
await executeThunk(fetchWaffleFlags(), store.dispatch);
mocks.axiosMock.onGet(getApiWaffleFlagsUrl()).reply(200, {});
});

it('can pick components from the modal', async () => {
Expand Down
5 changes: 0 additions & 5 deletions src/library-authoring/add-content/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@ const messages = defineMessages({
+ ' The {detail} text provides more information about the error.'
),
},
linkingComponentMessage: {
id: 'course-authoring.library-authoring.linking-collection-content.progress.text',
defaultMessage: 'Adding component to collection...',
description: 'Message when component is being linked to collection in library',
},
successAssociateComponentMessage: {
id: 'course-authoring.library-authoring.associate-collection-content.success.text',
defaultMessage: 'Content linked successfully.',
Expand Down
Loading