From cdb633be5d19fe3c98a6f6313e5d1e854f30256f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 11 Sep 2024 20:27:11 +0530 Subject: [PATCH 01/22] feat: collections bare bones page --- .../LibraryAuthoringPage.scss | 5 + .../LibraryAuthoringPage.tsx | 1 + .../LibraryCollectionPage.tsx | 196 ++++++++++++++++++ src/library-authoring/LibraryLayout.tsx | 5 + .../LibraryRecentlyModified.tsx | 1 + src/library-authoring/data/api.ts | 27 +++ src/library-authoring/data/apiHooks.ts | 14 ++ src/library-authoring/messages.ts | 10 + src/search-manager/SearchManager.ts | 13 +- src/search-manager/data/api.ts | 50 +++-- src/search-manager/data/apiHooks.ts | 29 ++- 11 files changed, 314 insertions(+), 37 deletions(-) create mode 100644 src/library-authoring/LibraryCollectionPage.tsx diff --git a/src/library-authoring/LibraryAuthoringPage.scss b/src/library-authoring/LibraryAuthoringPage.scss index 9680b8062b..c909627441 100644 --- a/src/library-authoring/LibraryAuthoringPage.scss +++ b/src/library-authoring/LibraryAuthoringPage.scss @@ -20,3 +20,8 @@ height: 100vh; overflow-y: auto; } + +// Reduce breadcrumb bottom margin +ol.list-inline { + margin-bottom: 0rem; +} diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 3f0a680a3b..0a0bf6cce2 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -168,6 +168,7 @@ const LibraryAuthoringPage = () => { } diff --git a/src/library-authoring/LibraryCollectionPage.tsx b/src/library-authoring/LibraryCollectionPage.tsx new file mode 100644 index 0000000000..e80ca565ea --- /dev/null +++ b/src/library-authoring/LibraryCollectionPage.tsx @@ -0,0 +1,196 @@ +import React, { useContext } from 'react'; +import classNames from 'classnames'; +import { StudioFooter } from '@edx/frontend-component-footer'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + Badge, + Button, + Breadcrumb, + Container, + Icon, + IconButton, + Stack, +} from '@openedx/paragon'; +import { Add, InfoOutline } from '@openedx/paragon/icons'; +import { Link, useParams } from 'react-router-dom'; + +import Loading from '../generic/Loading'; +import SubHeader from '../generic/sub-header/SubHeader'; +import Header from '../header'; +import NotFoundAlert from '../generic/NotFoundAlert'; +import { + ClearFiltersButton, + FilterByBlockType, + FilterByTags, + SearchContextProvider, + SearchKeywordsField, + SearchSortWidget, +} from '../search-manager'; +import { useCollection, useContentLibrary } from './data/apiHooks'; +import { LibraryContext, SidebarBodyComponentId } from './common/context'; +import messages from './messages'; + +interface HeaderActionsProps { + canEditLibrary: boolean; +} + +const HeaderActions = ({ canEditLibrary }: HeaderActionsProps) => { + const intl = useIntl(); + const { + openAddContentSidebar, + openInfoSidebar, + closeLibrarySidebar, + sidebarBodyComponent, + } = useContext(LibraryContext); + + if (!canEditLibrary) { + return null; + } + + const infoSidebarIsOpen = () => ( + sidebarBodyComponent === SidebarBodyComponentId.Info + ); + + const handleOnClickInfoSidebar = () => { + if (infoSidebarIsOpen()) { + closeLibrarySidebar(); + } else { + openInfoSidebar(); + } + }; + + return ( +
+ + +
+ ); +}; + +const SubHeaderTitle = ({ title, canEditLibrary }: { title: string, canEditLibrary: boolean }) => { + const intl = useIntl(); + + return ( + + + {title} + {}} + variant="primary" + /> + + { !canEditLibrary && ( +
+ + {intl.formatMessage(messages.readOnlyBadge)} + +
+ )} + + ); +}; + +const LibraryCollectionPage = ({ libraryId, collectionId }: { libraryId: string, collectionId: string }) => { + const intl = useIntl(); + + const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId); + const { data: collectionData, isLoading: isCollectionLoading } = useCollection(libraryId, collectionId); + + if (isLibLoading || isCollectionLoading) { + return ; + } + + if (!libraryData || !collectionData) { + return ; + } + + const breadcrumbs = [ + { + label: libraryData.title, + to: `/library/${libraryId}`, + }, + { + label: intl.formatMessage(messages.allCollections), + to: `/library/${libraryId}/collections`, + }, + // Adding empty breadcrumb to add the last `>` spacer. + { + label: '', + to: ``, + }, + ]; + + return ( +
+
+
+ + } + breadcrumbs={( + + )} + headerActions={} + /> + +
+ + + +
+ +
+ + +
+
+ ); +}; + +const LibraryCollectionPageWrapper = () => { + const { libraryId, collectionId } = useParams(); + if (!collectionId || !libraryId) { + throw new Error('Rendered without collectionId or libraryId URL parameter'); + } + + return ( + + + + ); +} + +export default LibraryCollectionPageWrapper; diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 59ee11797c..7298e10530 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -13,6 +13,7 @@ import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; import { invalidateComponentData } from './data/apiHooks'; +import LibraryCollectionPageWrapper from './LibraryCollectionPage'; const LibraryLayout = () => { const { libraryId } = useParams(); @@ -58,6 +59,10 @@ const LibraryLayout = () => { )} /> + } + /> } diff --git a/src/library-authoring/LibraryRecentlyModified.tsx b/src/library-authoring/LibraryRecentlyModified.tsx index 57828871ef..94e4196fc5 100644 --- a/src/library-authoring/LibraryRecentlyModified.tsx +++ b/src/library-authoring/LibraryRecentlyModified.tsx @@ -81,6 +81,7 @@ const LibraryRecentlyModified: React.FC<{ libraryId: string }> = ({ libraryId }) diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 76388b1641..4222e16d44 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -50,6 +50,10 @@ export const getXBlockOLXApiUrl = (usageKey: string) => `${getApiBaseUrl()}/api/ * Get the URL for the Library Collections API. */ export const getLibraryCollectionsApiUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/collections/`; +/** + * Get the URL for the collection API. + */ +export const getLibraryCollectionApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionsApiUrl(libraryId)}${collectionId}/`; export interface ContentLibrary { id: string; @@ -75,6 +79,20 @@ export interface ContentLibrary { updated: string | null; } +export interface Collection { + id: number; + key: string; + title: string; + description: string; + enabled: boolean; + createdBy: string | null; + created: string; + modified: string; + // TODO: Update the type below once entities are properly linked + entities: Array; + learningPackage: number; +} + export interface LibraryBlockType { blockType: string; displayName: string; @@ -294,3 +312,12 @@ 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 efbd59efc0..53a553218c 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -27,6 +27,7 @@ import { createCollection, getXBlockOLX, type CreateLibraryCollectionDataRequest, + getCollection, } from './api'; const libraryQueryPredicate = (query: Query, libraryId: string): boolean => { @@ -61,6 +62,7 @@ export const libraryAuthoringQueryKeys = { 'content', 'libraryBlockTypes', ], + collection: (collectionId?: string) => [...libraryAuthoringQueryKeys.all, collectionId], }; export const xblockQueryKeys = { @@ -270,3 +272,15 @@ 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.contentLibrary(collectionId), + queryFn: () => getCollection(libraryId!, collectionId!), + enabled: collectionId !== undefined && libraryId !== undefined, + }) +); + diff --git a/src/library-authoring/messages.ts b/src/library-authoring/messages.ts index d086d6b565..8da572a824 100644 --- a/src/library-authoring/messages.ts +++ b/src/library-authoring/messages.ts @@ -6,6 +6,11 @@ const messages = defineMessages({ defaultMessage: 'Content library', description: 'The page heading for the library page.', }, + allCollections: { + id: 'course-authoring.library-authoring.all-collections', + defaultMessage: 'All Collections', + description: 'Breadcrumbs text to navigate back to all collections', + }, headingInfoAlt: { id: 'course-authoring.library-authoring.heading-info-alt', defaultMessage: 'Info', @@ -116,6 +121,11 @@ const messages = defineMessages({ defaultMessage: 'Library Info', description: 'Text of button to open "Library Info sidebar"', }, + collectionInfoButton: { + id: 'course-authoring.library-authoring.buttons.collection-info.alt-text', + defaultMessage: 'Collection Info', + description: 'Alt text for collection info button besides the collection title', + }, readOnlyBadge: { id: 'course-authoring.library-authoring.badge.read-only', defaultMessage: 'Read Only', diff --git a/src/search-manager/SearchManager.ts b/src/search-manager/SearchManager.ts index d1e925a91a..380f62af1b 100644 --- a/src/search-manager/SearchManager.ts +++ b/src/search-manager/SearchManager.ts @@ -91,7 +91,8 @@ export const SearchContextProvider: React.FC<{ overrideSearchSortOrder?: SearchSortOption children: React.ReactNode, closeSearchModal?: () => void, -}> = ({ overrideSearchSortOrder, ...props }) => { + fetchCollections?: boolean, +}> = ({ overrideSearchSortOrder, fetchCollections, ...props }) => { const [searchKeywords, setSearchKeywords] = React.useState(''); const [blockTypesFilter, setBlockTypesFilter] = React.useState([]); const [problemTypesFilter, setProblemTypesFilter] = React.useState([]); @@ -130,14 +131,7 @@ export const SearchContextProvider: React.FC<{ }, []); // Initialize a connection to Meilisearch: - const { data: connectionDetails, isError: hasConnectionError } = useContentSearchConnection(); - const indexName = connectionDetails?.indexName; - const client = React.useMemo(() => { - if (connectionDetails?.apiKey === undefined || connectionDetails?.url === undefined) { - return undefined; - } - return new MeiliSearch({ host: connectionDetails.url, apiKey: connectionDetails.apiKey }); - }, [connectionDetails?.apiKey, connectionDetails?.url]); + const { client, indexName, hasConnectionError } = useContentSearchConnection(); // Run the search const result = useContentSearchResults({ @@ -149,6 +143,7 @@ export const SearchContextProvider: React.FC<{ problemTypesFilter, tagsFilter, sort, + fetchCollections, }); return React.createElement(SearchContext.Provider, { diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index 6dbb805e74..77e73a4ab4 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -165,6 +165,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, } export async function fetchSearchResults({ @@ -177,6 +178,7 @@ export async function fetchSearchResults({ extraFilter, sort, offset = 0, + fetchCollections = false, }: FetchSearchParams): Promise<{ hits: ContentHit[], nextOffset: number | undefined, @@ -243,26 +245,28 @@ export async function fetchSearchResults({ }); // Third query is to get the hits for collections, with all the filters applied. - 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, - }); + 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, + }); + } const { results } = await client.multiSearch(({ queries })); return { @@ -270,9 +274,9 @@ export async function fetchSearchResults({ totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? results[0].hits.length, blockTypes: results[1].facetDistribution?.block_type ?? {}, problemTypes: results[1].facetDistribution?.['content.problem_types'] ?? {}, - nextOffset: results[0].hits.length === limit || results[2].hits.length === limit ? offset + limit : undefined, - collectionHits: results[2].hits.map(formatSearchHit) as CollectionHit[], - totalCollectionHits: results[2].totalHits ?? results[2].estimatedTotalHits ?? results[2].hits.length, + nextOffset: results[0].hits.length === limit || (fetchCollections && results[2].hits.length === limit) ? offset + limit : undefined, + collectionHits: fetchCollections ? results[2].hits.map(formatSearchHit) as CollectionHit[] : [], + totalCollectionHits: fetchCollections ? results[2].totalHits ?? results[2].estimatedTotalHits ?? results[2].hits.length: 0, }; } diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index 703251e6ee..94c5eae320 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -1,6 +1,6 @@ import React from 'react'; import { useInfiniteQuery, useQuery } from '@tanstack/react-query'; -import type { Filter, MeiliSearch } from 'meilisearch'; +import { type Filter, MeiliSearch } from 'meilisearch'; import { SearchSortOption, @@ -9,6 +9,7 @@ import { fetchSearchResults, fetchTagsThatMatchKeyword, getContentSearchConfig, + fetchDocumentById, } from './api'; /** @@ -16,8 +17,12 @@ import { * to the current user that allows it to search all content he have permission to view. * */ -export const useContentSearchConnection = () => ( - useQuery({ +export const useContentSearchConnection = (): { + client?: MeiliSearch, + indexName?: string, + hasConnectionError: boolean; +} => { + const { data: connectionDetails, isError: hasConnectionError } = useQuery({ queryKey: ['content_search'], queryFn: getContentSearchConfig, cacheTime: 60 * 60_000, // Even if we're not actively using the search modal, keep it in memory up to an hour @@ -25,8 +30,18 @@ export const useContentSearchConnection = () => ( refetchInterval: 60 * 60_000, refetchOnWindowFocus: false, // This doesn't need to be refreshed when the user switches back to this tab. refetchOnMount: false, - }) -); + }); + + const indexName = connectionDetails?.indexName; + const client = React.useMemo(() => { + if (connectionDetails?.apiKey === undefined || connectionDetails?.url === undefined) { + return undefined; + } + return new MeiliSearch({ host: connectionDetails.url, apiKey: connectionDetails.apiKey }); + }, [connectionDetails?.apiKey, connectionDetails?.url]); + + return { client, indexName, hasConnectionError }; +}; /** * Get the results of a search @@ -40,6 +55,7 @@ export const useContentSearchResults = ({ problemTypesFilter = [], tagsFilter = [], sort = [], + fetchCollections = false, }: { /** The Meilisearch API client */ client?: MeiliSearch; @@ -57,6 +73,8 @@ export const useContentSearchResults = ({ tagsFilter?: string[]; /** Sort search results using these options */ sort?: SearchSortOption[]; + /** Set true to fetch collections along with components */ + fetchCollections?: boolean; }) => { const query = useInfiniteQuery({ enabled: client !== undefined && indexName !== undefined, @@ -89,6 +107,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, }); }, getNextPageParam: (lastPage) => lastPage.nextOffset, From 0f755d9beddb0624830f48a7f82e3cbbbd3ace9e Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 11 Sep 2024 20:27:38 +0530 Subject: [PATCH 02/22] feat: hook to fetch single meilisearch document --- src/search-manager/data/api.ts | 15 +++++++++++++++ src/search-manager/data/apiHooks.ts | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index 77e73a4ab4..064b3dd2c3 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -491,3 +491,18 @@ export async function fetchTagsThatMatchKeyword({ return { matches: Array.from(matches).map((tagPath) => ({ tagPath })), mayBeMissingResults: hits.length === limit }; } + +/** + * Fetch single document by its id + */ +export async function fetchDocumentById({ client, indexName, id } : { + /** The Meilisearch client instance */ + client: MeiliSearch; + /** Which index to search */ + indexName: string; + /** document id */ + id: string | number; +}): Promise { + const doc = await client.index(indexName).getDocument(id); + return formatSearchHit(doc); +} diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index 94c5eae320..d9ba0390c1 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -240,3 +240,26 @@ export const useTagFilterOptions = (args: { return { ...mainQuery, data }; }; + +export const useGetSingleDocument = ({ client, indexName, id }: { + client?: MeiliSearch; + indexName?: string; + id: string | number; +}) => ( + useQuery({ + enabled: client !== undefined && indexName !== undefined, + queryKey: [ + 'content_search', + client?.config.apiKey, + client?.config.host, + indexName, + id, + ], + queryFn: () => { + if (client === undefined || indexName === undefined) { + throw new Error('Required data unexpectedly undefined. Check "enable" condition of useQuery.'); + } + return fetchDocumentById({ client, indexName, id }); + }, + }) +) From 3f6a32ea773864e5cc78ba80a1c25344d0f7426b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 12 Sep 2024 15:19:52 +0530 Subject: [PATCH 03/22] feat: collection sidebar --- .../LibraryAuthoringPage.tsx | 2 +- src/library-authoring/LibraryHome.tsx | 2 +- src/library-authoring/LibraryLayout.tsx | 1 + .../collections/CollectionInfo.tsx | 47 +++++++++++++ .../LibraryCollectionPage.tsx | 70 ++++++++----------- .../{ => collections}/LibraryCollections.tsx | 10 +-- src/library-authoring/collections/index.tsx | 1 + src/library-authoring/collections/messages.ts | 16 +++++ src/library-authoring/common/context.tsx | 13 ++++ .../library-sidebar/LibrarySidebar.tsx | 13 +++- 10 files changed, 126 insertions(+), 49 deletions(-) create mode 100644 src/library-authoring/collections/CollectionInfo.tsx rename src/library-authoring/{ => collections}/LibraryCollectionPage.tsx (74%) rename src/library-authoring/{ => collections}/LibraryCollections.tsx (79%) create mode 100644 src/library-authoring/collections/index.tsx create mode 100644 src/library-authoring/collections/messages.ts diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 0a0bf6cce2..019527b679 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -29,7 +29,7 @@ import { SearchSortWidget, } from '../search-manager'; import LibraryComponents from './components/LibraryComponents'; -import LibraryCollections from './LibraryCollections'; +import LibraryCollections from './collections/LibraryCollections'; import LibraryHome from './LibraryHome'; import { useContentLibrary } from './data/apiHooks'; import { LibrarySidebar } from './library-sidebar'; diff --git a/src/library-authoring/LibraryHome.tsx b/src/library-authoring/LibraryHome.tsx index 73d0e44ad5..4b170b96fa 100644 --- a/src/library-authoring/LibraryHome.tsx +++ b/src/library-authoring/LibraryHome.tsx @@ -4,7 +4,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { useSearchContext } from '../search-manager'; import { NoComponents, NoSearchResults } from './EmptyStates'; -import LibraryCollections from './LibraryCollections'; +import LibraryCollections from './collections/LibraryCollections'; import { LibraryComponents } from './components'; import LibrarySection from './components/LibrarySection'; import LibraryRecentlyModified from './LibraryRecentlyModified'; diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 7298e10530..a69baa9859 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -14,6 +14,7 @@ import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; import { invalidateComponentData } from './data/apiHooks'; import LibraryCollectionPageWrapper from './LibraryCollectionPage'; +import LibraryCollectionPageWrapper from './collections/LibraryCollectionPage'; const LibraryLayout = () => { const { libraryId } = useParams(); diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx new file mode 100644 index 0000000000..432d2d9185 --- /dev/null +++ b/src/library-authoring/collections/CollectionInfo.tsx @@ -0,0 +1,47 @@ +import React from 'react'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + Tab, + Tabs, + Stack, +} from '@openedx/paragon'; + +import messages from './messages'; +import { useCollection } from '../data/apiHooks'; +import Loading from '../../generic/Loading'; + +interface CollectionInfoProps { + collectionId: string; + libraryId: string; +} + +const CollectionInfo = ({ libraryId, collectionId } : CollectionInfoProps) => { + const intl = useIntl(); + const { data: collectionData, isLoading: isCollectionLoading } = useCollection(libraryId, collectionId); + + if (isCollectionLoading) { + return ; + } + + return ( + +
+ {collectionData?.title} +
+ + + Manage tab placeholder + + + Details tab placeholder + + + + ); +}; + +export default CollectionInfo; diff --git a/src/library-authoring/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx similarity index 74% rename from src/library-authoring/LibraryCollectionPage.tsx rename to src/library-authoring/collections/LibraryCollectionPage.tsx index e80ca565ea..8eea41eac9 100644 --- a/src/library-authoring/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -1,5 +1,4 @@ -import React, { useContext } from 'react'; -import classNames from 'classnames'; +import React, { useContext, useEffect } from 'react'; import { StudioFooter } from '@edx/frontend-component-footer'; import { useIntl } from '@edx/frontend-platform/i18n'; import { @@ -14,10 +13,10 @@ import { import { Add, InfoOutline } from '@openedx/paragon/icons'; import { Link, useParams } from 'react-router-dom'; -import Loading from '../generic/Loading'; -import SubHeader from '../generic/sub-header/SubHeader'; -import Header from '../header'; -import NotFoundAlert from '../generic/NotFoundAlert'; +import Loading from '../../generic/Loading'; +import SubHeader from '../../generic/sub-header/SubHeader'; +import Header from '../../header'; +import NotFoundAlert from '../../generic/NotFoundAlert'; import { ClearFiltersButton, FilterByBlockType, @@ -25,10 +24,11 @@ import { SearchContextProvider, SearchKeywordsField, SearchSortWidget, -} from '../search-manager'; -import { useCollection, useContentLibrary } from './data/apiHooks'; -import { LibraryContext, SidebarBodyComponentId } from './common/context'; -import messages from './messages'; +} from '../../search-manager'; +import { useCollection, useContentLibrary } from '../data/apiHooks'; +import { LibraryContext } from '../common/context'; +import messages from '../messages'; +import { LibrarySidebar } from '../library-sidebar'; interface HeaderActionsProps { canEditLibrary: boolean; @@ -38,40 +38,14 @@ const HeaderActions = ({ canEditLibrary }: HeaderActionsProps) => { const intl = useIntl(); const { openAddContentSidebar, - openInfoSidebar, - closeLibrarySidebar, - sidebarBodyComponent, } = useContext(LibraryContext); if (!canEditLibrary) { return null; } - const infoSidebarIsOpen = () => ( - sidebarBodyComponent === SidebarBodyComponentId.Info - ); - - const handleOnClickInfoSidebar = () => { - if (infoSidebarIsOpen()) { - closeLibrarySidebar(); - } else { - openInfoSidebar(); - } - }; - return (
-
+ { !!sidebarBodyComponent && ( +
+ +
+ )}
); }; diff --git a/src/library-authoring/LibraryCollections.tsx b/src/library-authoring/collections/LibraryCollections.tsx similarity index 79% rename from src/library-authoring/LibraryCollections.tsx rename to src/library-authoring/collections/LibraryCollections.tsx index f8e1bf56b2..05f6950d65 100644 --- a/src/library-authoring/LibraryCollections.tsx +++ b/src/library-authoring/collections/LibraryCollections.tsx @@ -1,8 +1,8 @@ -import { useLoadOnScroll } from '../hooks'; -import { useSearchContext } from '../search-manager'; -import { NoComponents, NoSearchResults } from './EmptyStates'; -import CollectionCard from './components/CollectionCard'; -import { LIBRARY_SECTION_PREVIEW_LIMIT } from './components/LibrarySection'; +import { useLoadOnScroll } from '../../hooks'; +import { useSearchContext } from '../../search-manager'; +import { NoComponents, NoSearchResults } from '../EmptyStates'; +import CollectionCard from '../components/CollectionCard'; +import { LIBRARY_SECTION_PREVIEW_LIMIT } from '../components/LibrarySection'; type LibraryCollectionsProps = { variant: 'full' | 'preview', diff --git a/src/library-authoring/collections/index.tsx b/src/library-authoring/collections/index.tsx new file mode 100644 index 0000000000..d41c2f9381 --- /dev/null +++ b/src/library-authoring/collections/index.tsx @@ -0,0 +1 @@ +export { default as CollectionInfo } from './CollectionInfo'; diff --git a/src/library-authoring/collections/messages.ts b/src/library-authoring/collections/messages.ts new file mode 100644 index 0000000000..b60ca26801 --- /dev/null +++ b/src/library-authoring/collections/messages.ts @@ -0,0 +1,16 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + manageTabTitle: { + id: 'course-authoring.library-authoring.collections-sidebar.manage-tab.title', + defaultMessage: 'Manage', + description: 'Title for manage tab', + }, + detailsTabTitle: { + id: 'course-authoring.library-authoring.collections-sidebar.details-tab.title', + defaultMessage: 'Details', + description: 'Title for details tab', + }, +}); + +export default messages; diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index 6ad0f29b58..71b7c05383 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -5,6 +5,7 @@ export enum SidebarBodyComponentId { AddContent = 'add-content', Info = 'info', ComponentInfo = 'component-info', + CollectionInfo = 'collection-info', } export interface LibraryContextData { @@ -17,6 +18,7 @@ export interface LibraryContextData { isCreateCollectionModalOpen: boolean; openCreateCollectionModal: () => void; closeCreateCollectionModal: () => void; + openCollectionInfoSidebar: (collectionId: string) => void; } export const LibraryContext = React.createContext({ @@ -28,6 +30,7 @@ export const LibraryContext = React.createContext({ isCreateCollectionModalOpen: false, openCreateCollectionModal: () => {}, closeCreateCollectionModal: () => {}, + openCollectionInfoSidebar: (_collectionId: string) => {}, // eslint-disable-line @typescript-eslint/no-unused-vars } as LibraryContextData); /** @@ -37,6 +40,7 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { const [sidebarBodyComponent, setSidebarBodyComponent] = React.useState(null); const [currentComponentUsageKey, setCurrentComponentUsageKey] = React.useState(); const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); + const [currentComponentKey, setCurrentComponentKey] = React.useState(); const closeLibrarySidebar = React.useCallback(() => { setSidebarBodyComponent(null); @@ -57,6 +61,13 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { }, [], ); + const openCollectionInfoSidebar = React.useCallback( + (collectionId: string) => { + setCurrentComponentKey(collectionId); + setSidebarBodyComponent(SidebarBodyComponentId.CollectionInfo); + }, + [], + ); const context = React.useMemo(() => ({ sidebarBodyComponent, @@ -68,6 +79,7 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal, + openCollectionInfoSidebar, }), [ sidebarBodyComponent, closeLibrarySidebar, @@ -78,6 +90,7 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal, + openCollectionInfoSidebar, ]); return ( diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index 6ced6ba50e..9808e595b3 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -12,6 +12,7 @@ import { LibraryContext, SidebarBodyComponentId } from '../common/context'; import { LibraryInfo, LibraryInfoHeader } from '../library-info'; import { ComponentInfo, ComponentInfoHeader } from '../component-info'; import { ContentLibrary } from '../data/api'; +import { CollectionInfo } from '../collections'; type LibrarySidebarProps = { library: ContentLibrary, @@ -31,14 +32,17 @@ const LibrarySidebar = ({ library }: LibrarySidebarProps) => { const { sidebarBodyComponent, closeLibrarySidebar, - currentComponentUsageKey, + currentComponentKey, } = useContext(LibraryContext); const bodyComponentMap = { [SidebarBodyComponentId.AddContent]: , [SidebarBodyComponentId.Info]: , [SidebarBodyComponentId.ComponentInfo]: ( - currentComponentUsageKey && + currentComponentKey && + ), + [SidebarBodyComponentId.CollectionInfo]: ( + currentComponentKey && ), unknown: null, }; @@ -47,8 +51,11 @@ const LibrarySidebar = ({ library }: LibrarySidebarProps) => { [SidebarBodyComponentId.AddContent]: , [SidebarBodyComponentId.Info]: , [SidebarBodyComponentId.ComponentInfo]: ( - currentComponentUsageKey && + currentComponentKey && ), + //[SidebarBodyComponentId.CollectionInfo]: ( + // currentComponentKey && + //), unknown: null, }; From ac6709dd55d6c1f7ca865f722dac05c83ba8993a Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 12 Sep 2024 20:17:34 +0530 Subject: [PATCH 04/22] feat: components in collections --- src/library-authoring/EmptyStates.tsx | 31 ++++++------- .../collections/CollectionInfo.tsx | 45 ++++++------------- .../collections/CollectionInfoHeader.tsx | 15 +++++++ .../LibraryCollectionComponents.tsx | 24 ++++++++++ .../collections/LibraryCollectionPage.tsx | 40 +++++++++++------ .../collections/LibraryCollections.tsx | 5 ++- src/library-authoring/collections/index.tsx | 1 + src/library-authoring/collections/messages.ts | 15 +++++++ src/library-authoring/common/context.tsx | 15 +++---- .../library-sidebar/LibrarySidebar.tsx | 15 +++---- 10 files changed, 127 insertions(+), 79 deletions(-) create mode 100644 src/library-authoring/collections/CollectionInfoHeader.tsx create mode 100644 src/library-authoring/collections/LibraryCollectionComponents.tsx diff --git a/src/library-authoring/EmptyStates.tsx b/src/library-authoring/EmptyStates.tsx index 39e43ed1ed..a1ff3bb766 100644 --- a/src/library-authoring/EmptyStates.tsx +++ b/src/library-authoring/EmptyStates.tsx @@ -1,6 +1,7 @@ import React, { useContext, useCallback } from 'react'; import { useParams } from 'react-router'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import type { MessageDescriptor } from 'react-intl'; import { Button, Stack, } from '@openedx/paragon'; @@ -10,11 +11,13 @@ import messages from './messages'; import { LibraryContext } from './common/context'; import { useContentLibrary } from './data/apiHooks'; -type NoSearchResultsProps = { - searchType?: 'collection' | 'component', -}; - -export const NoComponents = ({ searchType = 'component' }: NoSearchResultsProps) => { +export const NoComponents = ({ + infoText = messages.noComponents, + addBtnText = messages.addComponent, +}: { + infoText: MessageDescriptor; + addBtnText: MessageDescriptor; +}) => { const { openAddContentSidebar, openCreateCollectionModal } = useContext(LibraryContext); const { libraryId } = useParams(); const { data: libraryData } = useContentLibrary(libraryId); @@ -30,25 +33,23 @@ export const NoComponents = ({ searchType = 'component' }: NoSearchResultsProps) return ( - {searchType === 'collection' - ? - : } + {canEditLibrary && ( )} ); }; -export const NoSearchResults = ({ searchType = 'component' }: NoSearchResultsProps) => ( +export const NoSearchResults = ({ + infoText = messages.noSearchResults, +}: { + infoText: MessageDescriptor; +}) => ( - {searchType === 'collection' - ? - : } + ); diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx index 432d2d9185..4c1f40e732 100644 --- a/src/library-authoring/collections/CollectionInfo.tsx +++ b/src/library-authoring/collections/CollectionInfo.tsx @@ -1,46 +1,27 @@ -import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Tab, Tabs, - Stack, } from '@openedx/paragon'; import messages from './messages'; -import { useCollection } from '../data/apiHooks'; -import Loading from '../../generic/Loading'; -interface CollectionInfoProps { - collectionId: string; - libraryId: string; -} - -const CollectionInfo = ({ libraryId, collectionId } : CollectionInfoProps) => { +const CollectionInfo = () => { const intl = useIntl(); - const { data: collectionData, isLoading: isCollectionLoading } = useCollection(libraryId, collectionId); - - if (isCollectionLoading) { - return ; - } return ( - -
- {collectionData?.title} -
- - - Manage tab placeholder - - - Details tab placeholder - - - + + + Manage tab placeholder + + + Details tab placeholder + + ); }; diff --git a/src/library-authoring/collections/CollectionInfoHeader.tsx b/src/library-authoring/collections/CollectionInfoHeader.tsx new file mode 100644 index 0000000000..2725679a30 --- /dev/null +++ b/src/library-authoring/collections/CollectionInfoHeader.tsx @@ -0,0 +1,15 @@ +import { Collection } from '../data/api'; + +interface CollectionInfoHeaderProps { + collection?: Collection; +} + +const CollectionInfoHeader = ({ collection } : CollectionInfoHeaderProps) => { + return ( +
+ {collection?.title} +
+ ); +}; + +export default CollectionInfoHeader; diff --git a/src/library-authoring/collections/LibraryCollectionComponents.tsx b/src/library-authoring/collections/LibraryCollectionComponents.tsx new file mode 100644 index 0000000000..fa77f64e01 --- /dev/null +++ b/src/library-authoring/collections/LibraryCollectionComponents.tsx @@ -0,0 +1,24 @@ +import { Stack } from '@openedx/paragon'; +import { NoComponents, NoSearchResults } from '../EmptyStates'; +import { useSearchContext } from '../../search-manager'; +import { LibraryComponents } from '../components'; +import messages from './messages'; + +const LibraryCollectionComponents = ({ libraryId }: { libraryId: string }) => { + const { totalHits: componentCount, isFiltered } = useSearchContext(); + + if (componentCount === 0) { + return isFiltered ? + + : ; + } + + return ( + +

Content ({componentCount})

+ + + ); +}; + +export default LibraryCollectionComponents; diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 8eea41eac9..9f9d2fbe58 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -1,4 +1,4 @@ -import React, { useContext, useEffect } from 'react'; +import { useContext, useEffect } from 'react'; import { StudioFooter } from '@edx/frontend-component-footer'; import { useIntl } from '@edx/frontend-platform/i18n'; import { @@ -8,6 +8,7 @@ import { Container, Icon, IconButton, + Row, Stack, } from '@openedx/paragon'; import { Add, InfoOutline } from '@openedx/paragon/icons'; @@ -29,12 +30,9 @@ import { useCollection, useContentLibrary } from '../data/apiHooks'; import { LibraryContext } from '../common/context'; import messages from '../messages'; import { LibrarySidebar } from '../library-sidebar'; +import LibraryCollectionComponents from './LibraryCollectionComponents'; -interface HeaderActionsProps { - canEditLibrary: boolean; -} - -const HeaderActions = ({ canEditLibrary }: HeaderActionsProps) => { +const HeaderActions = ({ canEditLibrary }: { canEditLibrary: boolean; }) => { const intl = useIntl(); const { openAddContentSidebar, @@ -59,7 +57,15 @@ const HeaderActions = ({ canEditLibrary }: HeaderActionsProps) => { ); }; -const SubHeaderTitle = ({ title, canEditLibrary, infoClickHandler }: { title: string, canEditLibrary: boolean, infoClickHandler: () => void }) => { +const SubHeaderTitle = ({ + title, + canEditLibrary, + infoClickHandler, +}: { + title: string; + canEditLibrary: boolean; + infoClickHandler: () => void; +}) => { const intl = useIntl(); return ( @@ -85,7 +91,13 @@ const SubHeaderTitle = ({ title, canEditLibrary, infoClickHandler }: { title: st ); }; -const LibraryCollectionPage = ({ libraryId, collectionId }: { libraryId: string, collectionId: string }) => { +const LibraryCollectionPage = ({ + libraryId, + collectionId, +}: { + libraryId: string; + collectionId: string; +}) => { const intl = useIntl(); const { @@ -94,7 +106,7 @@ const LibraryCollectionPage = ({ libraryId, collectionId }: { libraryId: string, } = useContext(LibraryContext); useEffect(() => { - openCollectionInfoSidebar(collectionId); + openCollectionInfoSidebar(); }, []); const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId); @@ -139,7 +151,7 @@ const LibraryCollectionPage = ({ libraryId, collectionId }: { libraryId: string, title={ openCollectionInfoSidebar(collectionId)} + infoClickHandler={openCollectionInfoSidebar} />} breadcrumbs={( } /> -
+
+
{ !!sidebarBodyComponent && (
- +
)}
@@ -178,7 +191,8 @@ const LibraryCollectionPageWrapper = () => { return ( diff --git a/src/library-authoring/collections/LibraryCollections.tsx b/src/library-authoring/collections/LibraryCollections.tsx index 05f6950d65..4ddd6846f3 100644 --- a/src/library-authoring/collections/LibraryCollections.tsx +++ b/src/library-authoring/collections/LibraryCollections.tsx @@ -3,6 +3,7 @@ import { useSearchContext } from '../../search-manager'; import { NoComponents, NoSearchResults } from '../EmptyStates'; import CollectionCard from '../components/CollectionCard'; import { LIBRARY_SECTION_PREVIEW_LIMIT } from '../components/LibrarySection'; +import messages from '../messages'; type LibraryCollectionsProps = { variant: 'full' | 'preview', @@ -35,7 +36,9 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => { ); if (totalCollectionHits === 0) { - return isFiltered ? : ; + return isFiltered ? + + : ; } return ( diff --git a/src/library-authoring/collections/index.tsx b/src/library-authoring/collections/index.tsx index d41c2f9381..dc3ec3a47d 100644 --- a/src/library-authoring/collections/index.tsx +++ b/src/library-authoring/collections/index.tsx @@ -1 +1,2 @@ export { default as CollectionInfo } from './CollectionInfo'; +export { default as CollectionInfoHeader } from './CollectionInfoHeader'; diff --git a/src/library-authoring/collections/messages.ts b/src/library-authoring/collections/messages.ts index b60ca26801..74eb334dfd 100644 --- a/src/library-authoring/collections/messages.ts +++ b/src/library-authoring/collections/messages.ts @@ -11,6 +11,21 @@ const messages = defineMessages({ defaultMessage: 'Details', description: 'Title for details tab', }, + noComponentsInCollection: { + id: 'course-authoring.library-authoring.collections-pag.no-components.text', + defaultMessage: 'This collection is currently empty.', + description: 'Text to display when collection has no associated components', + }, + addComponentsInCollection: { + id: 'course-authoring.library-authoring.collections-pag.add-components.btn-text', + defaultMessage: 'New', + description: 'Text to display in new button if no components in collection is found', + }, + noSearchResultsInCollection: { + id: 'course-authoring.library-authoring.collections-pag.no-search-results.text', + defaultMessage: 'No matching components found in this collections.', + description: 'Message displayed when no matching components are found in collection', + }, }); export default messages; diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index 71b7c05383..5b16a74f3e 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -18,7 +18,7 @@ export interface LibraryContextData { isCreateCollectionModalOpen: boolean; openCreateCollectionModal: () => void; closeCreateCollectionModal: () => void; - openCollectionInfoSidebar: (collectionId: string) => void; + openCollectionInfoSidebar: () => void; } export const LibraryContext = React.createContext({ @@ -30,7 +30,7 @@ export const LibraryContext = React.createContext({ isCreateCollectionModalOpen: false, openCreateCollectionModal: () => {}, closeCreateCollectionModal: () => {}, - openCollectionInfoSidebar: (_collectionId: string) => {}, // eslint-disable-line @typescript-eslint/no-unused-vars + openCollectionInfoSidebar: () => {}, // eslint-disable-line @typescript-eslint/no-unused-vars } as LibraryContextData); /** @@ -61,13 +61,10 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { }, [], ); - const openCollectionInfoSidebar = React.useCallback( - (collectionId: string) => { - setCurrentComponentKey(collectionId); - setSidebarBodyComponent(SidebarBodyComponentId.CollectionInfo); - }, - [], - ); + const openCollectionInfoSidebar = React.useCallback(() => { + setCurrentComponentKey(undefined); + setSidebarBodyComponent(SidebarBodyComponentId.CollectionInfo); + }, []); const context = React.useMemo(() => ({ sidebarBodyComponent, diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index 9808e595b3..77aec34580 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -11,11 +11,12 @@ 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 } from '../data/api'; -import { CollectionInfo } from '../collections'; +import { ContentLibrary, Collection } from '../data/api'; +import { CollectionInfo, CollectionInfoHeader } from '../collections'; type LibrarySidebarProps = { library: ContentLibrary, + collection?: Collection }; /** @@ -27,7 +28,7 @@ type LibrarySidebarProps = { * You can add more components in `bodyComponentMap`. * Use the returned actions to open and close this sidebar. */ -const LibrarySidebar = ({ library }: LibrarySidebarProps) => { +const LibrarySidebar = ({ library, collection }: LibrarySidebarProps) => { const intl = useIntl(); const { sidebarBodyComponent, @@ -41,9 +42,7 @@ const LibrarySidebar = ({ library }: LibrarySidebarProps) => { [SidebarBodyComponentId.ComponentInfo]: ( currentComponentKey && ), - [SidebarBodyComponentId.CollectionInfo]: ( - currentComponentKey && - ), + [SidebarBodyComponentId.CollectionInfo]: , unknown: null, }; @@ -53,9 +52,7 @@ const LibrarySidebar = ({ library }: LibrarySidebarProps) => { [SidebarBodyComponentId.ComponentInfo]: ( currentComponentKey && ), - //[SidebarBodyComponentId.CollectionInfo]: ( - // currentComponentKey && - //), + [SidebarBodyComponentId.CollectionInfo]: , unknown: null, }; From bfd3e7d51487216425ca28d78f3a32ef8b2205a8 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Sat, 14 Sep 2024 15:59:58 +0530 Subject: [PATCH 05/22] fix: rebase issues --- src/library-authoring/EmptyStates.tsx | 8 +++++--- src/library-authoring/LibraryLayout.tsx | 1 - .../collections/LibraryCollectionComponents.tsx | 5 ++++- .../collections/LibraryCollectionPage.tsx | 4 ++-- src/library-authoring/collections/LibraryCollections.tsx | 6 +++++- src/library-authoring/library-sidebar/LibrarySidebar.tsx | 6 +++--- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/library-authoring/EmptyStates.tsx b/src/library-authoring/EmptyStates.tsx index a1ff3bb766..25b1770b86 100644 --- a/src/library-authoring/EmptyStates.tsx +++ b/src/library-authoring/EmptyStates.tsx @@ -14,9 +14,11 @@ import { useContentLibrary } from './data/apiHooks'; export const NoComponents = ({ infoText = messages.noComponents, addBtnText = messages.addComponent, + searchType = "component", }: { - infoText: MessageDescriptor; - addBtnText: MessageDescriptor; + infoText?: MessageDescriptor; + addBtnText?: MessageDescriptor; + searchType?: "collection" | "component"; }) => { const { openAddContentSidebar, openCreateCollectionModal } = useContext(LibraryContext); const { libraryId } = useParams(); @@ -46,7 +48,7 @@ export const NoComponents = ({ export const NoSearchResults = ({ infoText = messages.noSearchResults, }: { - infoText: MessageDescriptor; + infoText?: MessageDescriptor; }) => ( diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index a69baa9859..7139c76e5a 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -13,7 +13,6 @@ import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; import { invalidateComponentData } from './data/apiHooks'; -import LibraryCollectionPageWrapper from './LibraryCollectionPage'; import LibraryCollectionPageWrapper from './collections/LibraryCollectionPage'; const LibraryLayout = () => { diff --git a/src/library-authoring/collections/LibraryCollectionComponents.tsx b/src/library-authoring/collections/LibraryCollectionComponents.tsx index fa77f64e01..a44b39d2b6 100644 --- a/src/library-authoring/collections/LibraryCollectionComponents.tsx +++ b/src/library-authoring/collections/LibraryCollectionComponents.tsx @@ -10,7 +10,10 @@ const LibraryCollectionComponents = ({ libraryId }: { libraryId: string }) => { if (componentCount === 0) { return isFiltered ? - : ; + : ; } return ( diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 9f9d2fbe58..8bd23af37f 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -137,8 +137,8 @@ const LibraryCollectionPage = ({ ]; return ( -
-
+
+
{ if (totalCollectionHits === 0) { return isFiltered ? - : ; + : ; } return ( diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index 77aec34580..bf77d52a4a 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -33,14 +33,14 @@ const LibrarySidebar = ({ library, collection }: LibrarySidebarProps) => { const { sidebarBodyComponent, closeLibrarySidebar, - currentComponentKey, + currentComponentUsageKey, } = useContext(LibraryContext); const bodyComponentMap = { [SidebarBodyComponentId.AddContent]: , [SidebarBodyComponentId.Info]: , [SidebarBodyComponentId.ComponentInfo]: ( - currentComponentKey && + currentComponentUsageKey && ), [SidebarBodyComponentId.CollectionInfo]: , unknown: null, @@ -50,7 +50,7 @@ const LibrarySidebar = ({ library, collection }: LibrarySidebarProps) => { [SidebarBodyComponentId.AddContent]: , [SidebarBodyComponentId.Info]: , [SidebarBodyComponentId.ComponentInfo]: ( - currentComponentKey && + currentComponentUsageKey && ), [SidebarBodyComponentId.CollectionInfo]: , unknown: null, From 5b2d734dcc0d1644982ceeeaf96ba90f176a378c Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 10:49:35 +0530 Subject: [PATCH 06/22] fix: darken card action btn hover state --- src/generic/block-type-utils/index.scss | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/generic/block-type-utils/index.scss b/src/generic/block-type-utils/index.scss index a546d8ca6b..c68dfbee49 100644 --- a/src/generic/block-type-utils/index.scss +++ b/src/generic/block-type-utils/index.scss @@ -4,6 +4,12 @@ .pgn__icon { color: white; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#005C9E, 15%); + } + } } .component-style-html { @@ -12,6 +18,12 @@ .pgn__icon { color: white; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#9747FF, 15%); + } + } } .component-style-collection { @@ -20,6 +32,12 @@ .pgn__icon { color: black; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#FFCD29, 15%); + } + } } .component-style-video { @@ -28,6 +46,12 @@ .pgn__icon { color: white; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#358F0A, 15%); + } + } } .component-style-vertical { @@ -36,6 +60,12 @@ .pgn__icon { color: white; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#0B8E77, 15%); + } + } } .component-style-other { @@ -44,4 +74,10 @@ .pgn__icon { color: white; } + + .btn-icon { + &:hover, &:active, &:focus { + background-color: darken(#646464, 15%); + } + } } From 6841d9db0095c76cd0bddb856ad6acbfcacc71d9 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 11:23:15 +0530 Subject: [PATCH 07/22] refactor: pass btn click handler to empty state component --- src/library-authoring/EmptyStates.tsx | 17 +++-------------- src/library-authoring/LibraryHome.tsx | 6 ++++-- .../collections/LibraryCollectionComponents.tsx | 4 ++++ .../collections/LibraryCollections.tsx | 9 +++++++-- src/library-authoring/common/context.tsx | 3 +-- .../components/LibraryComponents.tsx | 6 ++++-- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/library-authoring/EmptyStates.tsx b/src/library-authoring/EmptyStates.tsx index 25b1770b86..eea5ed732a 100644 --- a/src/library-authoring/EmptyStates.tsx +++ b/src/library-authoring/EmptyStates.tsx @@ -1,4 +1,3 @@ -import React, { useContext, useCallback } from 'react'; import { useParams } from 'react-router'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; import type { MessageDescriptor } from 'react-intl'; @@ -8,36 +7,26 @@ import { import { Add } from '@openedx/paragon/icons'; import { ClearFiltersButton } from '../search-manager'; import messages from './messages'; -import { LibraryContext } from './common/context'; import { useContentLibrary } from './data/apiHooks'; export const NoComponents = ({ infoText = messages.noComponents, addBtnText = messages.addComponent, - searchType = "component", + handleBtnClick, }: { infoText?: MessageDescriptor; addBtnText?: MessageDescriptor; - searchType?: "collection" | "component"; + handleBtnClick: () => void; }) => { - const { openAddContentSidebar, openCreateCollectionModal } = useContext(LibraryContext); const { libraryId } = useParams(); const { data: libraryData } = useContentLibrary(libraryId); const canEditLibrary = libraryData?.canEditLibrary ?? false; - const handleOnClickButton = useCallback(() => { - if (searchType === 'collection') { - openCreateCollectionModal(); - } else { - openAddContentSidebar(); - } - }, [searchType]); - return ( {canEditLibrary && ( - )} diff --git a/src/library-authoring/LibraryHome.tsx b/src/library-authoring/LibraryHome.tsx index 4b170b96fa..2ee1e5cb66 100644 --- a/src/library-authoring/LibraryHome.tsx +++ b/src/library-authoring/LibraryHome.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useContext } from 'react'; import { Stack } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -9,6 +9,7 @@ import { LibraryComponents } from './components'; import LibrarySection from './components/LibrarySection'; import LibraryRecentlyModified from './LibraryRecentlyModified'; import messages from './messages'; +import { LibraryContext } from './common/context'; type LibraryHomeProps = { libraryId: string, @@ -23,10 +24,11 @@ const LibraryHome = ({ libraryId, tabList, handleTabChange } : LibraryHomeProps) totalCollectionHits: collectionCount, isFiltered, } = useSearchContext(); + const { openAddContentSidebar } = useContext(LibraryContext); const renderEmptyState = () => { if (componentCount === 0 && collectionCount === 0) { - return isFiltered ? : ; + return isFiltered ? : ; } return null; }; diff --git a/src/library-authoring/collections/LibraryCollectionComponents.tsx b/src/library-authoring/collections/LibraryCollectionComponents.tsx index a44b39d2b6..1bafef00b7 100644 --- a/src/library-authoring/collections/LibraryCollectionComponents.tsx +++ b/src/library-authoring/collections/LibraryCollectionComponents.tsx @@ -1,11 +1,14 @@ +import { useContext } from 'react'; import { Stack } from '@openedx/paragon'; import { NoComponents, NoSearchResults } from '../EmptyStates'; import { useSearchContext } from '../../search-manager'; import { LibraryComponents } from '../components'; import messages from './messages'; +import { LibraryContext } from '../common/context'; const LibraryCollectionComponents = ({ libraryId }: { libraryId: string }) => { const { totalHits: componentCount, isFiltered } = useSearchContext(); + const { openAddContentSidebar } = useContext(LibraryContext); if (componentCount === 0) { return isFiltered ? @@ -13,6 +16,7 @@ const LibraryCollectionComponents = ({ libraryId }: { libraryId: string }) => { : ; } diff --git a/src/library-authoring/collections/LibraryCollections.tsx b/src/library-authoring/collections/LibraryCollections.tsx index d4d9ca0ce5..7f18f89bd6 100644 --- a/src/library-authoring/collections/LibraryCollections.tsx +++ b/src/library-authoring/collections/LibraryCollections.tsx @@ -1,9 +1,12 @@ +import { useContext } from 'react'; + import { useLoadOnScroll } from '../../hooks'; import { useSearchContext } from '../../search-manager'; import { NoComponents, NoSearchResults } from '../EmptyStates'; import CollectionCard from '../components/CollectionCard'; import { LIBRARY_SECTION_PREVIEW_LIMIT } from '../components/LibrarySection'; import messages from '../messages'; +import { LibraryContext } from '../common/context'; type LibraryCollectionsProps = { variant: 'full' | 'preview', @@ -26,6 +29,8 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => { isFiltered, } = useSearchContext(); + const { openCreateCollectionModal } = useContext(LibraryContext); + const collectionList = variant === 'preview' ? collectionHits.slice(0, LIBRARY_SECTION_PREVIEW_LIMIT) : collectionHits; useLoadOnScroll( @@ -35,13 +40,13 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => { variant === 'full', ); - if (totalCollectionHits === 0) { + if (totalCollectionHits === 1) { return isFiltered ? : ; } diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index 5b16a74f3e..b3811e6ddf 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -40,7 +40,6 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { const [sidebarBodyComponent, setSidebarBodyComponent] = React.useState(null); const [currentComponentUsageKey, setCurrentComponentUsageKey] = React.useState(); const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); - const [currentComponentKey, setCurrentComponentKey] = React.useState(); const closeLibrarySidebar = React.useCallback(() => { setSidebarBodyComponent(null); @@ -62,7 +61,7 @@ export const LibraryProvider = (props: { children?: React.ReactNode }) => { [], ); const openCollectionInfoSidebar = React.useCallback(() => { - setCurrentComponentKey(undefined); + setCurrentComponentUsageKey(undefined); setSidebarBodyComponent(SidebarBodyComponentId.CollectionInfo); }, []); diff --git a/src/library-authoring/components/LibraryComponents.tsx b/src/library-authoring/components/LibraryComponents.tsx index af02dff1f9..c91dbad55a 100644 --- a/src/library-authoring/components/LibraryComponents.tsx +++ b/src/library-authoring/components/LibraryComponents.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react'; +import React, { useContext, useMemo } from 'react'; import { useLoadOnScroll } from '../../hooks'; import { useSearchContext } from '../../search-manager'; @@ -6,6 +6,7 @@ import { NoComponents, NoSearchResults } from '../EmptyStates'; import { useLibraryBlockTypes } from '../data/apiHooks'; import ComponentCard from './ComponentCard'; import { LIBRARY_SECTION_PREVIEW_LIMIT } from './LibrarySection'; +import { LibraryContext } from '../common/context'; type LibraryComponentsProps = { libraryId: string, @@ -28,6 +29,7 @@ const LibraryComponents = ({ libraryId, variant }: LibraryComponentsProps) => { fetchNextPage, isFiltered, } = useSearchContext(); + const { openAddContentSidebar } = useContext(LibraryContext); const componentList = variant === 'preview' ? hits.slice(0, LIBRARY_SECTION_PREVIEW_LIMIT) : hits; @@ -51,7 +53,7 @@ const LibraryComponents = ({ libraryId, variant }: LibraryComponentsProps) => { ); if (componentCount === 0) { - return isFiltered ? : ; + return isFiltered ? : ; } return ( From 626c06ba821e2417447a3f2b42bb9ee06d6655c9 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 11:48:15 +0530 Subject: [PATCH 08/22] refactor: move messages related to collections in single file --- src/library-authoring/LibraryLayout.tsx | 4 +- .../collections/LibraryCollectionPage.tsx | 88 ++++++++----------- .../collections/LibraryCollections.tsx | 2 +- src/library-authoring/collections/messages.ts | 45 ++++++++++ src/library-authoring/messages.ts | 25 ------ src/search-manager/SearchKeywordsField.tsx | 6 +- 6 files changed, 90 insertions(+), 80 deletions(-) diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 7139c76e5a..6d7bce678d 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -13,7 +13,7 @@ import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; import { invalidateComponentData } from './data/apiHooks'; -import LibraryCollectionPageWrapper from './collections/LibraryCollectionPage'; +import LibraryCollectionPage from './collections/LibraryCollectionPage'; const LibraryLayout = () => { const { libraryId } = useParams(); @@ -61,7 +61,7 @@ const LibraryLayout = () => { /> } + element={} /> { +const LibraryCollectionPage = () => { + const { libraryId, collectionId } = useParams(); + + if (!collectionId || !libraryId) { + throw new Error('Rendered without collectionId or libraryId URL parameter'); + } + const intl = useIntl(); const { @@ -147,30 +146,35 @@ const LibraryCollectionPage = ({ isLibrary /> - } - breadcrumbs={( - - )} - headerActions={} - /> - -
- - - -
- -
- + + } + breadcrumbs={( + + )} + headerActions={} + /> + +
+ + + +
+ +
+ +
@@ -183,20 +187,4 @@ const LibraryCollectionPage = ({ ); }; -const LibraryCollectionPageWrapper = () => { - const { libraryId, collectionId } = useParams(); - if (!collectionId || !libraryId) { - throw new Error('Rendered without collectionId or libraryId URL parameter'); - } - - return ( - - - - ); -} - -export default LibraryCollectionPageWrapper; +export default LibraryCollectionPage; diff --git a/src/library-authoring/collections/LibraryCollections.tsx b/src/library-authoring/collections/LibraryCollections.tsx index 7f18f89bd6..8fb3c9292d 100644 --- a/src/library-authoring/collections/LibraryCollections.tsx +++ b/src/library-authoring/collections/LibraryCollections.tsx @@ -5,7 +5,7 @@ import { useSearchContext } from '../../search-manager'; import { NoComponents, NoSearchResults } from '../EmptyStates'; import CollectionCard from '../components/CollectionCard'; import { LIBRARY_SECTION_PREVIEW_LIMIT } from '../components/LibrarySection'; -import messages from '../messages'; +import messages from './messages'; import { LibraryContext } from '../common/context'; type LibraryCollectionsProps = { diff --git a/src/library-authoring/collections/messages.ts b/src/library-authoring/collections/messages.ts index 74eb334dfd..0f260f7033 100644 --- a/src/library-authoring/collections/messages.ts +++ b/src/library-authoring/collections/messages.ts @@ -26,6 +26,51 @@ const messages = defineMessages({ defaultMessage: 'No matching components found in this collections.', description: 'Message displayed when no matching components are found in collection', }, + newContentButton: { + id: 'course-authoring.library-authoring.collections.buttons.new-content.text', + defaultMessage: 'New', + description: 'Text of button to open "Add content drawer" in collections page', + }, + collectionInfoButton: { + id: 'course-authoring.library-authoring.buttons.collection-info.alt-text', + defaultMessage: 'Collection Info', + description: 'Alt text for collection info button besides the collection title', + }, + readOnlyBadge: { + id: 'course-authoring.library-authoring.collections.badge.read-only', + defaultMessage: 'Read Only', + description: 'Text in badge when the user has read only access in collections page', + }, + allCollections: { + id: 'course-authoring.library-authoring.all-collections.text', + defaultMessage: 'All Collections', + description: 'Breadcrumbs text to navigate back to all collections', + }, + breadcrumbsAriaLabel: { + id: 'course-authoring.library-authoring.breadcrumbs.label.text', + defaultMessage: 'Navigation breadcrumbs', + description: 'Aria label for navigation breadcrumbs', + }, + searchPlaceholder: { + id: 'course-authoring.library-authoring.search.placeholder.text', + defaultMessage: 'Search Collection', + description: 'Search placeholder text in collections page.', + }, + noSearchResultsCollections: { + id: 'course-authoring.library-authoring.no-search-results-collections', + defaultMessage: 'No matching collections found in this library.', + description: 'Message displayed when no matching collections are found', + }, + noCollections: { + id: 'course-authoring.library-authoring.no-collections', + defaultMessage: 'You have not added any collection to this library yet.', + description: 'Message displayed when the library has no collections', + }, + addCollection: { + id: 'course-authoring.library-authoring.add-collection', + defaultMessage: 'Add collection', + description: 'Button text to add a new collection', + }, }); export default messages; diff --git a/src/library-authoring/messages.ts b/src/library-authoring/messages.ts index 8da572a824..0f9bdfbd5b 100644 --- a/src/library-authoring/messages.ts +++ b/src/library-authoring/messages.ts @@ -6,11 +6,6 @@ const messages = defineMessages({ defaultMessage: 'Content library', description: 'The page heading for the library page.', }, - allCollections: { - id: 'course-authoring.library-authoring.all-collections', - defaultMessage: 'All Collections', - description: 'Breadcrumbs text to navigate back to all collections', - }, headingInfoAlt: { id: 'course-authoring.library-authoring.heading-info-alt', defaultMessage: 'Info', @@ -26,31 +21,16 @@ const messages = defineMessages({ defaultMessage: 'No matching components found in this library.', description: 'Message displayed when no search results are found', }, - noSearchResultsCollections: { - id: 'course-authoring.library-authoring.no-search-results-collections', - defaultMessage: 'No matching collections found in this library.', - description: 'Message displayed when no matching collections are found', - }, noComponents: { id: 'course-authoring.library-authoring.no-components', defaultMessage: 'You have not added any content to this library yet.', description: 'Message displayed when the library is empty', }, - noCollections: { - id: 'course-authoring.library-authoring.no-collections', - defaultMessage: 'You have not added any collection to this library yet.', - description: 'Message displayed when the library has no collections', - }, addComponent: { id: 'course-authoring.library-authoring.add-component', defaultMessage: 'Add component', description: 'Button text to add a new component', }, - addCollection: { - id: 'course-authoring.library-authoring.add-collection', - defaultMessage: 'Add collection', - description: 'Button text to add a new collection', - }, homeTab: { id: 'course-authoring.library-authoring.home-tab', defaultMessage: 'Home', @@ -121,11 +101,6 @@ const messages = defineMessages({ defaultMessage: 'Library Info', description: 'Text of button to open "Library Info sidebar"', }, - collectionInfoButton: { - id: 'course-authoring.library-authoring.buttons.collection-info.alt-text', - defaultMessage: 'Collection Info', - description: 'Alt text for collection info button besides the collection title', - }, readOnlyBadge: { id: 'course-authoring.library-authoring.badge.read-only', defaultMessage: 'Read Only', diff --git a/src/search-manager/SearchKeywordsField.tsx b/src/search-manager/SearchKeywordsField.tsx index 953cd7799d..54d9ca83ae 100644 --- a/src/search-manager/SearchKeywordsField.tsx +++ b/src/search-manager/SearchKeywordsField.tsx @@ -7,7 +7,7 @@ import { useSearchContext } from './SearchManager'; /** * The "main" input field where users type in search keywords. The search happens as they type (no need to press enter). */ -const SearchKeywordsField: React.FC<{ className?: string }> = (props) => { +const SearchKeywordsField: React.FC<{ className?: string, placeholder?: string }> = (props) => { const intl = useIntl(); const { searchKeywords, setSearchKeywords } = useSearchContext(); @@ -22,7 +22,9 @@ const SearchKeywordsField: React.FC<{ className?: string }> = (props) => { From 1b3754c0faa8ff5b5556656e6cf48d5887599ed7 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 12:04:05 +0530 Subject: [PATCH 09/22] fix: lint issues --- src/library-authoring/LibraryAuthoringPage.scss | 2 +- src/library-authoring/LibraryAuthoringPage.tsx | 2 +- .../LibraryRecentlyModified.tsx | 2 +- .../collections/CollectionInfoHeader.tsx | 12 +++++------- .../collections/LibraryCollectionComponents.tsx | 16 +++++++++------- .../collections/LibraryCollectionPage.tsx | 14 ++++++++------ .../collections/LibraryCollections.tsx | 2 +- src/library-authoring/data/api.ts | 1 - src/library-authoring/data/apiHooks.ts | 1 - src/search-manager/SearchKeywordsField.tsx | 2 +- src/search-manager/data/api.ts | 9 ++++++--- src/search-manager/data/apiHooks.ts | 2 +- 12 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.scss b/src/library-authoring/LibraryAuthoringPage.scss index c909627441..6e422a802f 100644 --- a/src/library-authoring/LibraryAuthoringPage.scss +++ b/src/library-authoring/LibraryAuthoringPage.scss @@ -23,5 +23,5 @@ // Reduce breadcrumb bottom margin ol.list-inline { - margin-bottom: 0rem; + margin-bottom: 0; } diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 019527b679..cec5262a49 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -168,7 +168,7 @@ const LibraryAuthoringPage = () => { } diff --git a/src/library-authoring/LibraryRecentlyModified.tsx b/src/library-authoring/LibraryRecentlyModified.tsx index 94e4196fc5..4371d6ced4 100644 --- a/src/library-authoring/LibraryRecentlyModified.tsx +++ b/src/library-authoring/LibraryRecentlyModified.tsx @@ -81,7 +81,7 @@ const LibraryRecentlyModified: React.FC<{ libraryId: string }> = ({ libraryId }) diff --git a/src/library-authoring/collections/CollectionInfoHeader.tsx b/src/library-authoring/collections/CollectionInfoHeader.tsx index 2725679a30..dcd6b66abd 100644 --- a/src/library-authoring/collections/CollectionInfoHeader.tsx +++ b/src/library-authoring/collections/CollectionInfoHeader.tsx @@ -4,12 +4,10 @@ interface CollectionInfoHeaderProps { collection?: Collection; } -const CollectionInfoHeader = ({ collection } : CollectionInfoHeaderProps) => { - return ( -
- {collection?.title} -
- ); -}; +const CollectionInfoHeader = ({ collection } : CollectionInfoHeaderProps) => ( +
+ {collection?.title} +
+); export default CollectionInfoHeader; diff --git a/src/library-authoring/collections/LibraryCollectionComponents.tsx b/src/library-authoring/collections/LibraryCollectionComponents.tsx index 1bafef00b7..5d870645c9 100644 --- a/src/library-authoring/collections/LibraryCollectionComponents.tsx +++ b/src/library-authoring/collections/LibraryCollectionComponents.tsx @@ -11,13 +11,15 @@ const LibraryCollectionComponents = ({ libraryId }: { libraryId: string }) => { const { openAddContentSidebar } = useContext(LibraryContext); if (componentCount === 0) { - return isFiltered ? - - : ; + return isFiltered + ? + : ( + + ); } return ( diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 48bb92e4c4..995906a9df 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -131,7 +131,7 @@ const LibraryCollectionPage = () => { // Adding empty breadcrumb to add the last `>` spacer. { label: '', - to: ``, + to: '', }, ]; @@ -151,11 +151,13 @@ const LibraryCollectionPage = () => { fetchCollections={false} > } + title={( + +)} breadcrumbs={( { variant === 'full', ); - if (totalCollectionHits === 1) { + if (totalCollectionHits === 0) { return isFiltered ? : diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index 064b3dd2c3..3a99672984 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -269,14 +269,17 @@ export async function fetchSearchResults({ } const { results } = await client.multiSearch(({ queries })); + const componentHitLength = results[0].hits.length; + const collectionHitLength = fetchCollections ? results[2].hits.length : 0; return { hits: results[0].hits.map(formatSearchHit) as ContentHit[], - totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? results[0].hits.length, + totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? componentHitLength, blockTypes: results[1].facetDistribution?.block_type ?? {}, problemTypes: results[1].facetDistribution?.['content.problem_types'] ?? {}, - nextOffset: results[0].hits.length === limit || (fetchCollections && results[2].hits.length === limit) ? offset + limit : undefined, + 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 ?? results[2].hits.length: 0, + totalCollectionHits: fetchCollections + ? results[2].totalHits ?? results[2].estimatedTotalHits ?? collectionHitLength : 0, }; } diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index d9ba0390c1..1f170531f3 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -262,4 +262,4 @@ export const useGetSingleDocument = ({ client, indexName, id }: { return fetchDocumentById({ client, indexName, id }); }, }) -) +); From fd1aa85e2d2916c47a37f18444aef1e92ef2f755 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 20:27:08 +0530 Subject: [PATCH 10/22] test: add test for collections page --- .../__mocks__/collection-search.json | 189 ++++++++++ .../LibraryCollectionPage.test.tsx | 341 ++++++++++++++++++ src/library-authoring/data/api.mocks.ts | 42 +++ src/search-manager/SearchManager.ts | 6 +- src/search-manager/data/apiHooks.ts | 1 + 5 files changed, 577 insertions(+), 2 deletions(-) create mode 100644 src/library-authoring/__mocks__/collection-search.json create mode 100644 src/library-authoring/collections/LibraryCollectionPage.test.tsx diff --git a/src/library-authoring/__mocks__/collection-search.json b/src/library-authoring/__mocks__/collection-search.json new file mode 100644 index 0000000000..3f48fa46ea --- /dev/null +++ b/src/library-authoring/__mocks__/collection-search.json @@ -0,0 +1,189 @@ +{ + "comment": "This mock is captured from a real search result and roughly edited to match the mocks in src/library-authoring/data/api.mocks.ts", + "note": "The _formatted fields have been removed from this result and should be re-added programatically when mocking.", + "results": [ + { + "indexUid": "studio_content", + "hits": [ + { + "id": "lbaximtesthtml571fe018-f3ce-45c9-8f53-5dafcb422fdd-273ebd90", + "display_name": "Introduction to Testing", + "block_id": "571fe018-f3ce-45c9-8f53-5dafcb422fdd", + "content": { + "html_content": "This is a text component which uses HTML." + }, + "tags": {}, + "type": "library_block", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1721857069.042984, + "modified": 1725878053.420395, + "last_published": 1725035862.450613, + "usage_key": "lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd", + "block_type": "html", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "collections": { + "display_name": [ + "My first collection" + ], + "key": [ + "my-first-collection" + ] + } + }, + { + "id": "lbaximtesthtml73a22298-bcd9-4f4c-ae34-0bc2b0612480-46b4a7f2", + "display_name": "Second Text Component", + "block_id": "73a22298-bcd9-4f4c-ae34-0bc2b0612480", + "content": { + "html_content": "Preview of the second text component here" + }, + "type": "library_block", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1724879593.066427, + "modified": 1725034981.663482, + "last_published": 1725035862.450613, + "usage_key": "lb:Axim:TEST:html:73a22298-bcd9-4f4c-ae34-0bc2b0612480", + "block_type": "html", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "collections": { + "display_name": [ + "My first collection" + ], + "key": [ + "my-first-collection" + ] + } + }, + { + "id": "lbaximtesthtmlbe5b5db9-26ba-4fac-86af-654538c70b5e-73dbaa95", + "display_name": "Third Text component", + "block_id": "be5b5db9-26ba-4fac-86af-654538c70b5e", + "content": { + "html_content": "This is a text component that I've edited within the library. " + }, + "tags": {}, + "type": "library_block", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1721857034.455737, + "modified": 1722551300.377488, + "last_published": 1724879092.002222, + "usage_key": "lb:Axim:TEST:html:be5b5db9-26ba-4fac-86af-654538c70b5e", + "block_type": "html", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "collections": { + "display_name": [ + "My first collection", + "My second collection" + ], + "key": [ + "my-first-collection", + "my-second-collection" + ] + } + }, + { + "id": "lbaximtesthtmle59e8c73-4056-4894-bca4-062781fb3f68-46a404b2", + "display_name": "Text 4", + "block_id": "e59e8c73-4056-4894-bca4-062781fb3f68", + "content": { + "html_content": "" + }, + "tags": {}, + "type": "library_block", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1720774228.49832, + "modified": 1720774228.49832, + "last_published": 1724879092.002222, + "usage_key": "lb:Axim:TEST:html:e59e8c73-4056-4894-bca4-062781fb3f68", + "block_type": "html", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "collections": { + "display_name": [ + "My first collection" + ], + "key": [ + "my-first-collection" + ] + } + }, + { + "id": "lbaximtestproblemf16116c9-516e-4bb9-b99e-103599f62417-f2798115", + "display_name": "Blank Problem", + "block_id": "f16116c9-516e-4bb9-b99e-103599f62417", + "content": { + "problem_types": [], + "capa_content": " " + }, + "type": "library_block", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1724725821.973896, + "modified": 1724725821.973896, + "last_published": 1724879092.002222, + "usage_key": "lb:Axim:TEST:problem:f16116c9-516e-4bb9-b99e-103599f62417", + "block_type": "problem", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "collections": { + "display_name": [ + "My first collection" + ], + "key": [ + "my-first-collection" + ] + } + } + ], + "query": "", + "processingTimeMs": 1, + "limit": 20, + "offset": 0, + "estimatedTotalHits": 5 + }, + { + "indexUid": "studio_content", + "hits": [], + "query": "", + "processingTimeMs": 0, + "limit": 0, + "offset": 0, + "estimatedTotalHits": 5, + "facetDistribution": { + "block_type": { + "html": 4, + "problem": 1 + }, + "content.problem_types": {} + }, + "facetStats": {} + } + ] +} diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx new file mode 100644 index 0000000000..0eba2a7cff --- /dev/null +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -0,0 +1,341 @@ +import fetchMock from 'fetch-mock-jest'; +import { + fireEvent, + initializeMocks, + render, + screen, + waitFor, + 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 } from '../data/api.mocks'; +import { mockContentSearchConfig } from '../../search-manager/data/api.mock'; +import { mockBroadcastChannel } from '../../generic/data/api.mock'; +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; +}; + +/** + * Returns 2 components from the search query. + * This lets us test that the StudioHome "View All" button is hidden when a + * low number of search results are shown (<=4 by default). +*/ +const returnLowNumberResults = (_url, req) => { + const requestData = JSON.parse(req.body?.toString() ?? ''); + const query = requestData?.queries[0]?.q ?? ''; + const newMockResult = { ...mockResult }; + // 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. + newMockResult.results[0].query = query; + // Limit number of results to just 2 + newMockResult.results[0].hits = mockResult.results[0]?.hits.slice(0, 2); + newMockResult.results[0].estimatedTotalHits = 2; + // And fake the required '_formatted' fields; it contains the highlighting ... around matched words + // eslint-disable-next-line no-underscore-dangle, no-param-reassign + newMockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); + return newMockResult; +}; + +const path = '/library/:libraryId/*'; +const libraryTitle = mockContentLibrary.libraryData.title; +const collectionTitle = mockCollection.collectionData.title; + +describe('', () => { + beforeEach(() => { + initializeMocks(); + + // The Meilisearch client-side API uses fetch, not Axios. + fetchMock.post(searchEndpoint, (_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 Instantsearch will update the UI and change the query, + // leading to unexpected results in the test cases. + mockResult.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 + mockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); + return mockResult; + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + fetchMock.mockReset(); + }); + + const renderLibraryCollectionPage = async (collectionId?: string, libraryId?: string) => { + const libId = libraryId || mockCollection.libraryId; + const colId = collectionId || mockCollection.collectionId; + render(, { + path, + routerProps: { + initialEntries: [`/library/${libId}/collections/${colId}`], + } + }); + }; + + 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); + 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); + expect(await screen.findByTestId('notFoundAlert')).toBeInTheDocument(); + }); + + it('shows collection data', async () => { + await renderLibraryCollectionPage(); + expect(await screen.findByText('All Collections')).toBeInTheDocument(); + expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); + + expect(screen.queryByText('This collection is currently empty.')).not.toBeInTheDocument(); + + // "Recently Modified" sort shown + expect(screen.getAllByText('Recently Modified').length).toEqual(1); + expect((await screen.findAllByText('Introduction to Testing'))[0]).toBeInTheDocument(); + // Content header with count + expect(await screen.findByText('Content (5)')).toBeInTheDocument(); + }); + + it('shows a collection without associated components', async () => { + fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); + await renderLibraryCollectionPage(); + + expect(await screen.findByText('All Collections')).toBeInTheDocument(); + expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(collectionTitle))[0]).toBeInTheDocument(); + + expect(screen.getByText('This collection is currently empty.')).toBeInTheDocument(); + + const addComponentButton = screen.getAllByRole('button', { name: /new/i })[1]; + fireEvent.click(addComponentButton); + expect(screen.getByText(/add content/i)).toBeInTheDocument(); + }); + + it('shows the new content button', async () => { + await renderLibraryCollectionPage(); + + expect(await screen.findByRole('heading')).toBeInTheDocument(); + expect(await screen.findByText('Content (5)')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /new/i })).toBeInTheDocument(); + expect(screen.queryByText('Read Only')).not.toBeInTheDocument(); + }); + + it('shows an empty read-only library collection, without a new button', async () => { + // 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'); }); + + expect(await screen.findByText('All Collections')).toBeInTheDocument(); + expect(screen.getByText('This collection is currently empty.')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /new/i })).not.toBeInTheDocument(); + expect(screen.getByText('Read Only')).toBeInTheDocument(); + }); + + it('show a collection without search results', async () => { + // Update search mock so it returns no results: + fetchMock.post(searchEndpoint, returnEmptyResult, { overwriteRoutes: true }); + await renderLibraryCollectionPage(); + + 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'); }); + + fireEvent.change(screen.getByRole('searchbox'), { target: { value: 'noresults' } }); + + // Ensure the search endpoint is called again, only once more since the recently modified call + // 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(); + }); + + 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(); + + const newButton = screen.getByRole('button', { name: /new/i }); + fireEvent.click(newButton); + + expect(screen.getByText(/add content/i)).toBeInTheDocument(); + + const closeButton = screen.getByRole('button', { name: /close/i }); + fireEvent.click(closeButton); + + expect(screen.queryByText(/add content/i)).not.toBeInTheDocument(); + }); + + it('should open collection Info by default', async () => { + 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(collectionTitle))[1]).toBeInTheDocument(); + + expect(screen.getByText('Manage')).toBeInTheDocument(); + expect(screen.getByText('Details')).toBeInTheDocument(); + }); + + it('should close and open Collection Info', async () => { + 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(collectionTitle))[1]).toBeInTheDocument(); + + // Open by default; close the library info sidebar + const closeButton = screen.getByRole('button', { name: /close/i }); + fireEvent.click(closeButton); + expect(screen.queryByText('Draft')).not.toBeInTheDocument(); + expect(screen.queryByText('(Never Published)')).not.toBeInTheDocument(); + + // Open library info sidebar with 'Library info' button + const collectionInfoBtn = screen.getByRole('button', { name: /collection info/i }); + fireEvent.click(collectionInfoBtn); + expect(screen.getByText('Manage')).toBeInTheDocument(); + expect(screen.getByText('Details')).toBeInTheDocument(); + }); + + it('sorts collection components', async () => { + await renderLibraryCollectionPage(); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + + expect(await screen.findByTitle('Sort search results')).toBeInTheDocument(); + + const testSortOption = (async (optionText, sortBy, isDefault) => { + // Open the drop-down menu + fireEvent.click(screen.getByTitle('Sort search results')); + + // Click the option with the given text + // Since the sort drop-down also shows the selected sort + // option in its toggle button, we need to make sure we're + // clicking on the last one found. + const options = screen.getAllByText(optionText); + expect(options.length).toBeGreaterThan(0); + fireEvent.click(options[options.length - 1]); + + // Did the search happen with the expected sort option? + const bodyText = sortBy ? `"sort":["${sortBy}"]` : '"sort":[]'; + await waitFor(() => { + expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { + body: expect.stringContaining(bodyText), + method: 'POST', + headers: expect.anything(), + }); + }); + + // Is the sort option stored in the query string? + // Note: we can't easily check this at the moment with + // const searchText = isDefault ? '' : `?sort=${encodeURIComponent(sortBy)}`; + // expect(window.location.href).toEqual(searchText); + + // Is the selected sort option shown in the toggle button (if not default) + // as well as in the drop-down menu? + expect(screen.getAllByText(optionText).length).toEqual(isDefault ? 1 : 2); + }); + + await testSortOption('Title, A-Z', 'display_name:asc', false); + await testSortOption('Title, Z-A', 'display_name:desc', false); + await testSortOption('Newest', 'created:desc', false); + await testSortOption('Oldest', 'created:asc', false); + + // Sorting by Recently Published also excludes unpublished components + await testSortOption('Recently Published', 'last_published:desc', false); + await waitFor(() => { + expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { + body: expect.stringContaining('last_published IS NOT NULL'), + method: 'POST', + headers: expect.anything(), + }); + }); + + // Re-selecting the previous sort option resets sort to default "Recently Modified" + await testSortOption('Recently Published', 'modified:desc', true); + expect(screen.getAllByText('Recently Modified').length).toEqual(2); + + // Enter a keyword into the search box + const searchBox = await screen.findByRole('searchbox'); + fireEvent.change(searchBox, { target: { value: 'words to find' } }); + + // Default sort option changes to "Most Relevant" + expect(screen.getAllByText('Most Relevant').length).toEqual(2); + await waitFor(() => { + expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { + body: expect.stringContaining('"sort":[]'), + method: 'POST', + headers: expect.anything(), + }); + }); + }); + + it('should open and close the component sidebar', async () => { + const mockResult0 = mockResult.results[0].hits[0]; + const displayName = 'Introduction to Testing'; + expect(mockResult0.display_name).toStrictEqual(displayName); + await renderLibraryCollectionPage(); + + // Click on the first component. It should appear twice, in both "Recently Modified" and "Components" + fireEvent.click((await screen.findAllByText(displayName))[0]); + + const sidebar = screen.getByTestId('library-sidebar'); + + const { getByRole, getByText } = within(sidebar); + + await waitFor(() => expect(getByText(displayName)).toBeInTheDocument()); + + const closeButton = getByRole('button', { name: /close/i }); + fireEvent.click(closeButton); + + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); + }); + + 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'); }); + + const filterButton = screen.getByRole('button', { name: /type/i }); + fireEvent.click(filterButton); + + expect(screen.getByText(/no matching components/i)).toBeInTheDocument(); + }); +}); diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 0002f7516a..b5260956db 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -252,3 +252,45 @@ 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', + entities: [], + 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/search-manager/SearchManager.ts b/src/search-manager/SearchManager.ts index 380f62af1b..2d1d1745fd 100644 --- a/src/search-manager/SearchManager.ts +++ b/src/search-manager/SearchManager.ts @@ -7,6 +7,7 @@ import React from 'react'; import { useSearchParams } from 'react-router-dom'; import { MeiliSearch, type Filter } from 'meilisearch'; +import { union } from 'lodash'; import { CollectionHit, ContentHit, SearchSortOption, forceArray, @@ -97,7 +98,7 @@ export const SearchContextProvider: React.FC<{ const [blockTypesFilter, setBlockTypesFilter] = React.useState([]); const [problemTypesFilter, setProblemTypesFilter] = React.useState([]); const [tagsFilter, setTagsFilter] = React.useState([]); - const extraFilter: string[] = forceArray(props.extraFilter); + let extraFilter: string[] = forceArray(props.extraFilter); // The search sort order can be set via the query string // E.g. ?sort=display_name:desc maps to SearchSortOption.TITLE_ZA. @@ -115,7 +116,8 @@ export const SearchContextProvider: React.FC<{ const sort: SearchSortOption[] = (searchSortOrderToUse === SearchSortOption.RELEVANCE ? [] : [searchSortOrderToUse]); // Selecting SearchSortOption.RECENTLY_PUBLISHED also excludes unpublished components. if (searchSortOrderToUse === SearchSortOption.RECENTLY_PUBLISHED) { - extraFilter.push('last_published IS NOT NULL'); + // pushing to array leads to duplicate values if props.extraFilter is already an array. + extraFilter = union(extraFilter, ['last_published IS NOT NULL']); } const canClearFilters = ( diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index 1f170531f3..b02c64b9d0 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -85,6 +85,7 @@ export const useContentSearchResults = ({ client?.config.host, indexName, extraFilter, + fetchCollections, searchKeywords, blockTypesFilter, problemTypesFilter, From 347871b3ca032c81a145dea74bed9c98eeb755e8 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 20:29:10 +0530 Subject: [PATCH 11/22] fix: lint issues --- .../LibraryCollectionPage.test.tsx | 27 +++---------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index 0eba2a7cff..165e913b64 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -9,7 +9,9 @@ import { } from '../../testUtils'; import mockResult from '../__mocks__/collection-search.json'; import mockEmptyResult from '../../search-modal/__mocks__/empty-search-result.json'; -import { mockCollection, mockContentLibrary, mockLibraryBlockTypes, mockXBlockFields } from '../data/api.mocks'; +import { + mockCollection, mockContentLibrary, mockLibraryBlockTypes, mockXBlockFields, +} from '../data/api.mocks'; import { mockContentSearchConfig } from '../../search-manager/data/api.mock'; import { mockBroadcastChannel } from '../../generic/data/api.mock'; import { LibraryLayout } from '..'; @@ -38,27 +40,6 @@ const returnEmptyResult = (_url, req) => { return mockEmptyResult; }; -/** - * Returns 2 components from the search query. - * This lets us test that the StudioHome "View All" button is hidden when a - * low number of search results are shown (<=4 by default). -*/ -const returnLowNumberResults = (_url, req) => { - const requestData = JSON.parse(req.body?.toString() ?? ''); - const query = requestData?.queries[0]?.q ?? ''; - const newMockResult = { ...mockResult }; - // 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. - newMockResult.results[0].query = query; - // Limit number of results to just 2 - newMockResult.results[0].hits = mockResult.results[0]?.hits.slice(0, 2); - newMockResult.results[0].estimatedTotalHits = 2; - // And fake the required '_formatted' fields; it contains the highlighting ... around matched words - // eslint-disable-next-line no-underscore-dangle, no-param-reassign - newMockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); - return newMockResult; -}; - const path = '/library/:libraryId/*'; const libraryTitle = mockContentLibrary.libraryData.title; const collectionTitle = mockCollection.collectionData.title; @@ -94,7 +75,7 @@ describe('', () => { path, routerProps: { initialEntries: [`/library/${libId}/collections/${colId}`], - } + }, }); }; From 0f4bad348e542b6f5792ca09271a9e4846f89843 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 20:48:37 +0530 Subject: [PATCH 12/22] fix: collection querykey --- src/library-authoring/data/apiHooks.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index b10b296067..ed09d2e751 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -62,7 +62,11 @@ export const libraryAuthoringQueryKeys = { 'content', 'libraryBlockTypes', ], - collection: (collectionId?: string) => [...libraryAuthoringQueryKeys.all, collectionId], + collection: (libraryId?: string, collectionId?: string) => [ + ...libraryAuthoringQueryKeys.all, + libraryId, + collectionId, + ], }; export const xblockQueryKeys = { @@ -278,7 +282,7 @@ export const useXBlockOLX = (usageKey: string) => ( */ export const useCollection = (libraryId: string | undefined, collectionId: string | undefined) => ( useQuery({ - queryKey: libraryAuthoringQueryKeys.contentLibrary(collectionId), + queryKey: libraryAuthoringQueryKeys.collection(libraryId, collectionId), queryFn: () => getCollection(libraryId!, collectionId!), enabled: collectionId !== undefined && libraryId !== undefined, }) From c8476d12d998203401f5da0e2ce64cbad1180651 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 16 Sep 2024 20:48:52 +0530 Subject: [PATCH 13/22] test: ignore unused functions --- src/search-manager/data/api.ts | 1 + src/search-manager/data/apiHooks.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index 3a99672984..34e19a352f 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -498,6 +498,7 @@ export async function fetchTagsThatMatchKeyword({ /** * Fetch single document by its id */ +/* istanbul ignore next */ export async function fetchDocumentById({ client, indexName, id } : { /** The Meilisearch client instance */ client: MeiliSearch; diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index b02c64b9d0..c9fd79e83f 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -242,6 +242,7 @@ export const useTagFilterOptions = (args: { return { ...mainQuery, data }; }; +/* istanbul ignore next */ export const useGetSingleDocument = ({ client, indexName, id }: { client?: MeiliSearch; indexName?: string; From d510554b1f3ebe58c25c1a9ebc6eb2db827a504d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 17 Sep 2024 12:08:17 +0530 Subject: [PATCH 14/22] refactor: remove unnecessary comment --- src/library-authoring/collections/LibraryCollectionPage.tsx | 2 +- src/search-manager/SearchManager.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 995906a9df..62945a88b1 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -157,7 +157,7 @@ const LibraryCollectionPage = () => { canEditLibrary={libraryData.canEditLibrary} infoClickHandler={openCollectionInfoSidebar} /> -)} + )} breadcrumbs={( Date: Tue, 17 Sep 2024 20:29:34 +0530 Subject: [PATCH 15/22] feat: create component under collection --- src/editors/EditorContainer.tsx | 12 +++++++----- src/library-authoring/LibraryLayout.tsx | 16 +++++++++++----- .../add-content/AddContentContainer.tsx | 13 +++++++++---- src/library-authoring/data/api.ts | 3 +++ src/library-authoring/data/apiHooks.ts | 2 +- 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/editors/EditorContainer.tsx b/src/editors/EditorContainer.tsx index c5b2d5ec89..1f2e2eda29 100644 --- a/src/editors/EditorContainer.tsx +++ b/src/editors/EditorContainer.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { useParams } from 'react-router-dom'; +import { useLocation, useParams } from 'react-router-dom'; import { getConfig } from '@edx/frontend-platform'; import EditorPage from './EditorPage'; @@ -8,7 +8,7 @@ interface Props { /** Course ID or Library ID */ learningContextId: string; /** Event handler sometimes called when user cancels out of the editor page */ - onClose?: () => void; + onClose?: (prevPath?: string) => void; /** * Event handler called after when user saves their changes using an editor * and sometimes called when user cancels the editor, instead of onClose. @@ -17,7 +17,7 @@ interface Props { * TODO: clean this up so there are separate onCancel and onSave callbacks, * and they are used consistently instead of this mess. */ - returnFunction?: () => (newData: Record | undefined) => void; + returnFunction?: (prevPath?: string) => (newData: Record | undefined) => void; } const EditorContainer: React.FC = ({ @@ -26,6 +26,8 @@ const EditorContainer: React.FC = ({ returnFunction, }) => { const { blockType, blockId } = useParams(); + const location = useLocation(); + if (blockType === undefined || blockId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. return
Error: missing URL parameters
; @@ -38,8 +40,8 @@ const EditorContainer: React.FC = ({ blockId={blockId} studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} - onClose={onClose} - returnFunction={returnFunction} + onClose={onClose ? () => onClose(location.state?.from) : undefined} + returnFunction={returnFunction ? () => returnFunction(location.state?.from) : undefined} />
); diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 6d7bce678d..b712586b5f 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -25,14 +25,20 @@ const LibraryLayout = () => { } const navigate = useNavigate(); - const goBack = React.useCallback(() => { - // Go back to the library - navigate(`/library/${libraryId}`); + const goBack = React.useCallback((prevPath?: string) => { + if (prevPath) { + // Redirects back to the previous route like collection page or library page + navigate(prevPath); + } else { + // Go back to the library + navigate(`/library/${libraryId}`); + } }, []); - const returnFunction = React.useCallback(() => { + + const returnFunction = React.useCallback((prevPath?: string) => { // When changes are cancelled, either onClose (goBack) or this returnFunction will be called. // When changes are saved, this returnFunction is called. - goBack(); + goBack(prevPath); return (args) => { if (args === undefined) { return; // Do nothing - the user cancelled the changes diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 0ba42b2d04..5dfd4cd9a0 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -16,7 +16,7 @@ import { ContentPaste, } from '@openedx/paragon/icons'; import { v4 as uuid4 } from 'uuid'; -import { useNavigate, useParams } from 'react-router-dom'; +import { useLocation, useNavigate, useParams } from 'react-router-dom'; import { ToastContext } from '../../generic/toast-context'; import { useCopyToClipboard } from '../../generic/clipboard'; @@ -62,7 +62,9 @@ const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonPro const AddContentContainer = () => { const intl = useIntl(); const navigate = useNavigate(); - const { libraryId } = useParams(); + const location = useLocation(); + const currentPath = location.pathname; + const { libraryId, collectionId } = useParams(); const createBlockMutation = useCreateLibraryBlock(); const pasteClipboardMutation = useLibraryPasteClipboard(); const { showToast } = useContext(ToastContext); @@ -147,10 +149,13 @@ const AddContentContainer = () => { libraryId, blockType, definitionId: `${uuid4()}`, + collectionId, }).then((data) => { const editUrl = getEditUrl(data.id); if (editUrl) { - navigate(editUrl); + // Pass currentPath in state so that we can come back to + // current page on save or cancel + navigate(editUrl, { state: { from: currentPath } }); } else { // We can't start editing this right away so just show a toast message: showToast(intl.formatMessage(messages.successCreateMessage)); @@ -168,7 +173,7 @@ const AddContentContainer = () => { return ( - + {!collectionId && }
{contentTypes.map((contentType) => ( { const client = getAuthenticatedHttpClient(); const { data } = await client.post( @@ -204,6 +206,7 @@ export async function createLibraryBlock({ { block_type: blockType, definition_id: definitionId, + collection_key: collectionId, }, ); return camelCaseObject(data); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index ed09d2e751..46eb958e91 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -128,7 +128,7 @@ export const useCreateLibraryBlock = () => { mutationFn: createLibraryBlock, onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(variables.libraryId) }); - queryClient.invalidateQueries({ queryKey: ['content_search'] }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, variables.libraryId) }); }, }); }; From 5846b83d0c38d539f6e2c00a1f06bea0a8b99976 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 17 Sep 2024 21:06:32 +0530 Subject: [PATCH 16/22] fix: test --- src/editors/EditorContainer.test.jsx | 1 + src/editors/EditorContainer.tsx | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/editors/EditorContainer.test.jsx b/src/editors/EditorContainer.test.jsx index d57d14c6b1..ea5ec4a8b4 100644 --- a/src/editors/EditorContainer.test.jsx +++ b/src/editors/EditorContainer.test.jsx @@ -8,6 +8,7 @@ jest.mock('react-router', () => ({ blockId: 'company-id1', blockType: 'html', }), + useLocation: () => {}, })); const props = { learningContextId: 'cOuRsEId' }; diff --git a/src/editors/EditorContainer.tsx b/src/editors/EditorContainer.tsx index 1f2e2eda29..90bd248766 100644 --- a/src/editors/EditorContainer.tsx +++ b/src/editors/EditorContainer.tsx @@ -40,8 +40,8 @@ const EditorContainer: React.FC = ({ blockId={blockId} studioEndpointUrl={getConfig().STUDIO_BASE_URL} lmsEndpointUrl={getConfig().LMS_BASE_URL} - onClose={onClose ? () => onClose(location.state?.from) : undefined} - returnFunction={returnFunction ? () => returnFunction(location.state?.from) : undefined} + onClose={onClose ? () => onClose(location.state?.from) : null} + returnFunction={returnFunction ? () => returnFunction(location.state?.from) : null} />
); From 2ae6c98fb90cd5d0917ddce16a464cbf2f570891 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 18 Sep 2024 15:30:28 +0530 Subject: [PATCH 17/22] fix: lint issues --- src/library-authoring/collections/LibraryCollectionPage.tsx | 1 + src/library-authoring/common/context.tsx | 2 +- src/library-authoring/data/api.mocks.ts | 1 - src/library-authoring/data/api.ts | 2 -- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 62945a88b1..3e09289d4b 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -94,6 +94,7 @@ 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'); } diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index b3811e6ddf..cd82a2d84a 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -30,7 +30,7 @@ export const LibraryContext = React.createContext({ isCreateCollectionModalOpen: false, openCreateCollectionModal: () => {}, closeCreateCollectionModal: () => {}, - openCollectionInfoSidebar: () => {}, // eslint-disable-line @typescript-eslint/no-unused-vars + openCollectionInfoSidebar: () => {}, } as LibraryContextData); /** diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index b5260956db..75f49bd150 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -283,7 +283,6 @@ mockCollection.collectionData = { key: mockCollection.collectionId, title: 'My first collection', description: 'A collection for testing', - entities: [], created: '2024-06-26T14:19:59Z', modified: '2024-07-20T17:36:51Z', enabled: true, diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 335a2ac4a5..0ac62e556e 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -88,8 +88,6 @@ export interface Collection { createdBy: string | null; created: string; modified: string; - // TODO: Update the type below once entities are properly linked - entities: Array; learningPackage: number; } From 26c0a9ef11d01ad08e8787e29a3feb2a76e14bfd Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 18 Sep 2024 15:30:39 +0530 Subject: [PATCH 18/22] feat: open collections page on save --- .../create-collection/CreateCollectionModal.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/library-authoring/create-collection/CreateCollectionModal.tsx b/src/library-authoring/create-collection/CreateCollectionModal.tsx index 905272b181..74bd7b1c85 100644 --- a/src/library-authoring/create-collection/CreateCollectionModal.tsx +++ b/src/library-authoring/create-collection/CreateCollectionModal.tsx @@ -5,7 +5,7 @@ import { Form, ModalDialog, } from '@openedx/paragon'; -import { useParams } from 'react-router-dom'; +import { useNavigate, useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Formik } from 'formik'; import * as Yup from 'yup'; @@ -17,6 +17,7 @@ import { ToastContext } from '../../generic/toast-context'; const CreateCollectionModal = () => { const intl = useIntl(); + const navigate = useNavigate(); const { libraryId } = useParams(); if (!libraryId) { throw new Error('Rendered without libraryId URL parameter'); @@ -29,8 +30,9 @@ const CreateCollectionModal = () => { const { showToast } = React.useContext(ToastContext); const handleCreate = React.useCallback((values) => { - create.mutateAsync(values).then(() => { + create.mutateAsync(values).then((data) => { closeCreateCollectionModal(); + navigate(`/library/${libraryId}/collections/${data.key}`); showToast(intl.formatMessage(messages.createCollectionSuccess)); }).catch(() => { showToast(intl.formatMessage(messages.createCollectionError)); From 2c06446916f4f52fb790a1bd27173b23862b3158 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 19 Sep 2024 12:22:40 +0530 Subject: [PATCH 19/22] refactor: update collection url --- src/library-authoring/LibraryLayout.tsx | 2 +- .../collections/LibraryCollectionPage.test.tsx | 2 +- .../create-collection/CreateCollectionModal.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index b712586b5f..abd2cf8ee6 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -66,7 +66,7 @@ const LibraryLayout = () => { )} /> } /> ', () => { render(, { path, routerProps: { - initialEntries: [`/library/${libId}/collections/${colId}`], + initialEntries: [`/library/${libId}/collection/${colId}`], }, }); }; diff --git a/src/library-authoring/create-collection/CreateCollectionModal.tsx b/src/library-authoring/create-collection/CreateCollectionModal.tsx index 74bd7b1c85..cc611b3a96 100644 --- a/src/library-authoring/create-collection/CreateCollectionModal.tsx +++ b/src/library-authoring/create-collection/CreateCollectionModal.tsx @@ -32,7 +32,7 @@ const CreateCollectionModal = () => { const handleCreate = React.useCallback((values) => { create.mutateAsync(values).then((data) => { closeCreateCollectionModal(); - navigate(`/library/${libraryId}/collections/${data.key}`); + navigate(`/library/${libraryId}/collection/${data.key}`); showToast(intl.formatMessage(messages.createCollectionSuccess)); }).catch(() => { showToast(intl.formatMessage(messages.createCollectionError)); From 3395c1e090528d4c9a0b40f2f12556333a2576ef Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 19 Sep 2024 12:44:59 +0530 Subject: [PATCH 20/22] fix: lint issues --- .../collections/LibraryCollections.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/library-authoring/collections/LibraryCollections.tsx b/src/library-authoring/collections/LibraryCollections.tsx index 2ba82c02b6..97d194f4a3 100644 --- a/src/library-authoring/collections/LibraryCollections.tsx +++ b/src/library-authoring/collections/LibraryCollections.tsx @@ -41,23 +41,25 @@ const LibraryCollections = ({ variant }: LibraryCollectionsProps) => { ); if (totalCollectionHits === 0) { - return isFiltered ? - - : ; + return isFiltered + ? + : ( + + ); } return (
- { collectionList.map((collectionHit) => ( + {collectionList.map((collectionHit) => ( - )) } + ))}
); }; From 2848746183a62ec155e83e2dbc20a65a055a4b4d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 19 Sep 2024 19:42:14 +0530 Subject: [PATCH 21/22] 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 cec5262a49..343bb94f62 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -168,7 +168,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 34e19a352f..d5d524a81e 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, From d388335e3721a7beb5eddb2cf5fd15c6597c433a Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 20 Sep 2024 12:36:08 +0530 Subject: [PATCH 22/22] feat: api call for adding component to collection --- .../add-content/AddContentContainer.tsx | 7 ++++-- src/library-authoring/add-content/messages.ts | 5 +++++ src/library-authoring/data/api.ts | 16 +++++++++++--- src/library-authoring/data/apiHooks.test.tsx | 19 +++++++++++++++- src/library-authoring/data/apiHooks.ts | 22 +++++++++++++++++++ 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 5dfd4cd9a0..1d46aaacbb 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -21,7 +21,7 @@ import { useLocation, useNavigate, useParams } from 'react-router-dom'; import { ToastContext } from '../../generic/toast-context'; import { useCopyToClipboard } from '../../generic/clipboard'; import { getCanEdit } from '../../course-unit/data/selectors'; -import { useCreateLibraryBlock, useLibraryPasteClipboard } from '../data/apiHooks'; +import { useCreateLibraryBlock, useLibraryPasteClipboard, useUpdateCollectionComponents } from '../data/apiHooks'; import { getEditUrl } from '../components/utils'; import messages from './messages'; @@ -66,6 +66,7 @@ const AddContentContainer = () => { const currentPath = location.pathname; const { libraryId, collectionId } = useParams(); const createBlockMutation = useCreateLibraryBlock(); + const updateComponentsMutation = useUpdateCollectionComponents(libraryId, collectionId); const pasteClipboardMutation = useLibraryPasteClipboard(); const { showToast } = useContext(ToastContext); const canEdit = useSelector(getCanEdit); @@ -149,9 +150,11 @@ const AddContentContainer = () => { libraryId, blockType, definitionId: `${uuid4()}`, - collectionId, }).then((data) => { const editUrl = getEditUrl(data.id); + updateComponentsMutation.mutateAsync([data.id]).catch(() => { + showToast(intl.formatMessage(messages.errorAssociateComponentMessage)); + }); if (editUrl) { // Pass currentPath in state so that we can come back to // current page on save or cancel diff --git a/src/library-authoring/add-content/messages.ts b/src/library-authoring/add-content/messages.ts index 4d84d344ea..286e41793e 100644 --- a/src/library-authoring/add-content/messages.ts +++ b/src/library-authoring/add-content/messages.ts @@ -51,6 +51,11 @@ const messages = defineMessages({ defaultMessage: 'There was an error creating the content.', description: 'Message when creation of content in library is on error', }, + errorAssociateComponentMessage: { + id: 'course-authoring.library-authoring.associate-collection-content.error.text', + defaultMessage: 'There was an error linking the content to this collection.', + description: 'Message when linking of content to a collection in library fails', + }, addContentTitle: { id: 'course-authoring.library-authoring.sidebar.title.add-content', defaultMessage: 'Add Content', diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index b1d6f34369..894151e903 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -54,6 +54,10 @@ export const getLibraryCollectionsApiUrl = (libraryId: string) => `${getApiBaseU * Get the URL for the collection API. */ export const getLibraryCollectionApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionsApiUrl(libraryId)}${collectionId}/`; +/** + * Get the URL for the collection API. + */ +export const getLibraryCollectionComponentApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionApiUrl(libraryId, collectionId)}components/`; export interface ContentLibrary { id: string; @@ -132,7 +136,6 @@ export interface CreateBlockDataRequest { libraryId: string; blockType: string; definitionId: string; - collectionId?: string; } export interface LibraryBlockMetadata { @@ -196,7 +199,6 @@ export async function createLibraryBlock({ libraryId, blockType, definitionId, - collectionId, }: CreateBlockDataRequest): Promise { const client = getAuthenticatedHttpClient(); const { data } = await client.post( @@ -204,7 +206,6 @@ export async function createLibraryBlock({ { block_type: blockType, definition_id: definitionId, - collection_key: collectionId, }, ); return camelCaseObject(data); @@ -313,3 +314,12 @@ export async function getXBlockOLX(usageKey: string): Promise { const { data } = await getAuthenticatedHttpClient().get(getXBlockOLXApiUrl(usageKey)); return data.olx; } + +/** + * Update collection components. + */ +export async function updateCollectionComponents(libraryId:string, collectionId: string, usageKeys: string[]) { + await getAuthenticatedHttpClient().patch(getLibraryCollectionComponentApiUrl(libraryId, collectionId), { + usage_keys: usageKeys, + }); +} diff --git a/src/library-authoring/data/apiHooks.test.tsx b/src/library-authoring/data/apiHooks.test.tsx index a92546c10f..70ba691635 100644 --- a/src/library-authoring/data/apiHooks.test.tsx +++ b/src/library-authoring/data/apiHooks.test.tsx @@ -5,12 +5,18 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { renderHook } from '@testing-library/react-hooks'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import MockAdapter from 'axios-mock-adapter'; -import { getCommitLibraryChangesUrl, getCreateLibraryBlockUrl, getLibraryCollectionsApiUrl } from './api'; +import { + getCommitLibraryChangesUrl, + getCreateLibraryBlockUrl, + getLibraryCollectionComponentApiUrl, + getLibraryCollectionsApiUrl, +} from './api'; import { useCommitLibraryChanges, useCreateLibraryBlock, useCreateLibraryCollection, useRevertLibraryChanges, + useUpdateCollectionComponents, } from './apiHooks'; let axiosMock; @@ -89,4 +95,15 @@ describe('library api hooks', () => { expect(axiosMock.history.post[0].url).toEqual(url); }); + + it('should add components to collection', async () => { + const libraryId = 'lib:org:1'; + const collectionId = 'my-first-collection'; + const url = getLibraryCollectionComponentApiUrl(libraryId, collectionId); + axiosMock.onPatch(url).reply(200); + const { result } = renderHook(() => useUpdateCollectionComponents(libraryId, collectionId), { wrapper }); + await result.current.mutateAsync(['some-usage-key']); + + expect(axiosMock.history.patch[0].url).toEqual(url); + }); }); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 08dee673e8..ef48443c3c 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -26,6 +26,7 @@ import { updateXBlockFields, createCollection, getXBlockOLX, + updateCollectionComponents, type CreateLibraryCollectionDataRequest, } from './api'; @@ -275,3 +276,24 @@ export const useXBlockOLX = (usageKey: string) => ( enabled: !!usageKey, }) ); + +/** + * Use this mutation to add components to a collection in a library + */ +export const useUpdateCollectionComponents = (libraryId?: string, collectionId?: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async (usage_keys: string[]) => { + if (libraryId !== undefined && collectionId !== undefined) { + return updateCollectionComponents(libraryId, collectionId, usage_keys); + } + return undefined; + }, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + onSettled: (_data, _error, _variables) => { + if (libraryId !== undefined && collectionId !== undefined) { + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); + } + }, + }); +};