refactor: stop using meilisearch to get a single collection

openedx · Sep 26, 2024 · d3e4d3d · d3e4d3d
1 parent cd3b912
commit d3e4d3d
Show file tree

Hide file tree

Showing 16 changed files with 194 additions and 135 deletions.
diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx
@@ -11,12 +11,18 @@ import {
 } from '../testUtils';
 import mockResult from './__mocks__/library-search.json';
 import mockEmptyResult from '../search-modal/__mocks__/empty-search-result.json';
-import { mockContentLibrary, mockLibraryBlockTypes, mockXBlockFields } from './data/api.mocks';
+import {
+  mockContentLibrary,
+  mockGetCollectionMetadata,
+  mockLibraryBlockTypes,
+  mockXBlockFields,
+} from './data/api.mocks';
 import { mockContentSearchConfig } from '../search-manager/data/api.mock';
 import { mockBroadcastChannel } from '../generic/data/api.mock';
 import { LibraryLayout } from '.';
 import { getLibraryCollectionsApiUrl } from './data/api';
 
+mockGetCollectionMetadata.applyMock();
 mockContentSearchConfig.applyMock();
 mockContentLibrary.applyMock();
 mockLibraryBlockTypes.applyMock();
@@ -459,17 +465,17 @@ describe('<LibraryAuthoringPage />', () => {
   });
 
   it('should open and close the collection sidebar', async () => {
-    const displayName = 'Collection 1';
     await renderLibraryPage();
 
     // Click on the first component. It could appear twice, in both "Recently Modified" and "Collections"
-    fireEvent.click((await screen.findAllByText(displayName))[0]);
+    fireEvent.click((await screen.findAllByText('Collection 1'))[0]);
 
     const sidebar = screen.getByTestId('library-sidebar');
 
     const { getByRole, getByText } = within(sidebar);
 
-    await waitFor(() => expect(getByText(displayName)).toBeInTheDocument());
+    // The mock data for the sidebar has a title of "Test Collection"
+    await waitFor(() => expect(getByText('Test Collection')).toBeInTheDocument());
 
     const closeButton = getByRole('button', { name: /close/i });
     fireEvent.click(closeButton);

diff --git a/src/library-authoring/__mocks__/library-search.json b/src/library-authoring/__mocks__/library-search.json
@@ -284,6 +284,7 @@
       "hits": [
         {
           "display_name": "Collection 1",
+          "block_id": "col1",
           "description": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque et mi ac nisi accumsan imperdiet vitae at odio. Vivamus tempor nec lorem eget lacinia. Vivamus efficitur lacus non dapibus porta. Nulla venenatis luctus nisi id posuere. Sed sollicitudin magna a sem ultrices accumsan. Praesent volutpat tortor vitae luctus rutrum. Integer.",
           "id": 1,
           "type": "collection",

diff --git a/src/library-authoring/collections/CollectionDetails.test.tsx b/src/library-authoring/collections/CollectionDetails.test.tsx
@@ -2,7 +2,6 @@ import type MockAdapter from 'axios-mock-adapter';
 import fetchMock from 'fetch-mock-jest';
 
 import { mockContentSearchConfig, mockGetBlockTypes } from '../../search-manager/data/api.mock';
-import { type CollectionHit, formatSearchHit } from '../../search-manager/data/api';
 import {
   initializeMocks,
   fireEvent,
@@ -11,19 +10,21 @@ import {
   waitFor,
   within,
 } from '../../testUtils';
-import mockResult from '../__mocks__/collection-search.json';
 import * as api from '../data/api';
-import { mockContentLibrary } from '../data/api.mocks';
+import { mockContentLibrary, mockGetCollectionMetadata } from '../data/api.mocks';
 import CollectionDetails from './CollectionDetails';
 
 let axiosMock: MockAdapter;
 let mockShowToast: (message: string) => void;
 
+mockGetCollectionMetadata.applyMock();
 mockContentSearchConfig.applyMock();
 mockGetBlockTypes.applyMock();
 
+const { collectionId } = mockGetCollectionMetadata;
+const { description: originalDescription } = mockGetCollectionMetadata.collectionData;
+
 const library = mockContentLibrary.libraryData;
-const collectionHit = formatSearchHit(mockResult.results[2].hits[0]) as CollectionHit;
 
 describe('<CollectionDetails />', () => {
   beforeEach(() => {
@@ -39,12 +40,11 @@ describe('<CollectionDetails />', () => {
   });
 
   it('should render Collection Details', async () => {
-    render(<CollectionDetails library={library} collection={collectionHit} />);
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
 
     // Collection Description
-    expect(screen.getByText('Description / Card Preview Text')).toBeInTheDocument();
-    const { description } = collectionHit;
-    expect(screen.getByText(description)).toBeInTheDocument();
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
+    expect(screen.getByText(originalDescription)).toBeInTheDocument();
 
     // Collection History
     expect(screen.getByText('Collection History')).toBeInTheDocument();
@@ -55,17 +55,12 @@ describe('<CollectionDetails />', () => {
   });
 
   it('should allow modifying the description', async () => {
-    render(<CollectionDetails library={library} collection={collectionHit} />);
-
-    const {
-      description: originalDescription,
-      blockId,
-      contextKey,
-    } = collectionHit;
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
 
     expect(screen.getByText(originalDescription)).toBeInTheDocument();
 
-    const url = api.getLibraryCollectionApiUrl(contextKey, blockId);
+    const url = api.getLibraryCollectionApiUrl(library.id, collectionId);
     axiosMock.onPatch(url).reply(200);
 
     const textArea = screen.getByRole('textbox');
@@ -94,17 +89,12 @@ describe('<CollectionDetails />', () => {
   });
 
   it('should show error while modifing the description', async () => {
-    render(<CollectionDetails library={library} collection={collectionHit} />);
-
-    const {
-      description: originalDescription,
-      blockId,
-      contextKey,
-    } = collectionHit;
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
 
     expect(screen.getByText(originalDescription)).toBeInTheDocument();
 
-    const url = api.getLibraryCollectionApiUrl(contextKey, blockId);
+    const url = api.getLibraryCollectionApiUrl(library.id, collectionId);
     axiosMock.onPatch(url).reply(500);
 
     const textArea = screen.getByRole('textbox');
@@ -124,7 +114,8 @@ describe('<CollectionDetails />', () => {
 
   it('should render Collection stats', async () => {
     mockGetBlockTypes('someBlocks');
-    render(<CollectionDetails library={library} collection={collectionHit} />);
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
 
     expect(screen.getByText('Collection Stats')).toBeInTheDocument();
     expect(await screen.findByText('Total')).toBeInTheDocument();
@@ -141,15 +132,17 @@ describe('<CollectionDetails />', () => {
 
   it('should render Collection stats for empty collection', async () => {
     mockGetBlockTypes('noBlocks');
-    render(<CollectionDetails library={library} collection={collectionHit} />);
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
 
     expect(screen.getByText('Collection Stats')).toBeInTheDocument();
     expect(await screen.findByText('This collection is currently empty.')).toBeInTheDocument();
   });
 
   it('should render Collection stats for big collection', async () => {
     mockGetBlockTypes('moreBlocks');
-    render(<CollectionDetails library={library} collection={collectionHit} />);
+    render(<CollectionDetails library={library} collectionId={collectionId} />);
+    expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument();
 
     expect(screen.getByText('Collection Stats')).toBeInTheDocument();
     expect(await screen.findByText('36')).toBeInTheDocument();

diff --git a/src/library-authoring/collections/CollectionDetails.tsx b/src/library-authoring/collections/CollectionDetails.tsx
@@ -1,13 +1,13 @@
 import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
 import { Icon, Stack } from '@openedx/paragon';
-import { useContext, useState } from 'react';
+import { useContext, useEffect, useState } from 'react';
 import classNames from 'classnames';
 
 import { getItemIcon } from '../../generic/block-type-utils';
 import { ToastContext } from '../../generic/toast-context';
-import { BlockTypeLabel, type CollectionHit, useGetBlockTypes } from '../../search-manager';
+import { BlockTypeLabel, useGetBlockTypes } from '../../search-manager';
 import type { ContentLibrary } from '../data/api';
-import { useUpdateCollection } from '../data/apiHooks';
+import { useCollection, useUpdateCollection } from '../data/apiHooks';
 import HistoryWidget from '../generic/history-widget';
 import messages from './messages';
 
@@ -37,13 +37,14 @@ const BlockCount = ({
 };
 
 interface CollectionStatsWidgetProps {
-  collection: CollectionHit,
+  libraryId: string,
+  collectionId: string,
 }
 
-const CollectionStatsWidget = ({ collection }: CollectionStatsWidgetProps) => {
+const CollectionStatsWidget = ({ libraryId, collectionId }: CollectionStatsWidgetProps) => {
   const { data: blockTypes } = useGetBlockTypes([
-    `context_key = "${collection.contextKey}"`,
-    `collections.key = "${collection.blockId}"`,
+    `context_key = "${libraryId}"`,
+    `collections.key = "${collectionId}"`,
   ]);
 
   if (!blockTypes) {
@@ -100,20 +101,26 @@ const CollectionStatsWidget = ({ collection }: CollectionStatsWidgetProps) => {
 
 interface CollectionDetailsProps {
   library: ContentLibrary,
-  collection: CollectionHit,
+  collectionId: string,
 }
 
-const CollectionDetails = ({ library, collection }: CollectionDetailsProps) => {
+const CollectionDetails = ({ library, collectionId }: CollectionDetailsProps) => {
   const intl = useIntl();
   const { showToast } = useContext(ToastContext);
 
-  const [description, setDescription] = useState(collection.description);
+  const updateMutation = useUpdateCollection(library.id, collectionId);
+  const { data: collection } = useCollection(library.id, collectionId);
 
-  const updateMutation = useUpdateCollection(collection.contextKey, collection.blockId);
+  const [description, setDescription] = useState(collection?.description || '');
+
+  useEffect(() => {
+    if (collection) {
+      setDescription(collection.description);
+    }
+  }, [collection]);
 
-  // istanbul ignore if: this should never happen
   if (!collection) {
-    throw new Error('A collection must be provided to CollectionDetails');
+    return null;
   }
 
   const onSubmit = (e: React.FocusEvent<HTMLTextAreaElement>) => {
@@ -151,16 +158,16 @@ const CollectionDetails = ({ library, collection }: CollectionDetailsProps) => {
         <h3 className="h5">
           {intl.formatMessage(messages.detailsTabStatsTitle)}
         </h3>
-        <CollectionStatsWidget collection={collection} />
+        <CollectionStatsWidget libraryId={library.id} collectionId={collectionId} />
       </div>
       <hr className="w-100" />
       <div>
         <h3 className="h5">
           {intl.formatMessage(messages.detailsTabHistoryTitle)}
         </h3>
         <HistoryWidget
-          created={collection.created ? new Date(collection.created * 1000) : null}
-          modified={collection.modified ? new Date(collection.modified * 1000) : null}
+          created={collection.created ? new Date(collection.created) : null}
+          modified={collection.modified ? new Date(collection.modified) : null}
         />
       </div>
     </Stack>

diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx
@@ -14,12 +14,12 @@ import messages from './messages';
 
 interface CollectionInfoProps {
   library: ContentLibrary,
-  collection: CollectionHit,
+  collectionId: string,
 }
 
-const CollectionInfo = ({ library, collection }: CollectionInfoProps) => {
+const CollectionInfo = ({ library, collectionId }: CollectionInfoProps) => {
   const intl = useIntl();
-  const url = `/library/${library.id}/collection/${collection.blockId}/`;
+  const url = `/library/${library.id}/collection/${collectionId}/`;
   const urlMatch = useMatch(url);
 
   return (
@@ -28,7 +28,7 @@ const CollectionInfo = ({ library, collection }: CollectionInfoProps) => {
         <div className="d-flex flex-wrap">
           <Button
             as={Link}
-            to={`/library/${library.id}/collection/${collection.blockId}/`}
+            to={url}
             variant="outline-primary"
             className="m-1 text-nowrap flex-grow-1"
             disabled={!!urlMatch}
@@ -47,9 +47,8 @@ const CollectionInfo = ({ library, collection }: CollectionInfoProps) => {
         </Tab>
         <Tab eventKey="details" title={intl.formatMessage(messages.detailsTabTitle)}>
           <CollectionDetails
-            key={collection.id} // This is necessary to force a re-render when the collection changes
             library={library}
-            collection={collection}
+            collectionId={collectionId}
           />
         </Tab>
       </Tabs>