From d3e4d3d6fe79d4d52eda7d8b29ce7ba74a20eb23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 26 Sep 2024 17:11:25 -0300 Subject: [PATCH] refactor: stop using meilisearch to get a single collection --- .../LibraryAuthoringPage.test.tsx | 14 ++++-- .../__mocks__/library-search.json | 1 + .../collections/CollectionDetails.test.tsx | 47 ++++++++----------- .../collections/CollectionDetails.tsx | 39 ++++++++------- .../collections/CollectionInfo.tsx | 11 ++--- .../collections/CollectionInfoHeader.test.tsx | 41 ++++++++++------ .../collections/CollectionInfoHeader.tsx | 21 +++++---- .../LibraryCollectionPage.test.tsx | 29 +++++++----- .../collections/LibraryCollectionPage.tsx | 22 +++++---- src/library-authoring/common/context.tsx | 19 ++++---- .../components/CollectionCard.tsx | 2 +- src/library-authoring/data/api.mocks.ts | 29 ++++++++++++ src/library-authoring/data/api.ts | 8 ++++ src/library-authoring/data/apiHooks.ts | 17 ++++++- .../library-sidebar/LibrarySidebar.tsx | 6 +-- src/search-manager/data/api.mock.ts | 23 --------- 16 files changed, 194 insertions(+), 135 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 78cfdbfda1..b2b044429a 100644 --- 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('', () => { }); 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 index f84d16c611..72ea474fdd 100644 --- 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 index 21af576981..5c4e69064e 100644 --- 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('', () => { beforeEach(() => { @@ -39,12 +40,11 @@ describe('', () => { }); it('should render Collection Details', async () => { - render(); + render(); // 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('', () => { }); it('should allow modifying the description', async () => { - render(); - - const { - description: originalDescription, - blockId, - contextKey, - } = collectionHit; + render(); + 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('', () => { }); it('should show error while modifing the description', async () => { - render(); - - const { - description: originalDescription, - blockId, - contextKey, - } = collectionHit; + render(); + 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('', () => { it('should render Collection stats', async () => { mockGetBlockTypes('someBlocks'); - render(); + render(); + expect(await screen.findByText('Description / Card Preview Text')).toBeInTheDocument(); expect(screen.getByText('Collection Stats')).toBeInTheDocument(); expect(await screen.findByText('Total')).toBeInTheDocument(); @@ -141,7 +132,8 @@ describe('', () => { it('should render Collection stats for empty collection', async () => { mockGetBlockTypes('noBlocks'); - render(); + render(); + 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(); @@ -149,7 +141,8 @@ describe('', () => { it('should render Collection stats for big collection', async () => { mockGetBlockTypes('moreBlocks'); - render(); + render(); + 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 index b7ecab76a3..9936177902 100644 --- 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) => { @@ -151,7 +158,7 @@ const CollectionDetails = ({ library, collection }: CollectionDetailsProps) => {

{intl.formatMessage(messages.detailsTabStatsTitle)}

- +
@@ -159,8 +166,8 @@ const CollectionDetails = ({ library, collection }: CollectionDetailsProps) => { {intl.formatMessage(messages.detailsTabHistoryTitle)}
diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx index 0ab2c7fe62..a02cc48406 100644 --- 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) => {