Use block type label instead of Library block_types REST API [FC-0062] (#1361)

* style: avoid using reserved word "type" as variable name

use componentType or blockType instead.

* refactor: let BlockTypeLabel handle displaying the component label

including the child count, if one is provided.

This change removes hooks for the block_types REST API

* test: add tests for BlockTypeLabel

---------

Co-authored-by: Chris Chávez <xnpiochv@gmail.com>
This commit is contained in:
Jillian
2024-10-05 03:34:23 +09:30
committed by GitHub
parent 9c1fd5a68c
commit b957f3b4e3
19 changed files with 118 additions and 153 deletions

View File

@@ -14,7 +14,6 @@ import mockEmptyResult from '../search-modal/__mocks__/empty-search-result.json'
import {
mockContentLibrary,
mockGetCollectionMetadata,
mockLibraryBlockTypes,
mockXBlockFields,
} from './data/api.mocks';
import { mockContentSearchConfig } from '../search-manager/data/api.mock';
@@ -25,7 +24,6 @@ import { getLibraryCollectionsApiUrl } from './data/api';
mockGetCollectionMetadata.applyMock();
mockContentSearchConfig.applyMock();
mockContentLibrary.applyMock();
mockLibraryBlockTypes.applyMock();
mockXBlockFields.applyMock();
mockBroadcastChannel();

View File

@@ -1,4 +1,3 @@
import React, { useMemo } from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import { orderBy } from 'lodash';
@@ -7,7 +6,6 @@ import { type CollectionHit, type ContentHit, SearchSortOption } from '../search
import LibrarySection, { LIBRARY_SECTION_PREVIEW_LIMIT } from './components/LibrarySection';
import messages from './messages';
import ComponentCard from './components/ComponentCard';
import { useLibraryBlockTypes } from './data/apiHooks';
import CollectionCard from './components/CollectionCard';
import { useLibraryContext } from './common/context';
@@ -19,7 +17,6 @@ const RecentlyModified: React.FC<Record<never, never>> = () => {
totalHits,
totalCollectionHits,
} = useSearchContext();
const { libraryId } = useLibraryContext();
const componentCount = totalHits + totalCollectionHits;
// Since we only display a fixed number of items in preview,
@@ -32,17 +29,6 @@ const RecentlyModified: React.FC<Record<never, never>> = () => {
...collectionList,
], ['modified'], ['desc']).slice(0, LIBRARY_SECTION_PREVIEW_LIMIT);
const { data: blockTypesData } = useLibraryBlockTypes(libraryId);
const blockTypes = useMemo(() => {
const result = {};
if (blockTypesData) {
blockTypesData.forEach(blockType => {
result[blockType.blockType] = blockType;
});
}
return result;
}, [blockTypesData]);
return componentCount > 0
? (
<LibrarySection
@@ -60,7 +46,6 @@ const RecentlyModified: React.FC<Record<never, never>> = () => {
<ComponentCard
key={contentHit.id}
contentHit={contentHit as ContentHit}
blockTypeDisplayName={blockTypes[(contentHit as ContentHit).blockType]?.displayName ?? ''}
/>
)
))}

View File

@@ -15,7 +15,6 @@ import * as textEditorHooks from '../../editors/containers/TextEditor/hooks';
import {
mockContentLibrary,
mockCreateLibraryBlock,
mockLibraryBlockTypes,
mockXBlockFields,
} from '../data/api.mocks';
import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock';
@@ -23,7 +22,6 @@ import { mockContentSearchConfig, mockSearchResult } from '../../search-manager/
import LibraryLayout from '../LibraryLayout';
mockContentSearchConfig.applyMock();
mockLibraryBlockTypes.applyMock();
mockClipboardEmpty.applyMock();
mockBroadcastChannel();
mockContentLibrary.applyMock();

View File

@@ -84,7 +84,7 @@ const CollectionStatsWidget = ({ libraryId, collectionId }: CollectionStatsWidge
{blockTypesArray.map(({ blockType, count }) => (
<BlockCount
key={blockType}
label={<BlockTypeLabel type={blockType} />}
label={<BlockTypeLabel blockType={blockType} />}
blockType={blockType}
count={count}
/>

View File

@@ -11,7 +11,6 @@ import {
import mockResult from '../__mocks__/collection-search.json';
import {
mockContentLibrary,
mockLibraryBlockTypes,
mockXBlockFields,
mockGetCollectionMetadata,
} from '../data/api.mocks';
@@ -24,7 +23,6 @@ mockGetCollectionMetadata.applyMock();
mockContentSearchConfig.applyMock();
mockGetBlockTypes.applyMock();
mockContentLibrary.applyMock();
mockLibraryBlockTypes.applyMock();
mockXBlockFields.applyMock();
mockBroadcastChannel();

View File

@@ -8,25 +8,25 @@ import {
import { getItemIcon, getComponentStyleColor } from '../../generic/block-type-utils';
import TagCount from '../../generic/tag-count';
import { ContentHitTags, Highlight } from '../../search-manager';
import { BlockTypeLabel, ContentHitTags, Highlight } from '../../search-manager';
type BaseComponentCardProps = {
type: string,
componentType: string,
displayName: string,
description: string,
numChildren?: number,
tags: ContentHitTags,
actions: React.ReactNode,
blockTypeDisplayName: string,
openInfoSidebar: () => void
};
const BaseComponentCard = ({
type,
componentType,
displayName,
description,
numChildren,
tags,
actions,
blockTypeDisplayName,
openInfoSidebar,
} : BaseComponentCardProps) => {
const tagCount = useMemo(() => {
@@ -37,7 +37,7 @@ const BaseComponentCard = ({
+ (tags.level2?.length || 0) + (tags.level3?.length || 0);
}, [tags]);
const componentIcon = getItemIcon(type);
const componentIcon = getItemIcon(componentType);
return (
<Container className="library-component-card">
@@ -51,7 +51,7 @@ const BaseComponentCard = ({
}}
>
<Card.Header
className={`library-component-header ${getComponentStyleColor(type)}`}
className={`library-component-header ${getComponentStyleColor(componentType)}`}
title={
<Icon src={componentIcon} className="library-component-header-icon" />
}
@@ -62,7 +62,9 @@ const BaseComponentCard = ({
<Stack direction="horizontal" className="d-flex justify-content-between">
<Stack direction="horizontal" gap={1}>
<Icon src={componentIcon} size="sm" />
<span className="small">{blockTypeDisplayName}</span>
<span className="small">
<BlockTypeLabel blockType={componentType} count={numChildren} />
</span>
</Stack>
<TagCount count={tagCount} />
</Stack>

View File

@@ -44,35 +44,30 @@ type CollectionCardProps = {
};
const CollectionCard = ({ collectionHit }: CollectionCardProps) => {
const intl = useIntl();
const {
openCollectionInfoSidebar,
} = useLibraryContext();
const {
type,
type: componentType,
formatted,
tags,
numChildren,
} = collectionHit;
const { displayName = '', description = '' } = formatted;
const blockTypeDisplayName = numChildren ? intl.formatMessage(
messages.collectionTypeWithCount,
{ numChildren },
) : intl.formatMessage(messages.collectionType);
return (
<BaseComponentCard
type={type}
componentType={componentType}
displayName={displayName}
description={description}
tags={tags}
numChildren={numChildren}
actions={(
<ActionRow>
<CollectionMenu collectionHit={collectionHit} />
</ActionRow>
)}
blockTypeDisplayName={blockTypeDisplayName}
openInfoSidebar={() => openCollectionInfoSidebar(collectionHit.blockId)}
/>
);

View File

@@ -42,7 +42,7 @@ const clipboardBroadcastChannelMock = {
(global as any).BroadcastChannel = jest.fn(() => clipboardBroadcastChannelMock);
const libraryId = 'lib:org1:Demo_Course';
const render = () => baseRender(<ComponentCard contentHit={contentHit} blockTypeDisplayName="text" />, {
const render = () => baseRender(<ComponentCard contentHit={contentHit} />, {
extraWrapper: ({ children }) => <LibraryProvider libraryId={libraryId}>{ children }</LibraryProvider>,
});

View File

@@ -20,7 +20,6 @@ import BaseComponentCard from './BaseComponentCard';
type ComponentCardProps = {
contentHit: ContentHit,
blockTypeDisplayName: string,
};
export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
@@ -63,7 +62,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
);
};
const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps) => {
const ComponentCard = ({ contentHit } : ComponentCardProps) => {
const {
openComponentInfoSidebar,
} = useLibraryContext();
@@ -83,7 +82,7 @@ const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps
return (
<BaseComponentCard
type={blockType}
componentType={blockType}
displayName={displayName}
description={description}
tags={tags}
@@ -92,7 +91,6 @@ const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps
<ComponentMenu usageKey={usageKey} />
</ActionRow>
)}
blockTypeDisplayName={blockTypeDisplayName}
openInfoSidebar={() => openComponentInfoSidebar(usageKey)}
/>
);

View File

@@ -7,7 +7,7 @@ import {
initializeMocks,
} from '../../testUtils';
import { getContentSearchConfigUrl } from '../../search-manager/data/api';
import { mockLibraryBlockTypes, mockContentLibrary } from '../data/api.mocks';
import { mockContentLibrary } from '../data/api.mocks';
import mockEmptyResult from '../../search-modal/__mocks__/empty-search-result.json';
import { LibraryProvider } from '../common/context';
import { libraryComponentsMock } from '../__mocks__';
@@ -15,7 +15,6 @@ import LibraryComponents from './LibraryComponents';
const searchEndpoint = 'http://mock.meilisearch.local/multi-search';
mockLibraryBlockTypes.applyMock();
mockContentLibrary.applyMock();
const mockFetchNextPage = jest.fn();
const mockUseSearchContext = jest.fn();

View File

@@ -1,10 +1,7 @@
import React, { useMemo } from 'react';
import { LoadingSpinner } from '../../generic/Loading';
import { useLoadOnScroll } from '../../hooks';
import { useSearchContext } from '../../search-manager';
import { NoComponents, NoSearchResults } from '../EmptyStates';
import { useLibraryBlockTypes } from '../data/apiHooks';
import ComponentCard from './ComponentCard';
import { LIBRARY_SECTION_PREVIEW_LIMIT } from './LibrarySection';
import { useLibraryContext } from '../common/context';
@@ -30,22 +27,10 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => {
isLoading,
isFiltered,
} = useSearchContext();
const { libraryId, openAddContentSidebar } = useLibraryContext();
const { openAddContentSidebar } = useLibraryContext();
const componentList = variant === 'preview' ? hits.slice(0, LIBRARY_SECTION_PREVIEW_LIMIT) : hits;
// TODO get rid of "useLibraryBlockTypes". Use <BlockTypeLabel> instead.
const { data: blockTypesData } = useLibraryBlockTypes(libraryId);
const blockTypes = useMemo(() => {
const result = {};
if (blockTypesData) {
blockTypesData.forEach(blockType => {
result[blockType.blockType] = blockType;
});
}
return result;
}, [blockTypesData]);
useLoadOnScroll(
hasNextPage,
isFetchingNextPage,
@@ -67,7 +52,6 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => {
<ComponentCard
key={contentHit.id}
contentHit={contentHit}
blockTypeDisplayName={blockTypes[contentHit.blockType]?.displayName ?? ''}
/>
)) }
</div>

View File

@@ -11,16 +11,6 @@ const messages = defineMessages({
defaultMessage: 'Collection actions menu',
description: 'Alt/title text for the collection card menu button.',
},
collectionType: {
id: 'course-authoring.library-authoring.collection.type',
defaultMessage: 'Collection',
description: 'Collection type text',
},
collectionTypeWithCount: {
id: 'course-authoring.library-authoring.collection.type-with-count',
defaultMessage: 'Collection ({numChildren})',
description: 'Collection type text with children count',
},
menuOpen: {
id: 'course-authoring.library-authoring.collection.menu.open',
defaultMessage: 'Open',

View File

@@ -3,49 +3,6 @@ import { mockContentTaxonomyTagsData } from '../../content-tags-drawer/data/api.
import { createAxiosError } from '../../testUtils';
import * as api from './api';
/**
* Mock for `getLibraryBlockTypes()`
*/
export async function mockLibraryBlockTypes(): Promise<api.LibraryBlockType[]> {
return [
{ blockType: 'about', displayName: 'overview' },
{ blockType: 'annotatable', displayName: 'Annotation' },
{ blockType: 'chapter', displayName: 'Section' },
{ blockType: 'conditional', displayName: 'Conditional' },
{ blockType: 'course', displayName: 'Empty' },
{ blockType: 'course_info', displayName: 'Text' },
{ blockType: 'discussion', displayName: 'Discussion' },
{ blockType: 'done', displayName: 'Completion' },
{ blockType: 'drag-and-drop-v2', displayName: 'Drag and Drop' },
{ blockType: 'edx_sga', displayName: 'Staff Graded Assignment' },
{ blockType: 'google-calendar', displayName: 'Google Calendar' },
{ blockType: 'google-document', displayName: 'Google Document' },
{ blockType: 'html', displayName: 'Text' },
{ blockType: 'library', displayName: 'Library' },
{ blockType: 'library_content', displayName: 'Randomized Content Block' },
{ blockType: 'lti', displayName: 'LTI' },
{ blockType: 'lti_consumer', displayName: 'LTI Consumer' },
{ blockType: 'openassessment', displayName: 'Open Response Assessment' },
{ blockType: 'poll', displayName: 'Poll' },
{ blockType: 'problem', displayName: 'Problem' },
{ blockType: 'scorm', displayName: 'Scorm module' },
{ blockType: 'sequential', displayName: 'Subsection' },
{ blockType: 'split_test', displayName: 'Content Experiment' },
{ blockType: 'staffgradedxblock', displayName: 'Staff Graded Points' },
{ blockType: 'static_tab', displayName: 'Empty' },
{ blockType: 'survey', displayName: 'Survey' },
{ blockType: 'thumbs', displayName: 'Thumbs' },
{ blockType: 'unit', displayName: 'Unit' },
{ blockType: 'vertical', displayName: 'Unit' },
{ blockType: 'video', displayName: 'Video' },
{ blockType: 'videoalpha', displayName: 'Video' },
{ blockType: 'word_cloud', displayName: 'Word cloud' },
];
}
mockLibraryBlockTypes.applyMock = () => {
jest.spyOn(api, 'getLibraryBlockTypes').mockImplementation(mockLibraryBlockTypes);
};
/**
* Mock for `getContentLibrary()`
*

View File

@@ -8,11 +8,6 @@ const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL;
*/
export const getContentLibraryApiUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/`;
/**
* Get the URL for getting block types of a library (what types can be created).
*/
export const getLibraryBlockTypesUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/block_types/`;
/**
* Get the URL for create content in library.
*/
@@ -182,14 +177,6 @@ export interface CreateLibraryCollectionDataRequest {
export type UpdateCollectionComponentsRequest = Partial<CreateLibraryCollectionDataRequest>;
/**
* Fetch the list of XBlock types that can be added to this library
*/
export async function getLibraryBlockTypes(libraryId: string): Promise<LibraryBlockType[]> {
const { data } = await getAuthenticatedHttpClient().get(getLibraryBlockTypesUrl(libraryId));
return camelCaseObject(data);
}
/**
* Fetch a content library by its ID.
*/

View File

@@ -14,7 +14,6 @@ import {
type XBlockFields,
type UpdateXBlockFieldsRequest,
getContentLibrary,
getLibraryBlockTypes,
createLibraryBlock,
getContentLibraryV2List,
commitLibraryChanges,
@@ -59,12 +58,6 @@ export const libraryAuthoringQueryKeys = {
'list',
...(customParams ? [customParams] : []),
],
contentLibraryBlockTypes: (contentLibraryId?: string) => [
...libraryAuthoringQueryKeys.all,
...libraryAuthoringQueryKeys.contentLibrary(contentLibraryId),
'content',
'libraryBlockTypes',
],
collection: (libraryId?: string, collectionId?: string) => [
...libraryAuthoringQueryKeys.all,
libraryId,
@@ -113,16 +106,6 @@ export const useContentLibrary = (libraryId: string | undefined) => (
})
);
/**
* Hook to fetch block types of a library.
*/
export const useLibraryBlockTypes = (libraryId: string) => (
useQuery({
queryKey: libraryAuthoringQueryKeys.contentLibraryBlockTypes(libraryId),
queryFn: () => getLibraryBlockTypes(libraryId),
})
);
/**
* Use this mutation to create a block in a library
*/

View File

@@ -0,0 +1,73 @@
import { render, screen } from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import BlockTypeLabel from './BlockTypeLabel';
import messages from './messages';
const testCases = [
{
blockType: 'annotatable',
count: undefined,
expectedLabel: messages['blockType.annotatable'].defaultMessage,
},
{
blockType: 'chapter',
count: undefined,
expectedLabel: messages['blockType.chapter'].defaultMessage,
},
{
blockType: 'chapter',
count: 10,
expectedLabel: messages['blockType.chapter'].defaultMessage,
},
{
blockType: 'drag-and-drop-v2',
count: undefined,
expectedLabel: messages['blockType.drag-and-drop-v2'].defaultMessage,
},
{
blockType: 'multiplechoiceresponse',
count: undefined,
expectedLabel: messages['blockType.multiplechoiceresponse'].defaultMessage,
},
{
blockType: 'html',
count: undefined,
expectedLabel: messages['blockType.html'].defaultMessage,
},
{
blockType: 'collection',
count: undefined,
expectedLabel: messages['blockType.collection'].defaultMessage,
},
{
blockType: 'collection',
count: 0,
expectedLabel: messages['blockType.collection'].defaultMessage,
},
{
blockType: 'collection',
count: 10,
expectedLabel: 'Collection (10)',
},
// XBlock types without an explicit label are capitalized using the textTransform style
{
blockType: 'survey',
count: undefined,
expectedLabel: 'survey',
},
];
describe('<BlockTypeLabel />', () => {
test.each(testCases)(
'render BlockTypeLabel for $blockType (count=$count)',
({ blockType, expectedLabel, count }) => {
render(
<IntlProvider locale="en">
<BlockTypeLabel blockType={blockType} count={count} />
</IntlProvider>,
);
expect(screen.getByText(expectedLabel)).toBeInTheDocument();
},
);
});

View File

@@ -5,18 +5,26 @@ import messages from './messages';
/**
* Displays a friendly, localized text name for the given XBlock/component type
* e.g. `vertical` becomes `"Unit"`
*
* Also accepts an optional `count` number, which will be displayed if
* it's non-zero and the block label supports it.
*/
const BlockTypeLabel: React.FC<{ type: string }> = ({ type }) => {
// TODO: Load the localized list of Component names from Studio REST API?
const msg = messages[`blockType.${type}`];
const BlockTypeLabel: React.FC<{ blockType: string, count?: number }> = ({ blockType, count }) => {
const msg = messages[`blockType.${blockType}`];
const msgWithCount = messages[`blockType.${blockType}.with_count`];
if (count && msgWithCount) {
return <FormattedMessage values={{ count }} {...msgWithCount} />;
}
if (msg) {
return <FormattedMessage {...msg} />;
}
// Replace underscores and hypens with spaces, then let the browser capitalize this
// in a locale-aware way to get a reasonable display value.
// e.g. 'drag-and-drop-v2' -> "Drag And Drop V2"
return <span style={{ textTransform: 'capitalize' }}>{type.replace(/[_-]/g, ' ')}</span>;
return <span style={{ textTransform: 'capitalize' }}>{blockType.replace(/[_-]/g, ' ')}</span>;
};
export default BlockTypeLabel;

View File

@@ -108,7 +108,7 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr
>
<div className="d-flex justify-content-between align-items-center">
<div>
<BlockTypeLabel type={blockType} />{' '}
<BlockTypeLabel blockType={blockType} />{' '}
<Badge variant="light" pill>{count}</Badge>
</div>
{ Object.keys(problemTypes).length !== 0 && (
@@ -146,7 +146,7 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr
onChange={handleProblemCheckboxChange}
>
<div style={{ textAlign: 'start' }}>
<BlockTypeLabel type={problemType} />{' '}
<BlockTypeLabel blockType={problemType} />{' '}
<Badge variant="light" pill>{problemTypeCount}</Badge>
</div>
</MenuItem>
@@ -199,7 +199,7 @@ const FilterItem = ({ blockType, count } : FilterItemProps) => {
onChange={handleCheckboxChange}
>
<div>
<BlockTypeLabel type={blockType} />{' '}
<BlockTypeLabel blockType={blockType} />{' '}
<Badge variant="light" pill>{count}</Badge>
</div>
</MenuItem>
@@ -259,7 +259,7 @@ const FilterByBlockType: React.FC<Record<never, never>> = () => {
});
const appliedFilters = [...blockTypesFilter, ...problemTypesFilter].map(
blockType => ({ label: <BlockTypeLabel type={blockType} /> }),
blockType => ({ label: <BlockTypeLabel blockType={blockType} /> }),
);
return (

View File

@@ -56,6 +56,16 @@ const messages = defineMessages({
defaultMessage: 'Section',
description: 'Name of the "Section" course outline level in Studio',
},
'blockType.collection': {
id: 'course-authoring.course-search.blockType.collection',
defaultMessage: 'Collection',
description: 'Collection type text',
},
'blockType.collection.with_count': {
id: 'course-authoring.course-search.blockType.collectionWithCount',
defaultMessage: 'Collection ({count})',
description: 'Collection type text with children count',
},
'blockType.discussion': {
id: 'course-authoring.course-search.blockType.discussion',
defaultMessage: 'Discussion',