Skip to content

Commit

Permalink
fix: disable filter by type on collections tab
Browse files Browse the repository at this point in the history
Re-implements the functionality added by
openedx#1576 which was
broken when storing types in search params. Had to use a different
approach to avoid "insecure operation" errors from React.

Also adds tests.
  • Loading branch information
pomegranited committed Jan 1, 2025
1 parent 2f28e3b commit b4c70ca
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 13 deletions.
28 changes: 28 additions & 0 deletions src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,34 @@ describe('<LibraryAuthoringPage />', () => {
});
});

it('Disables Type filter on Collections tab', async () => {
await renderLibraryPage();

expect(await screen.findByText('Content library')).toBeInTheDocument();
expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument();
expect((await screen.findAllByText('Introduction to Testing'))[0]).toBeInTheDocument();
expect((await screen.findAllByText('Collection 1'))[0]).toBeInTheDocument();

// Filter by Text block type
fireEvent.click(screen.getByRole('button', { name: /type/i }));
fireEvent.click(screen.getByRole('checkbox', { name: /text/i }));
// Escape to close the Types filter drop-down and re-enable the tabs
fireEvent.keyDown(screen.getByRole('button', { name: /type/i }), { key: 'Escape' });

// Navigate to the collections tab
fireEvent.click(await screen.findByRole('tab', { name: 'Collections' }));
expect((await screen.findAllByText('Collection 1'))[0]).toBeInTheDocument();
// No Types filter shown
expect(screen.queryByRole('button', { name: /type/i })).not.toBeInTheDocument();

// Navigate to the components tab
fireEvent.click(screen.getByRole('tab', { name: 'Components' }));
// Text components should be shown
expect((await screen.findAllByText('Introduction to Testing'))[0]).toBeInTheDocument();
// Types filter is shown
expect(screen.getByRole('button', { name: /type/i })).toBeInTheDocument();
});

it('Shows an error if libraries V2 is disabled', async () => {
const { axiosMock } = initializeMocks();
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, {
Expand Down
7 changes: 6 additions & 1 deletion src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
SearchContextProvider,
SearchKeywordsField,
SearchSortWidget,
TypesFilterData,
} from '../search-manager';
import LibraryContent from './LibraryContent';
import { LibrarySidebar } from './library-sidebar';
Expand Down Expand Up @@ -220,6 +221,9 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
extraFilter.push(activeTypeFilters[activeKey]);
}

// Disable filtering by block/problem type when viewing the Collections tab.
const overrideTypesFilter = insideCollections ? new TypesFilterData() : undefined;

return (
<div className="d-flex">
<div className="flex-grow-1">
Expand All @@ -239,6 +243,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
<Container className="px-4 mt-4 mb-5 library-authoring-page">
<SearchContextProvider
extraFilter={extraFilter}
overrideTypesFilter={overrideTypesFilter}
>
<SubHeader
title={<SubHeaderTitle title={libraryData.title} />}
Expand All @@ -260,7 +265,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
<ActionRow className="my-3">
<SearchKeywordsField className="mr-3" />
<FilterByTags />
<FilterByBlockType disabled={activeKey === ContentType.collections} />
{!insideCollections && <FilterByBlockType />}
<ClearFiltersButton />
<ActionRow.Spacer />
<SearchSortWidget />
Expand Down
10 changes: 1 addition & 9 deletions src/search-manager/FilterByBlockType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,12 @@ const FilterItem = ({ blockType, count } : FilterItemProps) => {
);
};

interface FilterByBlockTypeProps {
disabled?: boolean,
}
/**
* A button with a dropdown that allows filtering the current search by component type (XBlock type)
* e.g. Limit results to "Text" (html) and "Problem" (problem) components.
* The button displays the first type selected, and a count of how many other types are selected, if more than one.
* @param disabled - If true, the filter is disabled and hidden.
*/
const FilterByBlockType: React.FC<FilterByBlockTypeProps> = ({ disabled = false }) => {
const FilterByBlockType: React.FC<Record<never, never>> = () => {
const {
blockTypes,
typesFilter,
Expand Down Expand Up @@ -248,10 +244,6 @@ const FilterByBlockType: React.FC<FilterByBlockTypeProps> = ({ disabled = false
blockType => ({ label: <BlockTypeLabel blockType={blockType} /> }),
);

if (disabled) {
return null;
}

return (
<SearchFilterWidget
appliedFilters={appliedFilters}
Expand Down
13 changes: 10 additions & 3 deletions src/search-manager/SearchManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,19 @@ export interface SearchContextData {
const SearchContext = React.createContext<SearchContextData | undefined>(undefined);

export const SearchContextProvider: React.FC<{
extraFilter?: Filter;
extraFilter?: Filter,
overrideTypesFilter?: TypesFilterData,
overrideSearchSortOrder?: SearchSortOption
children: React.ReactNode,
closeSearchModal?: () => void,
skipBlockTypeFetch?: boolean,
skipUrlUpdate?: boolean,
}> = ({
overrideSearchSortOrder, skipBlockTypeFetch, skipUrlUpdate, ...props
overrideTypesFilter,
overrideSearchSortOrder,
skipBlockTypeFetch,
skipUrlUpdate,
...props
}) => {
// Search parameters can be set via the query string
// E.g. ?q=draft+text
Expand All @@ -69,13 +74,15 @@ export const SearchContextProvider: React.FC<{

// Block + problem types use alphanumeric plus a few other characters.
// E.g ?type=html&type=video&type=p.multiplechoiceresponse
const [typesFilter, setTypesFilter] = useStateOrUrlSearchParam<TypesFilterData>(
const [internalTypesFilter, setTypesFilter] = useStateOrUrlSearchParam<TypesFilterData>(
new TypesFilterData(),
'type',
(value: string | null) => new TypesFilterData(value),
(value: TypesFilterData | undefined) => (value ? value.toString() : undefined),
skipUrlUpdate,
);
// Callers can override the types filter when searching, but we still preserve the user's selected state.
const typesFilter = overrideTypesFilter ?? internalTypesFilter;

// Tags can be almost any string value (see openedx-learning's RESERVED_TAG_CHARS)
// and multiple tags may be selected together.
Expand Down
1 change: 1 addition & 0 deletions src/search-manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ export { default as SearchSortWidget } from './SearchSortWidget';
export { default as Stats } from './Stats';
export { HIGHLIGHT_PRE_TAG, HIGHLIGHT_POST_TAG } from './data/api';
export { useGetBlockTypes } from './data/apiHooks';
export { TypesFilterData } from './hooks';

export type { CollectionHit, ContentHit, ContentHitTags } from './data/api';

0 comments on commit b4c70ca

Please sign in to comment.