Skip to content

Commit

Permalink
fix: Show spinner while loading library components (#1331)
Browse files Browse the repository at this point in the history
  • Loading branch information
pomegranited authored Oct 4, 2024
1 parent 652af9f commit 9c1fd5a
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports[`SolutionWidget render snapshot: renders correct default 1`] = `
<FormattedMessage
defaultMessage="Provide an explanation for the correct answer"
description="Description of the solution widget"
id="authoring.problemEditor.solutionwidget.solutionDescriptionText"
id="authoring.problemEditor.explanationwidget.solutionDescriptionText"
/>
</div>
<TinyMceWidget
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ const messages = defineMessages({
description: 'Explanation Title',
},
solutionDescriptionText: {
id: 'authoring.problemEditor.solutionwidget.solutionDescriptionText',
id: 'authoring.problemEditor.explanationwidget.solutionDescriptionText',
defaultMessage: 'Provide an explanation for the correct answer',
description: 'Description of the solution widget',
},
placeholder: {
id: 'authoring.problemEditor.questionwidget.placeholder',
id: 'authoring.problemEditor.explanationwidget.placeholder',
defaultMessage: 'Enter your explanation',
description: 'Placeholder text for tinyMCE editor',
},
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ describe('<LibraryAuthoringPage />', () => {
expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument();

fireEvent.click(screen.getByRole('tab', { name: 'Collections' }));
expect(screen.getByText('You have not added any collection to this library yet.')).toBeInTheDocument();
expect(screen.getByText('You have not added any collections to this library yet.')).toBeInTheDocument();

// Open Create collection modal
const addCollectionButton = screen.getByRole('button', { name: /add collection/i });
Expand Down
5 changes: 5 additions & 0 deletions src/library-authoring/LibraryHome.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Stack } from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';

import { LoadingSpinner } from '../generic/Loading';
import { useSearchContext } from '../search-manager';
import { NoComponents, NoSearchResults } from './EmptyStates';
import LibraryCollections from './collections/LibraryCollections';
Expand All @@ -20,11 +21,15 @@ const LibraryHome = ({ tabList, handleTabChange } : LibraryHomeProps) => {
const {
totalHits: componentCount,
totalCollectionHits: collectionCount,
isLoading,
isFiltered,
} = useSearchContext();
const { openAddContentSidebar } = useLibraryContext();

const renderEmptyState = () => {
if (isLoading) {
return <LoadingSpinner />;
}
if (componentCount === 0 && collectionCount === 0) {
return isFiltered ? <NoSearchResults /> : <NoComponents handleBtnClick={openAddContentSidebar} />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const path = '/library/:libraryId/*';
const libraryTitle = mockContentLibrary.libraryData.title;
const mockCollection = {
collectionId: mockResult.results[2].hits[0].block_id,
collectionNeverLoads: 'collection-always-loading',
collectionNeverLoads: mockGetCollectionMetadata.collectionIdLoading,
collectionNoComponents: 'collection-no-components',
collectionEmpty: mockGetCollectionMetadata.collectionIdError,
};
Expand Down Expand Up @@ -108,7 +108,7 @@ describe('<LibraryCollectionPage />', () => {
it('shows an error component if no collection returned', async () => {
// This mock will simulate incorrect collection id
await renderLibraryCollectionPage(mockCollection.collectionEmpty);
expect(await screen.findByText(/Mocked request failed with status code 400./)).toBeInTheDocument();
expect(await screen.findByText(/Mocked request failed with status code 404./)).toBeInTheDocument();
});

it('shows collection data', async () => {
Expand Down
89 changes: 89 additions & 0 deletions src/library-authoring/collections/LibraryCollections.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import fetchMock from 'fetch-mock-jest';

import {
render,
screen,
initializeMocks,
} from '../../testUtils';
import { getContentSearchConfigUrl } from '../../search-manager/data/api';
import { mockContentLibrary } from '../data/api.mocks';
import mockEmptyResult from '../../search-modal/__mocks__/empty-search-result.json';
import { LibraryProvider } from '../common/context';
import LibraryCollections from './LibraryCollections';

const searchEndpoint = 'http://mock.meilisearch.local/multi-search';

mockContentLibrary.applyMock();
const mockFetchNextPage = jest.fn();
const mockUseSearchContext = jest.fn();

const data = {
totalHits: 1,
hits: [],
isFetchingNextPage: false,
hasNextPage: false,
fetchNextPage: mockFetchNextPage,
searchKeywords: '',
isFiltered: false,
isLoading: false,
};

const returnEmptyResult = (_url: string, req) => {
const requestData = JSON.parse(req.body?.toString() ?? '');
const query = requestData?.queries[0]?.q ?? '';
// We have to replace the query (search keywords) in the mock results with the actual query,
// because otherwise we may have an inconsistent state that causes more queries and unexpected results.
mockEmptyResult.results[0].query = query;
// And fake the required '_formatted' fields; it contains the highlighting <mark>...</mark> around matched words
// eslint-disable-next-line no-underscore-dangle, no-param-reassign
mockEmptyResult.results[0]?.hits.forEach((hit: any) => { hit._formatted = { ...hit }; });
return mockEmptyResult;
};

jest.mock('../../search-manager', () => ({
...jest.requireActual('../../search-manager'),
useSearchContext: () => mockUseSearchContext(),
}));

const clipboardBroadcastChannelMock = {
postMessage: jest.fn(),
close: jest.fn(),
};

(global as any).BroadcastChannel = jest.fn(() => clipboardBroadcastChannelMock);

const withLibraryId = (libraryId: string) => ({
extraWrapper: ({ children }: { children: React.ReactNode }) => (
<LibraryProvider libraryId={libraryId}>{children}</LibraryProvider>
),
});

describe('<LibraryCollections />', () => {
beforeEach(() => {
const { axiosMock } = initializeMocks();

fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true });

// The API method to get the Meilisearch connection details uses Axios:
axiosMock.onGet(getContentSearchConfigUrl()).reply(200, {
url: 'http://mock.meilisearch.local',
index_name: 'studio',
api_key: 'test-key',
});
});

afterEach(() => {
fetchMock.reset();
mockFetchNextPage.mockReset();
});

it('should render a spinner while loading', async () => {
mockUseSearchContext.mockReturnValue({
...data,
isLoading: true,
});

render(<LibraryCollections variant="full" />, withLibraryId(mockContentLibrary.libraryId));
expect(screen.getByText('Loading...')).toBeInTheDocument();
});
});
6 changes: 6 additions & 0 deletions src/library-authoring/collections/LibraryCollections.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { LoadingSpinner } from '../../generic/Loading';
import { useLoadOnScroll } from '../../hooks';
import { useSearchContext } from '../../search-manager';
import { NoComponents, NoSearchResults } from '../EmptyStates';
Expand All @@ -24,6 +25,7 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => {
isFetchingNextPage,
hasNextPage,
fetchNextPage,
isLoading,
isFiltered,
} = useSearchContext();

Expand All @@ -38,6 +40,10 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => {
variant === 'full',
);

if (isLoading) {
return <LoadingSpinner />;
}

if (totalCollectionHits === 0) {
return isFiltered
? <NoSearchResults infoText={messages.noSearchResultsCollections} />
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/collections/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const messages = defineMessages({
},
noCollections: {
id: 'course-authoring.library-authoring.no-collections',
defaultMessage: 'You have not added any collection to this library yet.',
defaultMessage: 'You have not added any collections to this library yet.',
description: 'Message displayed when the library has no collections',
},
addCollection: {
Expand Down
18 changes: 12 additions & 6 deletions src/library-authoring/components/LibraryComponents.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ const mockUseSearchContext = jest.fn();
const data = {
totalHits: 1,
hits: [],
isFetching: true,
isFetchingNextPage: false,
hasNextPage: false,
fetchNextPage: mockFetchNextPage,
searchKeywords: '',
isFiltered: false,
isLoading: false,
};

const returnEmptyResult = (_url: string, req) => {
Expand Down Expand Up @@ -102,11 +102,20 @@ describe('<LibraryComponents />', () => {
expect(screen.queryByRole('button', { name: /add component/i })).not.toBeInTheDocument();
});

it('should render a spinner while loading', async () => {
mockUseSearchContext.mockReturnValue({
...data,
isLoading: true,
});

render(<LibraryComponents variant="full" />, withLibraryId(mockContentLibrary.libraryId));
expect(screen.getByText('Loading...')).toBeInTheDocument();
});

it('should render components in full variant', async () => {
mockUseSearchContext.mockReturnValue({
...data,
hits: libraryComponentsMock,
isFetching: false,
});
render(<LibraryComponents variant="full" />, withLibraryId(mockContentLibrary.libraryId));

Expand All @@ -122,7 +131,6 @@ describe('<LibraryComponents />', () => {
mockUseSearchContext.mockReturnValue({
...data,
hits: libraryComponentsMock,
isFetching: false,
});
render(<LibraryComponents variant="preview" />, withLibraryId(mockContentLibrary.libraryId));

Expand All @@ -138,7 +146,6 @@ describe('<LibraryComponents />', () => {
mockUseSearchContext.mockReturnValue({
...data,
hits: libraryComponentsMock,
isFetching: false,
hasNextPage: true,
});

Expand All @@ -152,11 +159,10 @@ describe('<LibraryComponents />', () => {
expect(mockFetchNextPage).toHaveBeenCalled();
});

it('should not call `fetchNextPage` on croll to bottom in preview variant', async () => {
it('should not call `fetchNextPage` on scroll to bottom in preview variant', async () => {
mockUseSearchContext.mockReturnValue({
...data,
hits: libraryComponentsMock,
isFetching: false,
hasNextPage: true,
});

Expand Down
6 changes: 6 additions & 0 deletions src/library-authoring/components/LibraryComponents.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useMemo } from 'react';

import { LoadingSpinner } from '../../generic/Loading';
import { useLoadOnScroll } from '../../hooks';
import { useSearchContext } from '../../search-manager';
import { NoComponents, NoSearchResults } from '../EmptyStates';
Expand All @@ -26,6 +27,7 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => {
isFetchingNextPage,
hasNextPage,
fetchNextPage,
isLoading,
isFiltered,
} = useSearchContext();
const { libraryId, openAddContentSidebar } = useLibraryContext();
Expand All @@ -51,6 +53,10 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => {
variant === 'full',
);

if (isLoading) {
return <LoadingSpinner />;
}

if (componentCount === 0) {
return isFiltered ? <NoSearchResults /> : <NoComponents handleBtnClick={openAddContentSidebar} />;
}
Expand Down
15 changes: 12 additions & 3 deletions src/library-authoring/data/api.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,22 @@ mockLibraryBlockMetadata.applyMock = () => jest.spyOn(api, 'getLibraryBlockMetad
* This mock returns a fixed response for the collection ID *collection_1*.
*/
export async function mockGetCollectionMetadata(libraryId: string, collectionId: string): Promise<api.Collection> {
if (collectionId === mockGetCollectionMetadata.collectionIdError) {
throw createAxiosError({ code: 400, message: 'Not found.', path: api.getLibraryCollectionApiUrl(libraryId, collectionId) });
switch (collectionId) {
case mockGetCollectionMetadata.collectionIdError:
throw createAxiosError({
code: 404,
message: 'Not found.',
path: api.getLibraryCollectionApiUrl(libraryId, collectionId),
});
case mockGetCollectionMetadata.collectionIdLoading:
return new Promise(() => {});
default:
return Promise.resolve(mockGetCollectionMetadata.collectionData);
}
return Promise.resolve(mockGetCollectionMetadata.collectionData);
}
mockGetCollectionMetadata.collectionId = 'collection_1';
mockGetCollectionMetadata.collectionIdError = 'collection_error';
mockGetCollectionMetadata.collectionIdLoading = 'collection_loading';
mockGetCollectionMetadata.collectionData = {
id: 1,
key: 'collection_1',
Expand Down
2 changes: 1 addition & 1 deletion src/search-manager/SearchManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export interface SearchContextData {
defaultSearchSortOrder: SearchSortOption;
hits: ContentHit[];
totalHits: number;
isFetching: boolean;
isLoading: boolean;
hasNextPage: boolean | undefined;
isFetchingNextPage: boolean;
fetchNextPage: () => void;
Expand Down
2 changes: 1 addition & 1 deletion src/search-manager/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export const useContentSearchResults = ({
blockTypes: pages?.[0]?.blockTypes ?? {},
problemTypes: pages?.[0]?.problemTypes ?? {},
status: query.status,
isFetching: query.isFetching,
isLoading: query.isLoading,
isError: query.isError,
isFetchingNextPage: query.isFetchingNextPage,
// Call this to load more pages. We include some "safety" features recommended by the docs: this should never be
Expand Down

0 comments on commit 9c1fd5a

Please sign in to comment.