diff --git a/.env b/.env index 23fa3de59..3237da4c3 100644 --- a/.env +++ b/.env @@ -45,7 +45,7 @@ INVITE_STUDENTS_EMAIL_TO='' ENABLE_CHECKLIST_QUALITY='' ENABLE_GRADING_METHOD_IN_PROBLEMS=false # "Multi-level" blocks are unsupported in libraries -LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder" +LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder,library_content,itembank" # Fallback in local style files PARAGON_THEME_URLS={} COURSE_TEAM_SUPPORT_EMAIL='' diff --git a/.env.development b/.env.development index c24368594..970902cff 100644 --- a/.env.development +++ b/.env.development @@ -48,7 +48,7 @@ INVITE_STUDENTS_EMAIL_TO="someone@domain.com" ENABLE_CHECKLIST_QUALITY=true ENABLE_GRADING_METHOD_IN_PROBLEMS=false # "Multi-level" blocks are unsupported in libraries -LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder,library_content" +LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder,library_content,itembank" # Fallback in local style files PARAGON_THEME_URLS={} COURSE_TEAM_SUPPORT_EMAIL='' diff --git a/.env.test b/.env.test index e78d32b32..0e1e83d0c 100644 --- a/.env.test +++ b/.env.test @@ -40,6 +40,6 @@ INVITE_STUDENTS_EMAIL_TO="someone@domain.com" ENABLE_CHECKLIST_QUALITY=true ENABLE_GRADING_METHOD_IN_PROBLEMS=false # "Multi-level" blocks are unsupported in libraries -LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder" +LIBRARY_UNSUPPORTED_BLOCKS="conditional,step-builder,problem-builder,library_content,itembank" PARAGON_THEME_URLS= COURSE_TEAM_SUPPORT_EMAIL='support@example.com' diff --git a/src/library-authoring/import-course/stepper/ImportStepperPage.test.tsx b/src/library-authoring/import-course/stepper/ImportStepperPage.test.tsx index 1d309c14f..5f9d986a1 100644 --- a/src/library-authoring/import-course/stepper/ImportStepperPage.test.tsx +++ b/src/library-authoring/import-course/stepper/ImportStepperPage.test.tsx @@ -34,6 +34,7 @@ jest.mock('react-router-dom', () => ({ // Mock the useGetBlockTypes hook jest.mock('@src/search-manager', () => ({ useGetBlockTypes: jest.fn().mockReturnValue({ isPending: true, data: null }), + useGetContentHits: jest.fn().mockReturnValue({ isPending: true, data: null }), })); const renderComponent = (studioHomeState: Partial = {}) => { @@ -159,7 +160,7 @@ describe('', () => { expect(courseCard).toBeChecked(); }); - it('should import select course on button click', async () => { + it('should import selected course on button click', async () => { (useGetBlockTypes as jest.Mock).mockReturnValue({ isPending: false, data: { diff --git a/src/library-authoring/import-course/stepper/ReviewImportDetails.test.tsx b/src/library-authoring/import-course/stepper/ReviewImportDetails.test.tsx index ec4c96c69..b84af7b45 100644 --- a/src/library-authoring/import-course/stepper/ReviewImportDetails.test.tsx +++ b/src/library-authoring/import-course/stepper/ReviewImportDetails.test.tsx @@ -1,6 +1,6 @@ import { useCourseDetails } from '@src/course-outline/data/apiHooks'; import { useMigrationInfo } from '@src/library-authoring/data/apiHooks'; -import { useGetBlockTypes } from '@src/search-manager'; +import { useGetBlockTypes, useGetContentHits } from '@src/search-manager'; import { render as baseRender, screen, initializeMocks } from '@src/testUtils'; import { LibraryProvider } from '@src/library-authoring/common/context/LibraryContext'; import { mockContentLibrary } from '@src/library-authoring/data/api.mocks'; @@ -25,6 +25,7 @@ jest.mock('@src/library-authoring/data/apiHooks', () => ({ // Mock the useGetBlockTypes hook jest.mock('@src/search-manager', () => ({ useGetBlockTypes: jest.fn().mockReturnValue({ isPending: true, data: null }), + useGetContentHits: jest.fn().mockReturnValue({ isPending: true, data: null }), })); const render = (element: React.ReactElement) => { @@ -80,7 +81,7 @@ describe('ReviewImportDetails', () => { }], }, }); - (useGetBlockTypes as jest.Mock).mockReturnValue({ + (useGetBlockTypes as jest.Mock).mockReturnValueOnce({ isPending: false, data: { html: 1 }, }); @@ -103,7 +104,7 @@ describe('ReviewImportDetails', () => { isPending: false, data: null, }); - (useGetBlockTypes as jest.Mock).mockReturnValue({ + (useGetBlockTypes as jest.Mock).mockReturnValueOnce({ isPending: false, data: { chapter: 1, @@ -134,13 +135,63 @@ describe('ReviewImportDetails', () => { expect(markAnalysisComplete).toHaveBeenCalledWith(true); }); + it('considers children blocks of unsupportedBlocks', async () => { + (useCourseDetails as jest.Mock).mockReturnValue({ isPending: false, data: { title: 'Test Course' } }); + (useMigrationInfo as jest.Mock).mockReturnValue({ + isPending: false, + data: null, + }); + (useGetContentHits as jest.Mock).mockReturnValue({ + isPending: false, + data: { + hits: [{ usage_key: 'some-usage-key' }], + estimatedTotalHits: 1, + }, + }); + (useGetBlockTypes as jest.Mock).mockReturnValueOnce({ + isPending: false, + data: { + chapter: 1, + sequential: 2, + vertical: 3, + library_content: 1, + html: 1, + problem: 4, + }, + }).mockReturnValueOnce({ + isPending: false, + data: { + problem: 2, + }, + }); + + render(); + + expect(await screen.findByRole('alert')).toBeInTheDocument(); + expect(await screen.findByText(/Import Analysis Complete/i)).toBeInTheDocument(); + expect(await screen.findByText( + /25.00% of content cannot be imported. For details see below./i, + )).toBeInTheDocument(); + expect(await screen.findByText(/Total Blocks/i)).toBeInTheDocument(); + expect(await screen.findByText('9/12')).toBeInTheDocument(); + expect(await screen.findByText('Sections')).toBeInTheDocument(); + expect(await screen.findByText('1')).toBeInTheDocument(); + expect(await screen.findByText('Subsections')).toBeInTheDocument(); + expect(await screen.findByText('2')).toBeInTheDocument(); + expect(await screen.findByText('Units')).toBeInTheDocument(); + expect(await screen.findByText('3')).toBeInTheDocument(); + expect(await screen.findByText('Components')).toBeInTheDocument(); + expect(await screen.findByText('3/6')).toBeInTheDocument(); + expect(markAnalysisComplete).toHaveBeenCalledWith(true); + }); + it('renders success alert when no unsupported blocks', async () => { (useCourseDetails as jest.Mock).mockReturnValue({ isPending: false, data: { title: 'Test Course' } }); (useMigrationInfo as jest.Mock).mockReturnValue({ isPending: false, data: null, }); - (useGetBlockTypes as jest.Mock).mockReturnValue({ + (useGetBlockTypes as jest.Mock).mockReturnValueOnce({ isPending: false, data: { chapter: 1, diff --git a/src/library-authoring/import-course/stepper/ReviewImportDetails.tsx b/src/library-authoring/import-course/stepper/ReviewImportDetails.tsx index ebd7cfe6c..75ddebbbb 100644 --- a/src/library-authoring/import-course/stepper/ReviewImportDetails.tsx +++ b/src/library-authoring/import-course/stepper/ReviewImportDetails.tsx @@ -8,7 +8,7 @@ import { useEffect, useMemo } from 'react'; import { CheckCircle, Warning } from '@openedx/paragon/icons'; import { useLibraryContext } from '@src/library-authoring/common/context/LibraryContext'; import { useMigrationInfo } from '@src/library-authoring/data/apiHooks'; -import { useGetBlockTypes } from '@src/search-manager'; +import { useGetBlockTypes, useGetContentHits } from '@src/search-manager'; import { SummaryCard } from './SummaryCard'; import messages from '../messages'; @@ -120,30 +120,72 @@ export const ReviewImportDetails = ({ courseId, markAnalysisComplete }: Props) = ]); useEffect(() => { + // Mark complete to inform parent component of analysis completion. markAnalysisComplete(!isBlockDataPending); }, [isBlockDataPending]); - const totalUnsupportedBlocks = useMemo(() => { + /** Filter unsupported blocks by checking if the block type is in the library's list of unsupported blocks. */ + const unsupportedBlockTypes = useMemo(() => { if (!blockTypes) { - return 0; + return undefined; } - const unsupportedBlocks = Object.entries(blockTypes).reduce((total, [blockType, count]) => { - const isUnsupportedBlock = getConfig().LIBRARY_UNSUPPORTED_BLOCKS.includes(blockType); - if (isUnsupportedBlock) { - return total + count; - } - return total; - }, 0); - return unsupportedBlocks; + return Object.entries(blockTypes).filter(([blockType]) => ( + getConfig().LIBRARY_UNSUPPORTED_BLOCKS.includes(blockType) + )); }, [blockTypes]); + /** Calculate the total number of unsupported blocks by summing up the count for each block type. */ + const totalUnsupportedBlocks = useMemo(() => { + if (!unsupportedBlockTypes) { + return 0; + } + const unsupportedBlocks = unsupportedBlockTypes.reduce((total, [, count]) => total + count, 0); + return unsupportedBlocks; + }, [unsupportedBlockTypes]); + + // Fetch unsupported blocks usage_key information from meilisearch index. + const { data: unsupportedBlocksData } = useGetContentHits( + [ + `context_key = "${courseId}"`, + `block_type IN [${unsupportedBlockTypes?.flatMap(([value]) => `"${value}"`).join(',')}]`, + ], + totalUnsupportedBlocks > 0, + ['usage_key'], + totalUnsupportedBlocks, + 'always', + ); + + // Fetch children blocks for each block in the unsupportedBlocks array. + const { data: unsupportedBlocksChildren } = useGetBlockTypes([ + `context_key = "${courseId}"`, + `breadcrumbs.usage_key IN [${unsupportedBlocksData?.hits.map((value) => `"${value.usage_key}"`).join(',')}]`, + ], (unsupportedBlocksData?.estimatedTotalHits || 0) > 0); + + /** Calculate the total number of unsupported children blocks by summing up the count for each block. */ + const totalUnsupportedBlockChildren = useMemo(() => { + if (!unsupportedBlocksChildren) { + return 0; + } + const unsupportedBlocks = Object.values(unsupportedBlocksChildren).reduce((total, count) => total + count, 0); + return unsupportedBlocks; + }, [unsupportedBlocksChildren]); + + /** Finally calculate the final number of unsupported blocks by adding parent unsupported and children + unsupported blocks. */ + const finalUnssupportedBlocks = useMemo( + () => totalUnsupportedBlocks + totalUnsupportedBlockChildren, + [totalUnsupportedBlocks, totalUnsupportedBlockChildren], + ); + + /** Calculate total supported blocks by subtracting final unsupported blocks from the total number of blocks */ const totalBlocks = useMemo(() => { if (!blockTypes) { return undefined; } - return Object.values(blockTypes).reduce((total, block) => total + block, 0) - totalUnsupportedBlocks; - }, [blockTypes]); + return Object.values(blockTypes).reduce((total, block) => total + block, 0) - finalUnssupportedBlocks; + }, [blockTypes, finalUnssupportedBlocks]); + /** Calculate total components by excluding those that are chapters, sequential, or vertical. */ const totalComponents = useMemo(() => { if (!blockTypes) { return undefined; @@ -157,15 +199,16 @@ export const ReviewImportDetails = ({ courseId, markAnalysisComplete }: Props) = return total; }, 0, - ) - totalUnsupportedBlocks; - }, [blockTypes]); + ) - finalUnssupportedBlocks; + }, [blockTypes, finalUnssupportedBlocks]); + /** Calculate the unsupported block percentage based on the final total blocks and unsupported blocks. */ const unsupportedBlockPercentage = useMemo(() => { if (!blockTypes || !totalBlocks) { return 0; } - return (totalUnsupportedBlocks / (totalBlocks + totalUnsupportedBlocks)) * 100; - }, [blockTypes]); + return (finalUnssupportedBlocks / (totalBlocks + finalUnssupportedBlocks)) * 100; + }, [blockTypes, finalUnssupportedBlocks]); return ( @@ -181,10 +224,10 @@ export const ReviewImportDetails = ({ courseId, markAnalysisComplete }: Props) = sections={blockTypes?.chapter} subsections={blockTypes?.sequential} units={blockTypes?.vertical} - unsupportedBlocks={totalUnsupportedBlocks} + unsupportedBlocks={finalUnssupportedBlocks} isPending={isBlockDataPending} /> - {!isBlockDataPending && totalUnsupportedBlocks > 0 + {!isBlockDataPending && finalUnssupportedBlocks > 0 && ( <>

diff --git a/src/search-manager/data/api.mock.ts b/src/search-manager/data/api.mock.ts index 5153e7971..8ddd3738e 100644 --- a/src/search-manager/data/api.mock.ts +++ b/src/search-manager/data/api.mock.ts @@ -16,6 +16,7 @@ export async function mockContentSearchConfig(): ReturnType ( jest.spyOn(api, 'getContentSearchConfig').mockImplementation(mockContentSearchConfig) ); @@ -102,3 +103,24 @@ mockFetchIndexDocuments.applyMock = () => { { overwriteRoutes: true }, ); }; + +/** + * Mock the useGetContentHits + */ +export async function mockGetContentHits( + mockResponse: 'noHits' | 'someHits', +) { + fetchMock.post(mockContentSearchConfig.searchEndpointUrl, () => { + const mockResponseMap = { + noHits: { + hits: [], + estimatedTotalHits: 0, + }, + someHits: { + hits: [{ usage_key: 'some-key' }, { usage_key: 'other-key' }], + estimatedTotalHits: 2, + }, + }; + return mockResponseMap[mockResponse]; + }); +} diff --git a/src/search-manager/data/api.ts b/src/search-manager/data/api.ts index d6cf9db23..80e4fea0d 100644 --- a/src/search-manager/data/api.ts +++ b/src/search-manager/data/api.ts @@ -1,7 +1,7 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import type { - Filter, MeiliSearch, MultiSearchQuery, + Filter, MeiliSearch, MultiSearchQuery, SearchResponse, } from 'meilisearch'; import { ContainerType } from '../../generic/key-utils'; @@ -552,3 +552,25 @@ export async function fetchTagsThatMatchKeyword({ return { matches: Array.from(matches).map((tagPath) => ({ tagPath })), mayBeMissingResults: hits.length === limit }; } + +/** + * Fetch the blocks that match query + */ +export const fetchContentHits = async ( + client: MeiliSearch, + indexName: string, + extraFilter?: Filter, + limit?: number, + attributesToRetrieve?: string[], +): Promise>> => { + // Convert 'extraFilter' into an array + const extraFilterFormatted = forceArray(extraFilter); + + const results = await client.index(indexName).search('', { + filter: extraFilterFormatted, + limit, + attributesToRetrieve, + }); + + return results; +}; diff --git a/src/search-manager/data/apiHooks.test.tsx b/src/search-manager/data/apiHooks.test.tsx index b347d2e95..f4908ef62 100644 --- a/src/search-manager/data/apiHooks.test.tsx +++ b/src/search-manager/data/apiHooks.test.tsx @@ -3,9 +3,9 @@ import { renderHook, waitFor } from '@testing-library/react'; import fetchMock from 'fetch-mock-jest'; import mockResult from './__mocks__/block-types.json'; -import { mockContentSearchConfig } from './api.mock'; +import { mockContentSearchConfig, mockGetContentHits } from './api.mock'; import { - useGetBlockTypes, + useGetBlockTypes, useGetContentHits, } from './apiHooks'; mockContentSearchConfig.applyMock(); @@ -53,4 +53,17 @@ describe('search manager api hooks', () => { expect(result.current.data).toEqual(expectedData); expect(fetchMock.calls().length).toEqual(1); }); + + it('useGetContentHits should return hits', async () => { + mockGetContentHits('someHits'); + const { result } = renderHook(() => useGetContentHits('filter'), { wrapper }); + await waitFor(() => { + expect(result.current.isPending).toBeFalsy(); + }); + const expectedData = { + hits: [{ usage_key: 'some-key' }, { usage_key: 'other-key' }], + estimatedTotalHits: 2, + }; + expect(result.current.data).toEqual(expectedData); + }); }); diff --git a/src/search-manager/data/apiHooks.ts b/src/search-manager/data/apiHooks.ts index 0c1aa947b..34a785393 100644 --- a/src/search-manager/data/apiHooks.ts +++ b/src/search-manager/data/apiHooks.ts @@ -1,5 +1,7 @@ import React from 'react'; -import { keepPreviousData, useInfiniteQuery, useQuery } from '@tanstack/react-query'; +import { + keepPreviousData, skipToken, useInfiniteQuery, useQuery, +} from '@tanstack/react-query'; import { type Filter, MeiliSearch } from 'meilisearch'; import { @@ -11,6 +13,7 @@ import { getContentSearchConfig, fetchBlockTypes, type PublishStatus, + fetchContentHits, } from './api'; /** @@ -286,7 +289,7 @@ export const useTagFilterOptions = (args: { return { ...mainQuery, data }; }; -export const useGetBlockTypes = (extraFilters: Filter) => { +export const useGetBlockTypes = (extraFilters: Filter, enabled: boolean = true) => { const { client, indexName } = useContentSearchConnection(); return useQuery({ enabled: client !== undefined && indexName !== undefined, @@ -298,7 +301,35 @@ export const useGetBlockTypes = (extraFilters: Filter) => { extraFilters, 'block_types', ], - queryFn: () => fetchBlockTypes(client!, indexName!, extraFilters), + queryFn: enabled ? () => fetchBlockTypes(client!, indexName!, extraFilters) : skipToken, refetchOnMount: 'always', }); }; + +export const useGetContentHits = ( + extraFilters: Filter, + enabled: boolean = true, + attributesToRetrieve?: string[], + limit?: number, + refetchOnMount?: boolean | 'always', +) => { + const { client, indexName } = useContentSearchConnection(); + return useQuery({ + enabled: client !== undefined && indexName !== undefined, + queryKey: [ + 'content_search', + client?.config.apiKey, + client?.config.host, + indexName, + extraFilters, + ], + queryFn: enabled ? () => fetchContentHits( + client!, + indexName!, + extraFilters, + limit, + attributesToRetrieve, + ) : skipToken, + refetchOnMount, + }); +}; diff --git a/src/search-manager/index.ts b/src/search-manager/index.ts index 08c090f0a..e3e2d1a42 100644 --- a/src/search-manager/index.ts +++ b/src/search-manager/index.ts @@ -13,6 +13,7 @@ export { useContentSearchConnection, useContentSearchResults, useGetBlockTypes, + useGetContentHits, buildSearchQueryKey, } from './data/apiHooks'; export { TypesFilterData } from './hooks';