From 356a26e4728b560172d2ad85762780f7b71857b3 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 19 Sep 2024 19:42:14 +0530 Subject: [PATCH] refactor: fetch single collection info from meilisearch --- .../LibraryAuthoringPage.tsx | 1 - .../LibraryRecentlyModified.tsx | 1 - .../__mocks__/collection-search.json | 29 +++++ .../collections/CollectionInfoHeader.tsx | 6 +- .../LibraryCollectionPage.test.tsx | 103 +++++++++--------- .../collections/LibraryCollectionPage.tsx | 99 ++++++++++------- src/library-authoring/data/api.mocks.ts | 41 ------- src/library-authoring/data/api.ts | 8 -- src/library-authoring/data/apiHooks.ts | 12 -- .../library-sidebar/LibrarySidebar.tsx | 5 +- src/search-manager/SearchManager.ts | 8 +- src/search-manager/data/api.ts | 86 +++++++++------ src/search-manager/data/apiHooks.ts | 9 +- 13 files changed, 207 insertions(+), 201 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 68f02837ea..09e5a94fbd 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -166,7 +166,6 @@ const LibraryAuthoringPage = () => { } diff --git a/src/library-authoring/LibraryRecentlyModified.tsx b/src/library-authoring/LibraryRecentlyModified.tsx index 4371d6ced4..57828871ef 100644 --- a/src/library-authoring/LibraryRecentlyModified.tsx +++ b/src/library-authoring/LibraryRecentlyModified.tsx @@ -81,7 +81,6 @@ const LibraryRecentlyModified: React.FC<{ libraryId: string }> = ({ libraryId }) diff --git a/src/library-authoring/__mocks__/collection-search.json b/src/library-authoring/__mocks__/collection-search.json index 3f48fa46ea..3033e3c36a 100644 --- a/src/library-authoring/__mocks__/collection-search.json +++ b/src/library-authoring/__mocks__/collection-search.json @@ -184,6 +184,35 @@ "content.problem_types": {} }, "facetStats": {} + }, + { + "indexUid": "studio_content", + "hits": [ + { + "display_name": "My first collection", + "block_id": "my-first-collection", + "description": "A collection for testing", + "id": 1, + "type": "collection", + "breadcrumbs": [ + { + "display_name": "CS problems 2" + } + ], + "created": 1726740779.564664, + "modified": 1726740811.684142, + "usage_key": "lib-collection:OpenedX:CSPROB2:collection-from-meilisearch", + "context_key": "lib:OpenedX:CSPROB2", + "org": "OpenedX", + "access_id": 16, + "num_children": 5 + } + ], + "query": "", + "processingTimeMs": 0, + "limit": 1, + "offset": 0, + "estimatedTotalHits": 1 } ] } diff --git a/src/library-authoring/collections/CollectionInfoHeader.tsx b/src/library-authoring/collections/CollectionInfoHeader.tsx index dcd6b66abd..fda3f42eb9 100644 --- a/src/library-authoring/collections/CollectionInfoHeader.tsx +++ b/src/library-authoring/collections/CollectionInfoHeader.tsx @@ -1,12 +1,12 @@ -import { Collection } from '../data/api'; +import { type CollectionHit } from '../../search-manager/data/api'; interface CollectionInfoHeaderProps { - collection?: Collection; + collection?: CollectionHit; } const CollectionInfoHeader = ({ collection } : CollectionInfoHeaderProps) => (
- {collection?.title} + {collection?.displayName}
); diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index 63fbc2125a..129fd2fadb 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -1,4 +1,5 @@ import fetchMock from 'fetch-mock-jest'; +import { cloneDeep } from 'lodash'; import { fireEvent, initializeMocks, @@ -8,9 +9,8 @@ import { within, } from '../../testUtils'; import mockResult from '../__mocks__/collection-search.json'; -import mockEmptyResult from '../../search-modal/__mocks__/empty-search-result.json'; import { - mockCollection, mockContentLibrary, mockLibraryBlockTypes, mockXBlockFields, + mockContentLibrary, mockLibraryBlockTypes, mockXBlockFields, } from '../data/api.mocks'; import { mockContentSearchConfig } from '../../search-manager/data/api.mock'; import { mockBroadcastChannel } from '../../generic/data/api.mock'; @@ -18,31 +18,20 @@ import { LibraryLayout } from '..'; mockContentSearchConfig.applyMock(); mockContentLibrary.applyMock(); -mockCollection.applyMock(); mockLibraryBlockTypes.applyMock(); mockXBlockFields.applyMock(); mockBroadcastChannel(); const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; - -/** - * Returns 0 components from the search query. -*/ -const returnEmptyResult = (_url, 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 ... around matched words - // eslint-disable-next-line no-underscore-dangle, no-param-reassign - mockEmptyResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); - return mockEmptyResult; -}; - const path = '/library/:libraryId/*'; const libraryTitle = mockContentLibrary.libraryData.title; -const collectionTitle = mockCollection.collectionData.title; +const mockCollection = { + collectionId: mockResult.results[2].hits[0].block_id, + collectionNeverLoads: 'collection-always-loading', + collectionEmpty: 'collection-no-data', + collectionNoComponents: 'collection-no-components', + title: mockResult.results[2].hits[0].display_name, +}; describe('', () => { beforeEach(() => { @@ -52,14 +41,33 @@ describe('', () => { fetchMock.post(searchEndpoint, (_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); const query = requestData?.queries[0]?.q ?? ''; + const mockResultCopy = cloneDeep(mockResult); // We have to replace the query (search keywords) in the mock results with the actual query, // because otherwise Instantsearch will update the UI and change the query, // leading to unexpected results in the test cases. - mockResult.results[0].query = query; + mockResultCopy.results[0].query = query; + mockResultCopy.results[2].query = query; // And fake the required '_formatted' fields; it contains the highlighting ... around matched words // eslint-disable-next-line no-underscore-dangle, no-param-reassign - mockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); - return mockResult; + mockResultCopy.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); + const collectionQueryId = requestData?.queries[2]?.filter[2]?.split('block_id = "')[1].split('"')[0]; + switch (collectionQueryId) { + case mockCollection.collectionNeverLoads: + return new Promise(() => {}); + case mockCollection.collectionEmpty: + mockResultCopy.results[2].hits = []; + mockResultCopy.results[2].estimatedTotalHits = 0; + break; + case mockCollection.collectionNoComponents: + mockResultCopy.results[0].hits = []; + mockResultCopy.results[0].estimatedTotalHits = 0; + mockResultCopy.results[1].facetDistribution.block_type = {}; + mockResultCopy.results[2].hits[0].num_children = 0; + break; + default: + break; + } + return mockResultCopy; }); }); @@ -69,7 +77,7 @@ describe('', () => { }); const renderLibraryCollectionPage = async (collectionId?: string, libraryId?: string) => { - const libId = libraryId || mockCollection.libraryId; + const libId = libraryId || mockContentLibrary.libraryId; const colId = collectionId || mockCollection.collectionId; render(, { path, @@ -77,18 +85,23 @@ describe('', () => { initialEntries: [`/library/${libId}/collection/${colId}`], }, }); + + if (colId !== mockCollection.collectionNeverLoads) { + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + } }; it('shows the spinner before the query is complete', async () => { // This mock will never return data about the collection (it loads forever): - await renderLibraryCollectionPage(mockCollection.collectionIdThatNeverLoads); + await renderLibraryCollectionPage(mockCollection.collectionNeverLoads); const spinner = screen.getByRole('status'); expect(spinner.textContent).toEqual('Loading...'); }); it('shows an error component if no collection returned', async () => { - // This mock will simulate a 404 error: - await renderLibraryCollectionPage(mockCollection.collection404); + // This mock will simulate incorrect collection id + await renderLibraryCollectionPage(mockCollection.collectionEmpty); + screen.debug(); expect(await screen.findByTestId('notFoundAlert')).toBeInTheDocument(); }); @@ -96,7 +109,7 @@ describe('', () => { await renderLibraryCollectionPage(); expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[0]).toBeInTheDocument(); expect(screen.queryByText('This collection is currently empty.')).not.toBeInTheDocument(); @@ -108,12 +121,11 @@ describe('', () => { }); it('shows a collection without associated components', async () => { - fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); - await renderLibraryCollectionPage(); + await renderLibraryCollectionPage(mockCollection.collectionNoComponents); expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[0]).toBeInTheDocument(); expect(screen.getByText('This collection is currently empty.')).toBeInTheDocument(); @@ -125,7 +137,7 @@ describe('', () => { it('shows the new content button', async () => { await renderLibraryCollectionPage(); - expect(await screen.findByRole('heading')).toBeInTheDocument(); + expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect(await screen.findByText('Content (5)')).toBeInTheDocument(); expect(screen.getByRole('button', { name: /new/i })).toBeInTheDocument(); expect(screen.queryByText('Read Only')).not.toBeInTheDocument(); @@ -135,9 +147,7 @@ describe('', () => { // Use a library mock that is read-only: const libraryId = mockContentLibrary.libraryIdReadOnly; // Update search mock so it returns no results: - fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); - await renderLibraryCollectionPage(mockCollection.collectionId, libraryId); - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + await renderLibraryCollectionPage(mockCollection.collectionNoComponents, libraryId); expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect(screen.getByText('This collection is currently empty.')).toBeInTheDocument(); @@ -147,14 +157,11 @@ describe('', () => { it('show a collection without search results', async () => { // Update search mock so it returns no results: - fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); - await renderLibraryCollectionPage(); + await renderLibraryCollectionPage(mockCollection.collectionNoComponents); expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); - - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + expect((await screen.findAllByText(mockCollection.title))[0]).toBeInTheDocument(); fireEvent.change(screen.getByRole('searchbox'), { target: { value: 'noresults' } }); @@ -162,12 +169,11 @@ describe('', () => { // should not be impacted by the search await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); - expect(screen.getByText('No matching components found in this collections.')).toBeInTheDocument(); + expect(screen.queryByText('No matching components found in this collections.')).toBeInTheDocument(); }); it('should open and close new content sidebar', async () => { await renderLibraryCollectionPage(); - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect(screen.queryByText(/add content/i)).not.toBeInTheDocument(); @@ -188,8 +194,8 @@ describe('', () => { expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[1]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[1]).toBeInTheDocument(); expect(screen.getByText('Manage')).toBeInTheDocument(); expect(screen.getByText('Details')).toBeInTheDocument(); @@ -200,8 +206,8 @@ describe('', () => { expect(await screen.findByText('All Collections')).toBeInTheDocument(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(collectionTitle))[1]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(mockCollection.title))[1]).toBeInTheDocument(); // Open by default; close the library info sidebar const closeButton = screen.getByRole('button', { name: /close/i }); @@ -218,7 +224,6 @@ describe('', () => { it('sorts collection components', async () => { await renderLibraryCollectionPage(); - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); expect(await screen.findByTitle('Sort search results')).toBeInTheDocument(); @@ -310,9 +315,7 @@ describe('', () => { }); it('has an empty type filter when there are no results', async () => { - fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); - await renderLibraryCollectionPage(); - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + await renderLibraryCollectionPage(mockCollection.collectionNoComponents); const filterButton = screen.getByRole('button', { name: /type/i }); fireEvent.click(filterButton); diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 3e09289d4b..b2344a9b1f 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -13,6 +13,7 @@ import { import { Add, InfoOutline } from '@openedx/paragon/icons'; import { Link, useParams } from 'react-router-dom'; +import { SearchParams } from 'meilisearch'; import Loading from '../../generic/Loading'; import SubHeader from '../../generic/sub-header/SubHeader'; import Header from '../../header'; @@ -24,8 +25,9 @@ import { SearchContextProvider, SearchKeywordsField, SearchSortWidget, + useSearchContext, } from '../../search-manager'; -import { useCollection, useContentLibrary } from '../data/apiHooks'; +import { useContentLibrary } from '../data/apiHooks'; import { LibraryContext } from '../common/context'; import messages from './messages'; import { LibrarySidebar } from '../library-sidebar'; @@ -90,29 +92,24 @@ const SubHeaderTitle = ({ ); }; -const LibraryCollectionPage = () => { - const { libraryId, collectionId } = useParams(); - - if (!collectionId || !libraryId) { - // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. - throw new Error('Rendered without collectionId or libraryId URL parameter'); - } - +const LibraryCollectionPageInner = ({ libraryId }: { libraryId: string }) => { const intl = useIntl(); const { sidebarBodyComponent, openCollectionInfoSidebar, } = useContext(LibraryContext); + const { collectionHits: [collectionData], isFetching } = useSearchContext(); useEffect(() => { openCollectionInfoSidebar(); }, []); const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId); - const { data: collectionData, isLoading: isCollectionLoading } = useCollection(libraryId, collectionId); - if (isLibLoading || isCollectionLoading) { + // Only show loading if collection data is not fetched from index yet + // Loading info for search results will be handled by LibraryCollectionComponents component. + if (isLibLoading || (!collectionData && isFetching)) { return ; } @@ -147,37 +144,32 @@ const LibraryCollectionPage = () => { isLibrary /> - - - )} - breadcrumbs={( - - )} - headerActions={} - /> - -
- - - -
- -
- - + + )} + breadcrumbs={( + + )} + headerActions={} + /> + +
+ + + +
+ +
+
@@ -190,4 +182,27 @@ const LibraryCollectionPage = () => { ); }; +const LibraryCollectionPage = () => { + const { libraryId, collectionId } = useParams(); + + if (!collectionId || !libraryId) { + // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. + throw new Error('Rendered without collectionId or libraryId URL parameter'); + } + + const collectionQuery: SearchParams = { + filter: ['type = "collection"', `context_key = "${libraryId}"`, `block_id = "${collectionId}"`], + limit: 1, + }; + + return ( + + + + ); +}; + export default LibraryCollectionPage; diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 75f49bd150..0002f7516a 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -252,44 +252,3 @@ mockLibraryBlockMetadata.dataPublished = { } satisfies api.LibraryBlockMetadata; /** Apply this mock. Returns a spy object that can tell you if it's been called. */ mockLibraryBlockMetadata.applyMock = () => jest.spyOn(api, 'getLibraryBlockMetadata').mockImplementation(mockLibraryBlockMetadata); - -/** - * Mock for `getCollection()` - * - * This mock returns different data/responses depending on the ID of the library & collection - * that you request. - */ -export async function mockCollection(libraryId: string, collectionId: string): Promise { - // This mock has many different behaviors, depending on the collection ID: - switch (collectionId) { - case mockCollection.collectionIdThatNeverLoads: - // Return a promise that never resolves, to simulate never loading: - return new Promise(() => {}); - case mockCollection.collection404: - throw createAxiosError({ code: 400, message: 'Not found.', path: api.getLibraryCollectionApiUrl(libraryId, collectionId) }); - case mockCollection.collection500: - throw createAxiosError({ code: 500, message: 'Internal Error.', path: api.getLibraryCollectionApiUrl(libraryId, collectionId) }); - case mockCollection.collectionId: - return mockCollection.collectionData; - default: - throw new Error(`mockCollection: unknown collection ID "${collectionId}"`); - } -} -mockCollection.libraryId = 'lib:Axim:TEST'; -mockCollection.collectionId = 'my-first-collection'; -mockCollection.collectionData = { - // This is captured from a real API response: - id: 1, - key: mockCollection.collectionId, - title: 'My first collection', - description: 'A collection for testing', - created: '2024-06-26T14:19:59Z', - modified: '2024-07-20T17:36:51Z', - enabled: true, - createdBy: null, - learningPackage: 1, -} satisfies api.Collection; -mockCollection.collectionIdThatNeverLoads = 'infiniteLoading-collection'; -mockCollection.collection404 = 'collection-error404'; -mockCollection.collection500 = 'collection-error500'; -mockCollection.applyMock = () => jest.spyOn(api, 'getCollection').mockImplementation(mockCollection); diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 0ac62e556e..b1d6f34369 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -313,11 +313,3 @@ export async function getXBlockOLX(usageKey: string): Promise { const { data } = await getAuthenticatedHttpClient().get(getXBlockOLXApiUrl(usageKey)); return data.olx; } - -/** - * Fetch a collection by its ID. - */ -export async function getCollection(libraryId: string, collectionId: string): Promise { - const { data } = await getAuthenticatedHttpClient().get(getLibraryCollectionApiUrl(libraryId, collectionId)); - return camelCaseObject(data); -} diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 46eb958e91..08dee673e8 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -27,7 +27,6 @@ import { createCollection, getXBlockOLX, type CreateLibraryCollectionDataRequest, - getCollection, } from './api'; const libraryQueryPredicate = (query: Query, libraryId: string): boolean => { @@ -276,14 +275,3 @@ export const useXBlockOLX = (usageKey: string) => ( enabled: !!usageKey, }) ); - -/** - * Hook to fetch a collection by its ID. - */ -export const useCollection = (libraryId: string | undefined, collectionId: string | undefined) => ( - useQuery({ - queryKey: libraryAuthoringQueryKeys.collection(libraryId, collectionId), - queryFn: () => getCollection(libraryId!, collectionId!), - enabled: collectionId !== undefined && libraryId !== undefined, - }) -); diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index bf77d52a4a..d1ac43de22 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -11,12 +11,13 @@ import { AddContentContainer, AddContentHeader } from '../add-content'; import { LibraryContext, SidebarBodyComponentId } from '../common/context'; import { LibraryInfo, LibraryInfoHeader } from '../library-info'; import { ComponentInfo, ComponentInfoHeader } from '../component-info'; -import { ContentLibrary, Collection } from '../data/api'; +import { ContentLibrary } from '../data/api'; import { CollectionInfo, CollectionInfoHeader } from '../collections'; +import { type CollectionHit } from '../../search-manager/data/api'; type LibrarySidebarProps = { library: ContentLibrary, - collection?: Collection + collection?: CollectionHit, }; /** diff --git a/src/search-manager/SearchManager.ts b/src/search-manager/SearchManager.ts index c8d1aff766..bcce0779e7 100644 --- a/src/search-manager/SearchManager.ts +++ b/src/search-manager/SearchManager.ts @@ -10,7 +10,7 @@ import { MeiliSearch, type Filter } from 'meilisearch'; import { union } from 'lodash'; import { - CollectionHit, ContentHit, SearchSortOption, forceArray, + CollectionHit, ContentHit, SearchSortOption, forceArray, OverrideQueries, } from './data/api'; import { useContentSearchConnection, useContentSearchResults } from './data/apiHooks'; @@ -92,8 +92,8 @@ export const SearchContextProvider: React.FC<{ overrideSearchSortOrder?: SearchSortOption children: React.ReactNode, closeSearchModal?: () => void, - fetchCollections?: boolean, -}> = ({ overrideSearchSortOrder, fetchCollections, ...props }) => { + overrideQueries?: OverrideQueries, +}> = ({ overrideSearchSortOrder, overrideQueries, ...props }) => { const [searchKeywords, setSearchKeywords] = React.useState(''); const [blockTypesFilter, setBlockTypesFilter] = React.useState([]); const [problemTypesFilter, setProblemTypesFilter] = React.useState([]); @@ -144,7 +144,7 @@ export const SearchContextProvider: React.FC<{ problemTypesFilter, tagsFilter, sort, - fetchCollections, + overrideQueries, }); return React.createElement(SearchContext.Provider, { diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index 2f5de76d39..26db65bd3f 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -1,6 +1,8 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import type { Filter, MeiliSearch, MultiSearchQuery } from 'meilisearch'; +import type { + Filter, MeiliSearch, MultiSearchQuery, SearchParams, +} from 'meilisearch'; export const getContentSearchConfigUrl = () => new URL( 'api/content_search/v2/studio/', @@ -146,13 +148,32 @@ function formatSearchHit(hit: Record): ContentHit | CollectionHit { // eslint-disable-next-line @typescript-eslint/naming-convention const { _formatted, ...newHit } = hit; newHit.formatted = { - displayName: _formatted.display_name, - content: _formatted.content ?? {}, - description: _formatted.description, + displayName: _formatted?.display_name, + content: _formatted?.content ?? {}, + description: _formatted?.description, }; return camelCaseObject(newHit); } +export interface OverrideQueries { + components?: SearchParams, + collections?: SearchParams, +} + +function applyOverrideQueries( + queries: MultiSearchQuery[], + overrideQueries?: OverrideQueries, +): MultiSearchQuery[] { + const newQueries = [...queries]; + if (overrideQueries?.components) { + newQueries[0] = { ...overrideQueries.components, indexUid: queries[0].indexUid }; + } + if (overrideQueries?.collections) { + newQueries[2] = { ...overrideQueries.collections, indexUid: queries[2].indexUid }; + } + return newQueries; +} + interface FetchSearchParams { client: MeiliSearch, indexName: string, @@ -165,7 +186,7 @@ interface FetchSearchParams { sort?: SearchSortOption[], /** How many results to skip, e.g. if limit=20 then passing offset=20 gets the second page. */ offset?: number, - fetchCollections?: boolean, + overrideQueries?: OverrideQueries, } export async function fetchSearchResults({ @@ -177,8 +198,8 @@ export async function fetchSearchResults({ tagsFilter, extraFilter, sort, + overrideQueries, offset = 0, - fetchCollections = false, }: FetchSearchParams): Promise<{ hits: ContentHit[], nextOffset: number | undefined, @@ -188,7 +209,7 @@ export async function fetchSearchResults({ collectionHits: CollectionHit[], totalCollectionHits: number, }> { - const queries: MultiSearchQuery[] = []; + let queries: MultiSearchQuery[] = []; // Convert 'extraFilter' into an array const extraFilterFormatted = forceArray(extraFilter); @@ -245,41 +266,40 @@ export async function fetchSearchResults({ }); // Third query is to get the hits for collections, with all the filters applied. - if (fetchCollections) { - queries.push({ - indexUid: indexName, - q: searchKeywords, - filter: [ - // top-level entries in the array are AND conditions and must all match - // Inner arrays are OR conditions, where only one needs to match. - collectionsFilter, // include only collections - ...extraFilterFormatted, - // We exclude the block type filter as collections are only of 1 type i.e. collection. - ...tagsFilterFormatted, - ], - attributesToHighlight: ['display_name', 'description'], - highlightPreTag: HIGHLIGHT_PRE_TAG, - highlightPostTag: HIGHLIGHT_POST_TAG, - attributesToCrop: ['description'], - cropLength: 15, - sort, - offset, - limit, - }); - } + queries.push({ + indexUid: indexName, + q: searchKeywords, + filter: [ + // top-level entries in the array are AND conditions and must all match + // Inner arrays are OR conditions, where only one needs to match. + collectionsFilter, // include only collections + ...extraFilterFormatted, + // We exclude the block type filter as collections are only of 1 type i.e. collection. + ...tagsFilterFormatted, + ], + attributesToHighlight: ['display_name', 'description'], + highlightPreTag: HIGHLIGHT_PRE_TAG, + highlightPostTag: HIGHLIGHT_POST_TAG, + attributesToCrop: ['description'], + cropLength: 15, + sort, + offset, + limit, + }); + + queries = applyOverrideQueries(queries, overrideQueries); const { results } = await client.multiSearch(({ queries })); const componentHitLength = results[0].hits.length; - const collectionHitLength = fetchCollections ? results[2].hits.length : 0; + const collectionHitLength = results[2].hits.length; return { hits: results[0].hits.map(formatSearchHit) as ContentHit[], totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? componentHitLength, blockTypes: results[1].facetDistribution?.block_type ?? {}, problemTypes: results[1].facetDistribution?.['content.problem_types'] ?? {}, nextOffset: componentHitLength === limit || collectionHitLength === limit ? offset + limit : undefined, - collectionHits: fetchCollections ? results[2].hits.map(formatSearchHit) as CollectionHit[] : [], - totalCollectionHits: fetchCollections - ? results[2].totalHits ?? results[2].estimatedTotalHits ?? collectionHitLength : 0, + collectionHits: results[2].hits.map(formatSearchHit) as CollectionHit[], + totalCollectionHits: results[2].totalHits ?? results[2].estimatedTotalHits ?? collectionHitLength, }; } diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index c9fd79e83f..c2a330c1e0 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -10,6 +10,7 @@ import { fetchTagsThatMatchKeyword, getContentSearchConfig, fetchDocumentById, + OverrideQueries, } from './api'; /** @@ -55,7 +56,7 @@ export const useContentSearchResults = ({ problemTypesFilter = [], tagsFilter = [], sort = [], - fetchCollections = false, + overrideQueries, }: { /** The Meilisearch API client */ client?: MeiliSearch; @@ -74,7 +75,7 @@ export const useContentSearchResults = ({ /** Sort search results using these options */ sort?: SearchSortOption[]; /** Set true to fetch collections along with components */ - fetchCollections?: boolean; + overrideQueries?: OverrideQueries, }) => { const query = useInfiniteQuery({ enabled: client !== undefined && indexName !== undefined, @@ -85,12 +86,12 @@ export const useContentSearchResults = ({ client?.config.host, indexName, extraFilter, - fetchCollections, searchKeywords, blockTypesFilter, problemTypesFilter, tagsFilter, sort, + overrideQueries, ], queryFn: ({ pageParam = 0 }) => { if (client === undefined || indexName === undefined) { @@ -108,7 +109,7 @@ export const useContentSearchResults = ({ // For infinite pagination of results, we can retrieve additional pages if requested. // Note that if there are 20 results per page, the "second page" has offset=20, not 2. offset: pageParam, - fetchCollections, + overrideQueries, }); }, getNextPageParam: (lastPage) => lastPage.nextOffset,