fix: consider children blocks of unsupported blocks while analyzing course import (#2684)

The analysis step before importing a course considers the parent block while counting unsupported blocks but does not include children in the unsupported count.

We fetch usage_keys of all unsupported blocks and fetch the children blocks that contain these usage_keys in their breadcrumb field i.e., they are part of the unsupported blocks.

Author: navinkarkera

.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=''

.env.development

@@ -48,7 +48,7 @@ INVITE_STUDENTS_EMAIL_TO="[email protected]"
 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=''

.env.test

@@ -40,6 +40,6 @@ INVITE_STUDENTS_EMAIL_TO="[email protected]"
 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='[email protected]'

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<StudioHomeState> = {}) => {
@@ -159,7 +160,7 @@ describe('<ImportStepperModal />', () => {
     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: {

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(<ReviewImportDetails markAnalysisComplete={markAnalysisComplete} courseId="test-course-id" />);
+
+    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,

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 undefined;
+    }
+    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 = Object.entries(blockTypes).reduce((total, [blockType, count]) => {
-      const isUnsupportedBlock = getConfig().LIBRARY_UNSUPPORTED_BLOCKS.includes(blockType);
-      if (isUnsupportedBlock) {
-        return total + count;
-      }
-      return total;
-    }, 0);
+    const unsupportedBlocks = unsupportedBlockTypes.reduce((total, [, count]) => total + count, 0);
     return unsupportedBlocks;
-  }, [blockTypes]);
+  }, [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 (
     <Stack gap={4}>
@@ -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
         && (
         <>
           <h4><FormattedMessage {...messages.importCourseAnalysisDetails} /></h4>

src/search-manager/data/api.mock.ts

@@ -16,6 +16,7 @@ export async function mockContentSearchConfig(): ReturnType<typeof api.getConten
   };
 }
 mockContentSearchConfig.multisearchEndpointUrl = 'http://mock.meilisearch.local/multi-search';
+mockContentSearchConfig.searchEndpointUrl = 'http://mock.meilisearch.local/indexes/studio/search';
 mockContentSearchConfig.applyMock = () => (
   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];
+  });
+}

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<SearchResponse<Record<string, any>>> => {
+  // Convert 'extraFilter' into an array
+  const extraFilterFormatted = forceArray(extraFilter);
+
+  const results = await client.index(indexName).search('', {
+    filter: extraFilterFormatted,
+    limit,
+    attributesToRetrieve,
+  });
+
+  return results;
+};

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);
+  });
 });