From 64906a1b9d21f7786b3a62295ceeb123da946f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Wed, 18 Dec 2024 12:50:09 -0500 Subject: [PATCH 01/18] refactor: Update error message while adding library team member [FC-0062] (#1572) Update the error message while adding a library team member to show the error message generated by the server --- .../library-team/LibraryTeam.test.tsx | 31 +++++++++++++++++-- .../library-team/LibraryTeam.tsx | 11 ++++++- .../library-team/messages.ts | 8 ++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/library-authoring/library-team/LibraryTeam.test.tsx b/src/library-authoring/library-team/LibraryTeam.test.tsx index 8e43d0f42..05f8d1ff5 100644 --- a/src/library-authoring/library-team/LibraryTeam.test.tsx +++ b/src/library-authoring/library-team/LibraryTeam.test.tsx @@ -183,10 +183,10 @@ describe('', () => { expect(await screen.findByText('Team Member added')).toBeInTheDocument(); }); - it('shows error when user do not exist', async () => { + it('shows error when specific error (string)', async () => { const url = getLibraryTeamApiUrl(libraryId); const axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - axiosMock.onPost(url).reply(400, { email: 'Error' }); + axiosMock.onPost(url).reply(400, { email: 'This is a specific error.' }); await renderLibraryTeam(); @@ -204,7 +204,32 @@ describe('', () => { }); expect(await screen.findByText( - 'Error adding Team Member. Please verify that the email is correct and belongs to a registered user.', + 'Error adding Team Member. This is a specific error.', + )).toBeInTheDocument(); + }); + + it('shows error when specific error (Array)', async () => { + const url = getLibraryTeamApiUrl(libraryId); + const axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + axiosMock.onPost(url).reply(400, { email: ['This is a specific error.'] }); + + await renderLibraryTeam(); + + const addButton = screen.getByRole('button', { name: 'New team member' }); + userEvent.click(addButton); + const emailInput = screen.getByRole('textbox', { name: 'User\'s email address' }); + userEvent.click(emailInput); + userEvent.type(emailInput, 'another@user.tld'); + + const saveButton = screen.getByRole('button', { name: /add member/i }); + userEvent.click(saveButton); + + await waitFor(() => { + expect(axiosMock.history.post.length).toEqual(1); + }); + + expect(await screen.findByText( + 'Error adding Team Member. This is a specific error.', )).toBeInTheDocument(); }); diff --git a/src/library-authoring/library-team/LibraryTeam.tsx b/src/library-authoring/library-team/LibraryTeam.tsx index 881ce2782..d7f14cbc8 100644 --- a/src/library-authoring/library-team/LibraryTeam.tsx +++ b/src/library-authoring/library-team/LibraryTeam.tsx @@ -68,7 +68,16 @@ const LibraryTeam: React.FC> = () => { }).catch((addMemberError) => { const errorData = typeof addMemberError === 'object' ? addMemberError.response?.data : undefined; if (errorData && 'email' in errorData) { - showToast(intl.formatMessage(messages.addMemberEmailError)); + const errorEmail = errorData.email; + if (typeof errorEmail === 'string') { + showToast(intl.formatMessage(messages.addMemberSpecificError, { + message: errorEmail, + })); + } else { + showToast(intl.formatMessage(messages.addMemberSpecificError, { + message: errorEmail[0], + })); + } } else { showToast(intl.formatMessage(messages.addMemberError)); } diff --git a/src/library-authoring/library-team/messages.ts b/src/library-authoring/library-team/messages.ts index d56d60615..32623b237 100644 --- a/src/library-authoring/library-team/messages.ts +++ b/src/library-authoring/library-team/messages.ts @@ -124,10 +124,10 @@ const messages = defineMessages({ defaultMessage: 'Error adding Team Member', description: 'Message shown when an error occurs while adding a Library Team member', }, - addMemberEmailError: { - id: 'course-authoring.library-authoring.library-team.add-member-email-error', - defaultMessage: 'Error adding Team Member. Please verify that the email is correct and belongs to a registered user.', - description: 'Message shown when an error occurs with email while adding a Library Team member.', + addMemberSpecificError: { + id: 'course-authoring.library-authoring.library-team.add-member-specific-error', + defaultMessage: 'Error adding Team Member. {message}', + description: 'Message shown when an error occurs while adding a Library Team member, including a specific error message.', }, deleteMemberSuccess: { id: 'course-authoring.library-authoring.library-team.delete-member-success', From 230960b711119aa911750f9ff8a1a5527f0e7dd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 18 Dec 2024 14:56:21 -0300 Subject: [PATCH 02/18] refactor: improve library sub header (#1573) Changes the Library (and Collection) subheader --- src/hooks.ts | 6 +- .../LibraryAuthoringPage.tsx | 39 +++--- .../LibraryCollectionPage.test.tsx | 15 ++- .../collections/LibraryCollectionPage.tsx | 121 +++++++++--------- src/library-authoring/collections/messages.ts | 10 -- src/search-manager/FilterByBlockType.tsx | 8 +- 6 files changed, 94 insertions(+), 105 deletions(-) diff --git a/src/hooks.ts b/src/hooks.ts index 1984625cb..d9cde4a80 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -50,11 +50,11 @@ export const useLoadOnScroll = ( useEffect(() => { if (enabled) { const canFetchNextPage = hasNextPage && !isFetchingNextPage; + // Used `loadLimit` to fetch next page before reach the end of the screen. + const loadLimit = 300; const onscroll = () => { // Verify the position of the scroll to implement an infinite scroll. - // Used `loadLimit` to fetch next page before reach the end of the screen. - const loadLimit = 300; const scrolledTo = window.scrollY + window.innerHeight; const scrollDiff = document.body.scrollHeight - scrolledTo; const isNearToBottom = scrollDiff <= loadLimit; @@ -65,7 +65,7 @@ export const useLoadOnScroll = ( window.addEventListener('scroll', onscroll); // If the content is less than the screen height, fetch the next page. - const hasNoScroll = document.body.scrollHeight <= window.innerHeight; + const hasNoScroll = (document.body.scrollHeight - loadLimit) <= window.innerHeight; if (hasNoScroll && canFetchNextPage) { fetchNextPage(); } diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 01fc146b6..46dd1cd49 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -4,6 +4,7 @@ import classNames from 'classnames'; import { StudioFooter } from '@edx/frontend-component-footer'; import { useIntl } from '@edx/frontend-platform/i18n'; import { + ActionRow, Alert, Badge, Breadcrumb, @@ -57,12 +58,10 @@ const HeaderActions = () => { const { componentPickerMode } = useComponentPickerContext(); - const infoSidebarIsOpen = () => ( - sidebarComponentInfo?.type === SidebarBodyComponentId.Info - ); + const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.Info; const handleOnClickInfoSidebar = () => { - if (infoSidebarIsOpen()) { + if (infoSidebarIsOpen) { closeLibrarySidebar(); } else { openInfoSidebar(); @@ -73,8 +72,8 @@ const HeaderActions = () => {
-
- ); -}; - -const SubHeaderTitle = ({ - title, - infoClickHandler, -}: { - title: string; - infoClickHandler: () => void; -}) => { - const intl = useIntl(); - - const { componentPickerMode } = useComponentPickerContext(); - const { readOnly } = useLibraryContext(); - - const showReadOnlyBadge = readOnly && !componentPickerMode; - - return ( - - - {title} - - - {showReadOnlyBadge && ( -
- - {intl.formatMessage(messages.readOnlyBadge)} - -
+ {!componentPickerMode && ( + )} - + ); }; @@ -181,6 +178,7 @@ const LibraryCollectionPage = () => { return (
+ {libraryData.title} | {process.env.SITE_NAME} {!componentPickerMode && (
{ org={libraryData.org} contextId={libraryId} isLibrary + containerProps={{ + size: undefined, + }} /> )} - + openCollectionInfoSidebar(collectionId)} - /> - )} + title={} breadcrumbs={breadcumbs} headerActions={} + hideBorder /> - -
+ + -
+ -
+ - + {!componentPickerMode && }
{!!sidebarComponentInfo?.type && (
diff --git a/src/library-authoring/collections/messages.ts b/src/library-authoring/collections/messages.ts index 1dfaecd4a..ab8cbbc21 100644 --- a/src/library-authoring/collections/messages.ts +++ b/src/library-authoring/collections/messages.ts @@ -76,11 +76,6 @@ const messages = defineMessages({ 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', @@ -91,11 +86,6 @@ const messages = defineMessages({ 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.', diff --git a/src/search-manager/FilterByBlockType.tsx b/src/search-manager/FilterByBlockType.tsx index cd887e3cc..98410be18 100644 --- a/src/search-manager/FilterByBlockType.tsx +++ b/src/search-manager/FilterByBlockType.tsx @@ -262,6 +262,8 @@ const FilterByBlockType: React.FC> = () => { blockType => ({ label: }), ); + const hiddenBlockTypes = blockTypesFilter.filter(blockType => !Object.keys(blockTypes).includes(blockType)); + return ( > = () => { value={blockTypesFilter} > + { + // Show applied filter items for block types that are not in the current search results + hiddenBlockTypes.map(blockType => ) + } { Object.entries(sortedBlockTypes).map(([blockType, count]) => ( @@ -282,7 +288,7 @@ const FilterByBlockType: React.FC> = () => { } { // Show a message if there are no options at all to avoid the impression that the dropdown isn't working - Object.keys(sortedBlockTypes).length === 0 ? ( + Object.keys(sortedBlockTypes).length === 0 && hiddenBlockTypes.length === 0 ? ( ) : null } From dc0ba6aac4bc1218885d45fe1fe12d6f956fbe15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Sat, 21 Dec 2024 09:01:22 -0300 Subject: [PATCH 03/18] fix: disable filter by block on collections tab [FC-0076] (#1576) Adds the disabled prop for `FilterByBlockType` component and uses it on the Library Collection tab. --- .../LibraryAuthoringPage.tsx | 2 +- src/search-manager/FilterByBlockType.tsx | 34 ++++++++++++++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 46dd1cd49..88b92eb1a 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -258,7 +258,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage - + diff --git a/src/search-manager/FilterByBlockType.tsx b/src/search-manager/FilterByBlockType.tsx index 98410be18..95c8ee756 100644 --- a/src/search-manager/FilterByBlockType.tsx +++ b/src/search-manager/FilterByBlockType.tsx @@ -206,12 +206,16 @@ 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> = () => { +const FilterByBlockType: React.FC = ({ disabled = false }) => { const { blockTypes, blockTypesFilter, @@ -225,6 +229,22 @@ const FilterByBlockType: React.FC> = () => { setProblemTypesFilter([]); }, []); + useEffect(() => { + if (disabled) { + // Clear filters when disabled + const selectedBlockTypesFilter = blockTypesFilter; + const selectedProblemTypesFilter = problemTypesFilter; + clearFilters(); + + return () => { + // Restore filters when re-enabled + setBlockTypesFilter(selectedBlockTypesFilter); + setProblemTypesFilter(selectedProblemTypesFilter); + }; + } + return () => {}; + }, [disabled]); + // Sort blocktypes in order of hierarchy followed by alphabetically for components const sortedBlockTypeKeys = Object.keys(blockTypes).sort((a, b) => { const order = { @@ -262,7 +282,9 @@ const FilterByBlockType: React.FC> = () => { blockType => ({ label: }), ); - const hiddenBlockTypes = blockTypesFilter.filter(blockType => !Object.keys(blockTypes).includes(blockType)); + if (disabled) { + return null; + } return ( > = () => { value={blockTypesFilter} > - { - // Show applied filter items for block types that are not in the current search results - hiddenBlockTypes.map(blockType => ) - } { Object.entries(sortedBlockTypes).map(([blockType, count]) => ( @@ -288,9 +306,9 @@ const FilterByBlockType: React.FC> = () => { } { // Show a message if there are no options at all to avoid the impression that the dropdown isn't working - Object.keys(sortedBlockTypes).length === 0 && hiddenBlockTypes.length === 0 ? ( + Object.keys(sortedBlockTypes).length === 0 && ( - ) : null + ) } From f586b095fa9a298bceb0da627e35dfc8431a18e6 Mon Sep 17 00:00:00 2001 From: edX requirements bot <49161187+edx-requirements-bot@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:38:02 -0500 Subject: [PATCH 04/18] chore: update browserslist DB (#1494) Co-authored-by: abdullahwaheed <42172960+abdullahwaheed@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6689a22ce..24bd8a4ac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6644,9 +6644,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001677", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001677.tgz", - "integrity": "sha512-fmfjsOlJUpMWu+mAAtZZZHz7UEwsUxIIvu1TJfO1HqFQvB/B+ii0xr9B5HpbZY/mC4XZ8SvjHJqtAY6pDPQEog==", + "version": "1.0.30001690", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001690.tgz", + "integrity": "sha512-5ExiE3qQN6oF8Clf8ifIDcMRCRE/dMGcETG/XGMD8/XiXm6HXQgQTh1yZYLXXpSOsEUlJm1Xr7kGULZTuGtP/w==", "funding": [ { "type": "opencollective", From 811be226d16d0197e97a8b96f2e83081e3a918d7 Mon Sep 17 00:00:00 2001 From: Jillian Date: Sat, 11 Jan 2025 02:06:46 +1030 Subject: [PATCH 05/18] feat: shareable URLs for library components and searches [FC-0076] (#1575) Adds new routes and URL parameters to use when viewing and performing searches on library components. These changes allow these pages to be bookmarked or shared by copy/pasting the browser's current URL. No changes were made to the UI. Use cases covered: * As an author working with content libraries, I want to easily share any component in a library with other people on my team, by copying the URL from my browser and sending it to them. * As an author working with content libraries, I want to easily share any search results with other people on my team, by copying the URL from my browser and sending it to them. * As an author working with content libraries, I want to bookmark a search in my browser and return to it at any time, with the same filters and keywords applied. * As an author of a content library with public read access, I want to easily share any component in a library with any authors on the same Open edX instance, by copying the URL from my browser and sending it to them. * As an author of a content library, I want to easily share a library's "Manage Team" page with other people on my team by copying the URL from my browser and sending it to them. * As an author working with content libraries, I want to easily share any selected sidebar tab with other people on my team, by copying the URL from my browser and sending it to them. --- .../ContentTagsDrawer.test.jsx | 2 +- src/hooks.ts | 117 ++++++- .../LibraryAuthoringPage.test.tsx | 65 ++-- .../LibraryAuthoringPage.tsx | 69 +++-- src/library-authoring/LibraryContent.tsx | 7 +- src/library-authoring/LibraryLayout.tsx | 60 ++-- .../add-content/AddContentContainer.test.tsx | 8 +- .../PickLibraryContentModal.test.tsx | 1 - .../collections/CollectionInfo.tsx | 35 +-- .../LibraryCollectionComponents.tsx | 3 +- .../collections/LibraryCollectionPage.tsx | 14 +- .../common/context/LibraryContext.tsx | 26 +- .../common/context/SidebarContext.tsx | 108 ++++--- .../component-info/ComponentInfo.tsx | 30 +- .../component-info/ComponentManagement.tsx | 8 +- .../component-info/ManageCollections.test.tsx | 5 +- .../component-info/ManageCollections.tsx | 47 +-- .../component-picker/ComponentPicker.tsx | 1 + .../components/BaseComponentCard.tsx | 8 +- .../components/CollectionCard.tsx | 13 +- .../components/ComponentCard.tsx | 25 +- .../library-info/LibraryInfo.tsx | 13 +- .../library-info/LibraryInfoHeader.tsx | 4 +- src/library-authoring/routes.test.tsx | 289 ++++++++++++++++++ src/library-authoring/routes.ts | 128 ++++++++ src/search-manager/FilterByBlockType.tsx | 140 +++------ src/search-manager/SearchManager.ts | 143 ++++----- src/search-manager/hooks.ts | 133 ++++++++ src/search-manager/index.ts | 1 + src/search-modal/SearchUI.test.tsx | 24 +- src/testUtils.tsx | 6 +- 31 files changed, 1140 insertions(+), 393 deletions(-) create mode 100644 src/library-authoring/routes.test.tsx create mode 100644 src/library-authoring/routes.ts create mode 100644 src/search-manager/hooks.ts diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 426f0d532..0ff99eaf2 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -18,7 +18,7 @@ import { } from './data/api.mocks'; import { getContentTaxonomyTagsApiUrl } from './data/api'; -const path = '/content/:contentId/*'; +const path = '/content/:contentId?/*'; const mockOnClose = jest.fn(); const mockSetBlockingSheet = jest.fn(); const mockNavigate = jest.fn(); diff --git a/src/hooks.ts b/src/hooks.ts index d9cde4a80..d121ddda8 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,6 +1,13 @@ -import { useEffect, useState } from 'react'; +import { + type Dispatch, + type SetStateAction, + useCallback, + useEffect, + useRef, + useState, +} from 'react'; import { history } from '@edx/frontend-platform'; -import { useLocation } from 'react-router-dom'; +import { useLocation, useSearchParams } from 'react-router-dom'; export const useScrollToHashElement = ({ isLoading }: { isLoading: boolean }) => { const [elementWithHash, setElementWithHash] = useState(null); @@ -77,3 +84,109 @@ export const useLoadOnScroll = ( return () => { }; }, [hasNextPage, isFetchingNextPage, fetchNextPage]); }; + +/** + * Types used by the useStateWithUrlSearchParam hook. + */ +export type FromStringFn = (value: string | null) => Type | undefined; +export type ToStringFn = (value: Type | undefined) => string | undefined; + +/** + * Hook that stores/retrieves state variables using the URL search parameters. + * This function is overloaded to accept simple Types or Array values. + * + * @param defaultValue: Type | Type[] + * Returned when no valid value is found in the url search parameter. + * If an Array Type is used, then an Array Type of state values will be maintained. + * @param paramName: name of the url search parameter to store this value in. + * @param fromString: returns the Type equivalent of the given string value, + * or undefined if the value is invalid. + * @param toString: returns the string equivalent of the given Type value. + * Return defaultValue to clear the url search paramName. + */ +export function useStateWithUrlSearchParam( + defaultValue: Type[], + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, +): [value: Type[], setter: Dispatch>]; +export function useStateWithUrlSearchParam( + defaultValue: Type, + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, +): [value: Type, setter: Dispatch>]; +export function useStateWithUrlSearchParam( + defaultValue: Type | Type[], + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, +): [value: Type | Type[], setter: Dispatch>] { + // STATE WORKAROUND: + // If we use this hook to control multiple state parameters on the same + // page, we can run into state update issues. Because our state variables + // are actually stored in setSearchParams, and not in separate variables like + // useState would do, the searchParams "previous" state may not be updated + // for sequential calls to returnSetter in the same render loop (as we do in + // SearchManager's clearFilters). + // + // One workaround could be to use window.location.search as the "previous" + // value when returnSetter constructs the new URLSearchParams. This works + // fine with BrowserRouter, however our test suite uses MemoryRouter, and + // that router doesn't store URL search params, cf + // https://github.com/remix-run/react-router/issues/9757 + // + // So instead, we maintain a reference to the current useLocation() + // object, and use its search params as the "previous" value when + // initializing URLSearchParams. + const location = useLocation(); + const locationRef = useRef(location); + const [searchParams, setSearchParams] = useSearchParams(); + const paramValues = searchParams.getAll(paramName); + + const returnValue: Type | Type[] = ( + defaultValue instanceof Array + ? (paramValues.map(fromString).filter((v) => v !== undefined)) as Type[] + : fromString(paramValues[0]) + ) ?? defaultValue; + + // Update the url search parameter using: + type ReturnSetterParams = ( + // a Type value + value?: Type | Type[] + // or a function that returns a Type from the previous returnValue + | ((value: Type | Type[]) => Type | Type[]) + ) => void; + const returnSetter: Dispatch> = useCallback((value) => { + setSearchParams((/* previous -- see STATE WORKAROUND above */) => { + const useValue = value instanceof Function ? value(returnValue) : value; + const paramValue: string | string[] | undefined = ( + useValue instanceof Array + ? useValue.map(toString).filter((v) => v !== undefined) as string[] + : toString(useValue) + ); + + const newSearchParams = new URLSearchParams(locationRef.current.search); + if (paramValue === undefined || paramValue === defaultValue) { + // If the provided value was invalid (toString returned undefined) or + // the same as the defaultValue, remove it from the search params. + newSearchParams.delete(paramName); + } else if (paramValue instanceof Array) { + // Replace paramName with the new list of values. + newSearchParams.delete(paramName); + paramValue.forEach((v) => v && newSearchParams.append(paramName, v)); + } else { + // Otherwise, just set the new (single) value. + newSearchParams.set(paramName, paramValue); + } + + // Update locationRef + locationRef.current.search = newSearchParams.toString(); + + return newSearchParams; + }, { replace: true }); + }, [returnValue, setSearchParams]); + + // Return the computed value and wrapped set state function + return [returnValue, returnSetter]; +} diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 0c725fbf2..2b55375fd 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -137,7 +137,7 @@ describe('', () => { expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); fireEvent.click(screen.getByRole('tab', { name: 'Collections' })); - expect(screen.getByText('You have not added any collections to this library yet.')).toBeInTheDocument(); + expect(await screen.findByText('You have not added any collections to this library yet.')).toBeInTheDocument(); // Open Create collection modal const addCollectionButton = screen.getByRole('button', { name: /add collection/i }); @@ -151,7 +151,7 @@ describe('', () => { expect(collectionModalHeading).not.toBeInTheDocument(); fireEvent.click(screen.getByRole('tab', { name: 'All Content' })); - expect(screen.getByText('You have not added any content to this library yet.')).toBeInTheDocument(); + expect(await screen.findByText('You have not added any content to this library yet.')).toBeInTheDocument(); const addComponentButton = screen.getByRole('button', { name: /add component/i }); fireEvent.click(addComponentButton); @@ -196,15 +196,15 @@ 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 library.')).toBeInTheDocument(); + expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument(); // Navigate to the components tab fireEvent.click(screen.getByRole('tab', { name: 'Components' })); - expect(screen.getByText('No matching components found in this library.')).toBeInTheDocument(); + expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument(); // Navigate to the collections tab fireEvent.click(screen.getByRole('tab', { name: 'Collections' })); - expect(screen.getByText('No matching collections found in this library.')).toBeInTheDocument(); + expect(await screen.findByText('No matching collections found in this library.')).toBeInTheDocument(); // Go back to Home tab // This step is necessary to avoid the url change leak to other tests @@ -431,27 +431,23 @@ describe('', () => { // Click on the first collection fireEvent.click((await screen.findByText('Collection 1'))); - const sidebar = screen.getByTestId('library-sidebar'); - - const { getByRole } = within(sidebar); - // Click on the Details tab - fireEvent.click(getByRole('tab', { name: 'Details' })); + fireEvent.click(screen.getByRole('tab', { name: 'Details' })); // Change to a component fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]); // Check that the Details tab is still selected - expect(getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true'); // Click on the Previews tab - fireEvent.click(getByRole('tab', { name: 'Preview' })); + fireEvent.click(screen.getByRole('tab', { name: 'Preview' })); // Switch back to the collection fireEvent.click((await screen.findByText('Collection 1'))); // The Manage (default) tab should be selected because the collection does not have a Preview tab - expect(getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true'); }); it('can filter by capa problem type', async () => { @@ -505,6 +501,15 @@ describe('', () => { // eslint-disable-next-line no-await-in-loop await validateSubmenu(key); } + }); + + it('can filter by block type', async () => { + await renderLibraryPage(); + + // Ensure the search endpoint is called + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + const filterButton = screen.getByRole('button', { name: /type/i }); + fireEvent.click(filterButton); // Validate click on Problem type const problemMenu = screen.getAllByText('Problem')[0]; @@ -528,15 +533,13 @@ describe('', () => { }); // Validate clear filters - const submenu = screen.getByText('Checkboxes'); - expect(submenu).toBeInTheDocument(); - fireEvent.click(submenu); + fireEvent.click(problemMenu); const clearFitlersButton = screen.getByRole('button', { name: /clear filters/i }); fireEvent.click(clearFitlersButton); await waitFor(() => { expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, { - body: expect.not.stringContaining(`content.problem_types = ${problemTypes.Checkboxes}`), + body: expect.not.stringContaining('block_type = problem'), method: 'POST', headers: expect.anything(), }); @@ -706,6 +709,34 @@ describe('', () => { }); }); + 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, { diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 88b92eb1a..b952310f5 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { Helmet } from 'react-helmet'; import classNames from 'classnames'; import { StudioFooter } from '@edx/frontend-component-footer'; @@ -16,12 +16,7 @@ import { Tabs, } from '@openedx/paragon'; import { Add, ArrowBack, InfoOutline } from '@openedx/paragon/icons'; -import { - Link, - useLocation, - useNavigate, - useSearchParams, -} from 'react-router-dom'; +import { Link } from 'react-router-dom'; import Loading from '../generic/Loading'; import SubHeader from '../generic/sub-header/SubHeader'; @@ -35,12 +30,14 @@ import { SearchContextProvider, SearchKeywordsField, SearchSortWidget, + TypesFilterData, } from '../search-manager'; -import LibraryContent, { ContentType } from './LibraryContent'; +import LibraryContent from './LibraryContent'; import { LibrarySidebar } from './library-sidebar'; import { useComponentPickerContext } from './common/context/ComponentPickerContext'; import { useLibraryContext } from './common/context/LibraryContext'; import { SidebarBodyComponentId, useSidebarContext } from './common/context/SidebarContext'; +import { ContentType, useLibraryRoutes } from './routes'; import messages from './messages'; @@ -51,7 +48,7 @@ const HeaderActions = () => { const { openAddContentSidebar, - openInfoSidebar, + openLibrarySidebar, closeLibrarySidebar, sidebarComponentInfo, } = useSidebarContext(); @@ -60,13 +57,19 @@ const HeaderActions = () => { const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.Info; - const handleOnClickInfoSidebar = () => { + const { navigateTo } = useLibraryRoutes(); + const handleOnClickInfoSidebar = useCallback(() => { if (infoSidebarIsOpen) { closeLibrarySidebar(); } else { - openInfoSidebar(); + openLibrarySidebar(); } - }; + + if (!componentPickerMode) { + // Reset URL to library home + navigateTo({ componentId: '', collectionId: '' }); + } + }, [navigateTo, sidebarComponentInfo, closeLibrarySidebar, openLibrarySidebar]); return (
@@ -124,8 +127,6 @@ interface LibraryAuthoringPageProps { const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPageProps) => { const intl = useIntl(); - const location = useLocation(); - const navigate = useNavigate(); const { isLoadingPage: isLoadingStudioHome, @@ -139,29 +140,34 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage libraryData, isLoadingLibraryData, showOnlyPublished, + componentId, + collectionId, } = useLibraryContext(); const { openInfoSidebar, sidebarComponentInfo } = useSidebarContext(); - const [activeKey, setActiveKey] = useState(ContentType.home); - - useEffect(() => { - const currentPath = location.pathname.split('/').pop(); + const { insideCollections, insideComponents, navigateTo } = useLibraryRoutes(); - if (componentPickerMode || currentPath === libraryId || currentPath === '') { - setActiveKey(ContentType.home); - } else if (currentPath && currentPath in ContentType) { - setActiveKey(ContentType[currentPath] || ContentType.home); + // The activeKey determines the currently selected tab. + const getActiveKey = () => { + if (componentPickerMode) { + return ContentType.home; } - }, []); + if (insideCollections) { + return ContentType.collections; + } + if (insideComponents) { + return ContentType.components; + } + return ContentType.home; + }; + const [activeKey, setActiveKey] = useState(getActiveKey); useEffect(() => { if (!componentPickerMode) { - openInfoSidebar(); + openInfoSidebar(componentId, collectionId); } }, []); - const [searchParams] = useSearchParams(); - if (isLoadingLibraryData) { return ; } @@ -181,10 +187,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage const handleTabChange = (key: ContentType) => { setActiveKey(key); if (!componentPickerMode) { - navigate({ - pathname: key, - search: searchParams.toString(), - }); + navigateTo({ contentType: key }); } }; @@ -218,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 (
@@ -237,6 +243,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage } @@ -258,7 +265,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage - + {!insideCollections && } diff --git a/src/library-authoring/LibraryContent.tsx b/src/library-authoring/LibraryContent.tsx index 5eb050520..1913994b2 100644 --- a/src/library-authoring/LibraryContent.tsx +++ b/src/library-authoring/LibraryContent.tsx @@ -6,15 +6,10 @@ import { useLibraryContext } from './common/context/LibraryContext'; import { useSidebarContext } from './common/context/SidebarContext'; import CollectionCard from './components/CollectionCard'; import ComponentCard from './components/ComponentCard'; +import { ContentType } from './routes'; import { useLoadOnScroll } from '../hooks'; import messages from './collections/messages'; -export enum ContentType { - home = '', - components = 'components', - collections = 'collections', -} - /** * Library Content to show content grid * diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 610a28eac..c093af7ad 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -1,10 +1,12 @@ +import { useCallback } from 'react'; import { Route, Routes, useParams, - useMatch, + useLocation, } from 'react-router-dom'; +import { ROUTES } from './routes'; import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context/LibraryContext'; import { SidebarProvider } from './common/context/SidebarContext'; @@ -16,22 +18,18 @@ import { ComponentEditorModal } from './components/ComponentEditorModal'; const LibraryLayout = () => { const { libraryId } = useParams(); - const match = useMatch('/library/:libraryId/collection/:collectionId'); - - const collectionId = match?.params.collectionId; - if (libraryId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. throw new Error('Error: route is missing libraryId.'); } - return ( + const location = useLocation(); + const context = useCallback((childPage) => ( LibraryAuthoringPage/LibraryCollectionPage > @@ -39,20 +37,38 @@ const LibraryLayout = () => { componentPicker={ComponentPicker} > - - } - /> - } - /> - - - + <> + {childPage} + + + + ), [location.pathname]); + + return ( + + )} + /> + )} + /> + )} + /> + )} + /> + )} + /> + ); }; diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 2f233629c..229948c39 100644 --- a/src/library-authoring/add-content/AddContentContainer.test.tsx +++ b/src/library-authoring/add-content/AddContentContainer.test.tsx @@ -25,17 +25,13 @@ jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCs const { libraryId } = mockContentLibrary; const render = (collectionId?: string) => { - const params: { libraryId: string, collectionId?: string } = { libraryId }; - if (collectionId) { - params.collectionId = collectionId; - } + const params: { libraryId: string, collectionId?: string } = { libraryId, collectionId }; return baseRender(, { - path: '/library/:libraryId/*', + path: '/library/:libraryId/:collectionId?', params, extraWrapper: ({ children }) => ( { children } diff --git a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx index f5d3606c5..80efc8cb3 100644 --- a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx +++ b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx @@ -34,7 +34,6 @@ const render = () => baseRender( ( {children} diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx index 4c370f26e..277a6fc1c 100644 --- a/src/library-authoring/collections/CollectionInfo.tsx +++ b/src/library-authoring/collections/CollectionInfo.tsx @@ -6,7 +6,6 @@ import { Tabs, } from '@openedx/paragon'; import { useCallback } from 'react'; -import { useNavigate, useMatch } from 'react-router-dom'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; @@ -17,6 +16,7 @@ import { isCollectionInfoTab, useSidebarContext, } from '../common/context/SidebarContext'; +import { useLibraryRoutes } from '../routes'; import { ContentTagsDrawer } from '../../content-tags-drawer'; import { buildCollectionUsageKey } from '../../generic/key-utils'; import CollectionDetails from './CollectionDetails'; @@ -24,36 +24,33 @@ import messages from './messages'; const CollectionInfo = () => { const intl = useIntl(); - const navigate = useNavigate(); const { componentPickerMode } = useComponentPickerContext(); - const { libraryId, collectionId, setCollectionId } = useLibraryContext(); - const { sidebarComponentInfo, setSidebarCurrentTab } = useSidebarContext(); + const { libraryId, setCollectionId } = useLibraryContext(); + const { sidebarComponentInfo, sidebarTab, setSidebarTab } = useSidebarContext(); const tab: CollectionInfoTab = ( - sidebarComponentInfo?.currentTab && isCollectionInfoTab(sidebarComponentInfo.currentTab) - ) ? sidebarComponentInfo?.currentTab : COLLECTION_INFO_TABS.Manage; + sidebarTab && isCollectionInfoTab(sidebarTab) + ) ? sidebarTab : COLLECTION_INFO_TABS.Manage; - const sidebarCollectionId = sidebarComponentInfo?.id; + const collectionId = sidebarComponentInfo?.id; // istanbul ignore if: this should never happen - if (!sidebarCollectionId) { - throw new Error('sidebarCollectionId is required'); + if (!collectionId) { + throw new Error('collectionId is required'); } - const url = `/library/${libraryId}/collection/${sidebarCollectionId}`; - const urlMatch = useMatch(url); + const collectionUsageKey = buildCollectionUsageKey(libraryId, collectionId); - const showOpenCollectionButton = !urlMatch && collectionId !== sidebarCollectionId; - - const collectionUsageKey = buildCollectionUsageKey(libraryId, sidebarCollectionId); + const { insideCollection, navigateTo } = useLibraryRoutes(); + const showOpenCollectionButton = !insideCollection || componentPickerMode; const handleOpenCollection = useCallback(() => { - if (!componentPickerMode) { - navigate(url); + if (componentPickerMode) { + setCollectionId(collectionId); } else { - setCollectionId(sidebarCollectionId); + navigateTo({ collectionId }); } - }, [componentPickerMode, url]); + }, [componentPickerMode, navigateTo]); return ( @@ -73,7 +70,7 @@ const CollectionInfo = () => { className="my-3 d-flex justify-content-around" defaultActiveKey={COMPONENT_INFO_TABS.Manage} activeKey={tab} - onSelect={setSidebarCurrentTab} + onSelect={setSidebarTab} > { const { totalHits: componentCount, isFiltered } = useSearchContext(); diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 1d9eca821..1305183b6 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -13,6 +13,7 @@ import classNames from 'classnames'; import { Helmet } from 'react-helmet'; import { Link } from 'react-router-dom'; +import { useLibraryRoutes } from '../routes'; import Loading from '../../generic/Loading'; import ErrorAlert from '../../generic/alert-error'; import SubHeader from '../../generic/sub-header/SubHeader'; @@ -46,6 +47,7 @@ const HeaderActions = () => { openCollectionInfoSidebar, sidebarComponentInfo, } = useSidebarContext(); + const { navigateTo } = useLibraryRoutes(); // istanbul ignore if: this should never happen if (!collectionId) { @@ -61,6 +63,10 @@ const HeaderActions = () => { } else { openCollectionInfoSidebar(collectionId); } + + if (!componentPickerMode) { + navigateTo({ collectionId }); + } }; return ( @@ -102,8 +108,8 @@ const LibraryCollectionPage = () => { } const { componentPickerMode } = useComponentPickerContext(); - const { showOnlyPublished, setCollectionId } = useLibraryContext(); - const { sidebarComponentInfo, openCollectionInfoSidebar } = useSidebarContext(); + const { showOnlyPublished, setCollectionId, componentId } = useLibraryContext(); + const { sidebarComponentInfo, openInfoSidebar } = useSidebarContext(); const { data: collectionData, @@ -113,8 +119,8 @@ const LibraryCollectionPage = () => { } = useCollection(libraryId, collectionId); useEffect(() => { - openCollectionInfoSidebar(collectionId); - }, [collectionData]); + openInfoSidebar(componentId, collectionId); + }, []); const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId); diff --git a/src/library-authoring/common/context/LibraryContext.tsx b/src/library-authoring/common/context/LibraryContext.tsx index 9612a9285..d1abb4078 100644 --- a/src/library-authoring/common/context/LibraryContext.tsx +++ b/src/library-authoring/common/context/LibraryContext.tsx @@ -6,6 +6,7 @@ import { useMemo, useState, } from 'react'; +import { useParams } from 'react-router-dom'; import type { ComponentPicker } from '../../component-picker'; import type { ContentLibrary } from '../../data/api'; @@ -25,6 +26,8 @@ export type LibraryContextData = { isLoadingLibraryData: boolean; collectionId: string | undefined; setCollectionId: (collectionId?: string) => void; + componentId: string | undefined; + setComponentId: (componentId?: string) => void; // Only show published components showOnlyPublished: boolean; // "Create New Collection" modal @@ -53,9 +56,10 @@ const LibraryContext = createContext(undefined); type LibraryProviderProps = { children?: React.ReactNode; libraryId: string; - /** The initial collection ID to show */ - collectionId?: string; showOnlyPublished?: boolean; + // If set, will initialize the current collection and/or component from the current URL + skipUrlUpdate?: boolean; + /** The component picker modal to use. We need to pass it as a reference instead of * directly importing it to avoid the import cycle: * ComponentPicker > LibraryAuthoringPage/LibraryCollectionPage > @@ -69,11 +73,10 @@ type LibraryProviderProps = { export const LibraryProvider = ({ children, libraryId, - collectionId: collectionIdProp, showOnlyPublished = false, + skipUrlUpdate = false, componentPicker, }: LibraryProviderProps) => { - const [collectionId, setCollectionId] = useState(collectionIdProp); const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); const [componentBeingEdited, setComponentBeingEdited] = useState(); const closeComponentEditor = useCallback(() => { @@ -94,12 +97,23 @@ export const LibraryProvider = ({ const readOnly = !!componentPickerMode || !libraryData?.canEditLibrary; + // Parse the initial collectionId and/or componentId from the current URL params + const params = useParams(); + const [componentId, setComponentId] = useState( + skipUrlUpdate ? undefined : params.componentId, + ); + const [collectionId, setCollectionId] = useState( + skipUrlUpdate ? undefined : params.collectionId, + ); + const context = useMemo(() => { const contextValue = { libraryId, libraryData, collectionId, setCollectionId, + componentId, + setComponentId, readOnly, isLoadingLibraryData, showOnlyPublished, @@ -115,9 +129,11 @@ export const LibraryProvider = ({ return contextValue; }, [ libraryId, + libraryData, collectionId, setCollectionId, - libraryData, + componentId, + setComponentId, readOnly, isLoadingLibraryData, showOnlyPublished, diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx index d9ba68d69..7dd7e9c7d 100644 --- a/src/library-authoring/common/context/SidebarContext.tsx +++ b/src/library-authoring/common/context/SidebarContext.tsx @@ -5,6 +5,7 @@ import { useMemo, useState, } from 'react'; +import { useStateWithUrlSearchParam } from '../../../hooks'; export enum SidebarBodyComponentId { AddContent = 'add-content', @@ -32,28 +33,36 @@ export const isComponentInfoTab = (tab: string): tab is ComponentInfoTab => ( Object.values(COMPONENT_INFO_TABS).includes(tab) ); +type SidebarInfoTab = ComponentInfoTab | CollectionInfoTab; +const toSidebarInfoTab = (tab: string): SidebarInfoTab | undefined => ( + isComponentInfoTab(tab) || isCollectionInfoTab(tab) + ? tab : undefined +); + export interface SidebarComponentInfo { type: SidebarBodyComponentId; id: string; - /** Additional action on Sidebar display */ - additionalAction?: SidebarAdditionalActions; - /** Current tab in the sidebar */ - currentTab?: CollectionInfoTab | ComponentInfoTab; } -export enum SidebarAdditionalActions { +export enum SidebarActions { JumpToAddCollections = 'jump-to-add-collections', + ManageTeam = 'manage-team', + None = '', } export type SidebarContextData = { closeLibrarySidebar: () => void; openAddContentSidebar: () => void; - openInfoSidebar: () => void; - openCollectionInfoSidebar: (collectionId: string, additionalAction?: SidebarAdditionalActions) => void; - openComponentInfoSidebar: (usageKey: string, additionalAction?: SidebarAdditionalActions) => void; + openInfoSidebar: (componentId?: string, collectionId?: string) => void; + openLibrarySidebar: () => void; + openCollectionInfoSidebar: (collectionId: string) => void; + openComponentInfoSidebar: (usageKey: string) => void; sidebarComponentInfo?: SidebarComponentInfo; - resetSidebarAdditionalActions: () => void; - setSidebarCurrentTab: (tab: CollectionInfoTab | ComponentInfoTab) => void; + sidebarAction: SidebarActions; + setSidebarAction: (action: SidebarActions) => void; + resetSidebarAction: () => void; + sidebarTab: SidebarInfoTab; + setSidebarTab: (tab: SidebarInfoTab) => void; }; /** @@ -71,7 +80,7 @@ type SidebarProviderProps = { }; /** - * React component to provide `LibraryContext` + * React component to provide `SidebarContext` */ export const SidebarProvider = ({ children, @@ -81,12 +90,22 @@ export const SidebarProvider = ({ initialSidebarComponentInfo, ); - /** Helper function to consume addtional action once performed. - Required to redo the action. - */ - const resetSidebarAdditionalActions = useCallback(() => { - setSidebarComponentInfo((prev) => (prev && { ...prev, additionalAction: undefined })); - }, []); + const [sidebarTab, setSidebarTab] = useStateWithUrlSearchParam( + COMPONENT_INFO_TABS.Preview, + 'st', + (value: string) => toSidebarInfoTab(value), + (value: SidebarInfoTab) => value.toString(), + ); + + const [sidebarAction, setSidebarAction] = useStateWithUrlSearchParam( + SidebarActions.None, + 'sa', + (value: string) => Object.values(SidebarActions).find((enumValue) => value === enumValue), + (value: SidebarActions) => value.toString(), + ); + const resetSidebarAction = useCallback(() => { + setSidebarAction(SidebarActions.None); + }, [setSidebarAction]); const closeLibrarySidebar = useCallback(() => { setSidebarComponentInfo(undefined); @@ -94,33 +113,32 @@ export const SidebarProvider = ({ const openAddContentSidebar = useCallback(() => { setSidebarComponentInfo({ id: '', type: SidebarBodyComponentId.AddContent }); }, []); - const openInfoSidebar = useCallback(() => { + const openLibrarySidebar = useCallback(() => { setSidebarComponentInfo({ id: '', type: SidebarBodyComponentId.Info }); }, []); - const openComponentInfoSidebar = useCallback((usageKey: string, additionalAction?: SidebarAdditionalActions) => { - setSidebarComponentInfo((prev) => ({ - ...prev, + const openComponentInfoSidebar = useCallback((usageKey: string) => { + setSidebarComponentInfo({ id: usageKey, type: SidebarBodyComponentId.ComponentInfo, - additionalAction, - })); + }); }, []); - const openCollectionInfoSidebar = useCallback(( - newCollectionId: string, - additionalAction?: SidebarAdditionalActions, - ) => { - setSidebarComponentInfo((prev) => ({ - ...prev, + const openCollectionInfoSidebar = useCallback((newCollectionId: string) => { + setSidebarComponentInfo({ id: newCollectionId, type: SidebarBodyComponentId.CollectionInfo, - additionalAction, - })); + }); }, []); - const setSidebarCurrentTab = useCallback((tab: CollectionInfoTab | ComponentInfoTab) => { - setSidebarComponentInfo((prev) => (prev && { ...prev, currentTab: tab })); + const openInfoSidebar = useCallback((componentId?: string, collectionId?: string) => { + if (componentId) { + openComponentInfoSidebar(componentId); + } else if (collectionId) { + openCollectionInfoSidebar(collectionId); + } else { + openLibrarySidebar(); + } }, []); const context = useMemo(() => { @@ -128,11 +146,15 @@ export const SidebarProvider = ({ closeLibrarySidebar, openAddContentSidebar, openInfoSidebar, + openLibrarySidebar, openComponentInfoSidebar, sidebarComponentInfo, openCollectionInfoSidebar, - resetSidebarAdditionalActions, - setSidebarCurrentTab, + sidebarAction, + setSidebarAction, + resetSidebarAction, + sidebarTab, + setSidebarTab, }; return contextValue; @@ -140,11 +162,15 @@ export const SidebarProvider = ({ closeLibrarySidebar, openAddContentSidebar, openInfoSidebar, + openLibrarySidebar, openComponentInfoSidebar, sidebarComponentInfo, openCollectionInfoSidebar, - resetSidebarAdditionalActions, - setSidebarCurrentTab, + sidebarAction, + setSidebarAction, + resetSidebarAction, + sidebarTab, + setSidebarTab, ]); return ( @@ -162,10 +188,14 @@ export function useSidebarContext(): SidebarContextData { closeLibrarySidebar: () => {}, openAddContentSidebar: () => {}, openInfoSidebar: () => {}, + openLibrarySidebar: () => {}, openComponentInfoSidebar: () => {}, openCollectionInfoSidebar: () => {}, - resetSidebarAdditionalActions: () => {}, - setSidebarCurrentTab: () => {}, + sidebarAction: SidebarActions.None, + setSidebarAction: () => {}, + resetSidebarAction: () => {}, + sidebarTab: COMPONENT_INFO_TABS.Preview, + setSidebarTab: () => {}, sidebarComponentInfo: undefined, }; } diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index c9db685e9..085e6d120 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/src/library-authoring/component-info/ComponentInfo.tsx @@ -16,7 +16,7 @@ import { useLibraryContext } from '../common/context/LibraryContext'; import { type ComponentInfoTab, COMPONENT_INFO_TABS, - SidebarAdditionalActions, + SidebarActions, isComponentInfoTab, useSidebarContext, } from '../common/context/SidebarContext'; @@ -101,27 +101,27 @@ const ComponentInfo = () => { const intl = useIntl(); const { readOnly, openComponentEditor } = useLibraryContext(); - const { setSidebarCurrentTab, sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext(); + const { + sidebarTab, + setSidebarTab, + sidebarComponentInfo, + sidebarAction, + } = useSidebarContext(); - const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections; + const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; const tab: ComponentInfoTab = ( - sidebarComponentInfo?.currentTab && isComponentInfoTab(sidebarComponentInfo.currentTab) - ) ? sidebarComponentInfo?.currentTab : COMPONENT_INFO_TABS.Preview; + isComponentInfoTab(sidebarTab) + ? sidebarTab + : COMPONENT_INFO_TABS.Preview + ); useEffect(() => { // Show Manage tab if JumpToAddCollections action is set in sidebarComponentInfo if (jumpToCollections) { - setSidebarCurrentTab(COMPONENT_INFO_TABS.Manage); - } - }, [jumpToCollections]); - - useEffect(() => { - // This is required to redo actions. - if (tab !== COMPONENT_INFO_TABS.Manage) { - resetSidebarAdditionalActions(); + setSidebarTab(COMPONENT_INFO_TABS.Manage); } - }, [tab]); + }, [jumpToCollections, setSidebarTab]); const usageKey = sidebarComponentInfo?.id; // istanbul ignore if: this should never happen @@ -169,7 +169,7 @@ const ComponentInfo = () => { className="my-3 d-flex justify-content-around" defaultActiveKey={COMPONENT_INFO_TABS.Preview} activeKey={tab} - onSelect={setSidebarCurrentTab} + onSelect={setSidebarTab} > diff --git a/src/library-authoring/component-info/ComponentManagement.tsx b/src/library-authoring/component-info/ComponentManagement.tsx index bbdfc51ef..67a224dd0 100644 --- a/src/library-authoring/component-info/ComponentManagement.tsx +++ b/src/library-authoring/component-info/ComponentManagement.tsx @@ -7,7 +7,7 @@ import { } from '@openedx/paragon/icons'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarAdditionalActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; import { useLibraryBlockMetadata } from '../data/apiHooks'; import StatusWidget from '../generic/status-widget'; import messages from './messages'; @@ -18,8 +18,8 @@ import ManageCollections from './ManageCollections'; const ComponentManagement = () => { const intl = useIntl(); const { readOnly, isLoadingLibraryData } = useLibraryContext(); - const { sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext(); - const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections; + const { sidebarComponentInfo, sidebarAction, resetSidebarAction } = useSidebarContext(); + const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; const [tagsCollapseIsOpen, setTagsCollapseOpen] = React.useState(!jumpToCollections); const [collectionsCollapseIsOpen, setCollectionsCollapseOpen] = React.useState(true); @@ -33,7 +33,7 @@ const ComponentManagement = () => { useEffect(() => { // This is required to redo actions. if (tagsCollapseIsOpen || !collectionsCollapseIsOpen) { - resetSidebarAdditionalActions(); + resetSidebarAction(); } }, [tagsCollapseIsOpen, collectionsCollapseIsOpen]); diff --git a/src/library-authoring/component-info/ManageCollections.test.tsx b/src/library-authoring/component-info/ManageCollections.test.tsx index 9c3d69654..790fb0a99 100644 --- a/src/library-authoring/component-info/ManageCollections.test.tsx +++ b/src/library-authoring/component-info/ManageCollections.test.tsx @@ -13,6 +13,7 @@ import { mockContentSearchConfig } from '../../search-manager/data/api.mock'; import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; import ManageCollections from './ManageCollections'; import { LibraryProvider } from '../common/context/LibraryContext'; +import { SidebarProvider } from '../common/context/SidebarContext'; import { getLibraryBlockCollectionsUrl } from '../data/api'; let axiosMock: MockAdapter; @@ -25,7 +26,9 @@ mockContentSearchConfig.applyMock(); const render = (ui: React.ReactElement) => baseRender(ui, { extraWrapper: ({ children }) => ( - {children} + + {children} + ), }); diff --git a/src/library-authoring/component-info/ManageCollections.tsx b/src/library-authoring/component-info/ManageCollections.tsx index b7cbc9832..1ab32c8b9 100644 --- a/src/library-authoring/component-info/ManageCollections.tsx +++ b/src/library-authoring/component-info/ManageCollections.tsx @@ -1,4 +1,4 @@ -import { useContext, useEffect, useState } from 'react'; +import { useContext, useState } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { Button, Icon, Scrollable, SelectableBox, Stack, StatefulButton, useCheckboxSetValues, @@ -16,7 +16,7 @@ import { useUpdateComponentCollections } from '../data/apiHooks'; import { ToastContext } from '../../generic/toast-context'; import { CollectionMetadata } from '../data/api'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarAdditionalActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; interface ManageCollectionsProps { usageKey: string; @@ -191,38 +191,23 @@ const ComponentCollections = ({ collections, onManageClick }: { }; const ManageCollections = ({ usageKey, collections }: ManageCollectionsProps) => { - const { sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext(); - const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections; - const [editing, setEditing] = useState(jumpToCollections); + const { sidebarAction, resetSidebarAction, setSidebarAction } = useSidebarContext(); const collectionNames = collections.map((collection) => collection.title); - useEffect(() => { - if (jumpToCollections) { - setEditing(true); - } - }, [sidebarComponentInfo]); - - useEffect(() => { - // This is required to redo actions. - if (!editing) { - resetSidebarAdditionalActions(); - } - }, [editing]); - - if (editing) { - return ( - setEditing(false)} - /> - ); - } return ( - setEditing(true)} - /> + sidebarAction === SidebarActions.JumpToAddCollections + ? ( + resetSidebarAction()} + /> + ) : ( + setSidebarAction(SidebarActions.JumpToAddCollections)} + /> + ) ); }; diff --git a/src/library-authoring/component-picker/ComponentPicker.tsx b/src/library-authoring/component-picker/ComponentPicker.tsx index 115c081fe..75bf178ac 100644 --- a/src/library-authoring/component-picker/ComponentPicker.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.tsx @@ -105,6 +105,7 @@ export const ComponentPicker: React.FC = ({ { calcShowOnlyPublished diff --git a/src/library-authoring/components/BaseComponentCard.tsx b/src/library-authoring/components/BaseComponentCard.tsx index 3b5aa748c..00ccc0016 100644 --- a/src/library-authoring/components/BaseComponentCard.tsx +++ b/src/library-authoring/components/BaseComponentCard.tsx @@ -16,7 +16,7 @@ type BaseComponentCardProps = { numChildren?: number, tags: ContentHitTags, actions: React.ReactNode, - openInfoSidebar: () => void + onSelect: () => void }; const BaseComponentCard = ({ @@ -26,7 +26,7 @@ const BaseComponentCard = ({ numChildren, tags, actions, - openInfoSidebar, + onSelect, } : BaseComponentCardProps) => { const tagCount = useMemo(() => { if (!tags) { @@ -42,10 +42,10 @@ const BaseComponentCard = ({ { if (['Enter', ' '].includes(e.key)) { - openInfoSidebar(); + onSelect(); } }} > diff --git a/src/library-authoring/components/CollectionCard.tsx b/src/library-authoring/components/CollectionCard.tsx index 6935bc12c..ee165e04a 100644 --- a/src/library-authoring/components/CollectionCard.tsx +++ b/src/library-authoring/components/CollectionCard.tsx @@ -14,6 +14,7 @@ import { type CollectionHit } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useSidebarContext } from '../common/context/SidebarContext'; +import { useLibraryRoutes } from '../routes'; import BaseComponentCard from './BaseComponentCard'; import { ToastContext } from '../../generic/toast-context'; import { useDeleteCollection, useRestoreCollection } from '../data/apiHooks'; @@ -112,6 +113,7 @@ const CollectionCard = ({ collectionHit } : CollectionCardProps) => { const { type: componentType, + blockId: collectionId, formatted, tags, numChildren, @@ -124,6 +126,15 @@ const CollectionCard = ({ collectionHit } : CollectionCardProps) => { const { displayName = '', description = '' } = formatted; + const { navigateTo } = useLibraryRoutes(); + const openCollection = useCallback(() => { + openCollectionInfoSidebar(collectionId); + + if (!componentPickerMode) { + navigateTo({ collectionId }); + } + }, [collectionId, navigateTo, openCollectionInfoSidebar]); + return ( { )} - openInfoSidebar={() => openCollectionInfoSidebar(collectionHit.blockId)} + onSelect={openCollection} /> ); }; diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index 813255b97..196a060f9 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -1,4 +1,4 @@ -import { useContext, useState } from 'react'; +import { useCallback, useContext, useState } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, @@ -21,8 +21,10 @@ import { ToastContext } from '../../generic/toast-context'; import { type ContentHit } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarAdditionalActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; import { useRemoveComponentsFromCollection } from '../data/apiHooks'; +import { useLibraryRoutes } from '../routes'; + import BaseComponentCard from './BaseComponentCard'; import { canEditComponent } from './ComponentEditorModal'; import messages from './messages'; @@ -44,6 +46,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { sidebarComponentInfo, openComponentInfoSidebar, closeLibrarySidebar, + setSidebarAction, } = useSidebarContext(); const canEdit = usageKey && canEditComponent(usageKey); @@ -73,9 +76,10 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { }); }; - const showManageCollections = () => { - openComponentInfoSidebar(usageKey, SidebarAdditionalActions.JumpToAddCollections); - }; + const showManageCollections = useCallback(() => { + setSidebarAction(SidebarActions.JumpToAddCollections); + openComponentInfoSidebar(usageKey); + }, [setSidebarAction, openComponentInfoSidebar, usageKey]); return ( @@ -200,6 +204,15 @@ const ComponentCard = ({ contentHit }: ComponentCardProps) => { showOnlyPublished ? formatted.published?.displayName : formatted.displayName ) ?? ''; + const { navigateTo } = useLibraryRoutes(); + const openComponent = useCallback(() => { + openComponentInfoSidebar(usageKey); + + if (!componentPickerMode) { + navigateTo({ componentId: usageKey }); + } + }, [usageKey, navigateTo, openComponentInfoSidebar]); + return ( { )} )} - openInfoSidebar={() => openComponentInfoSidebar(usageKey)} + onSelect={openComponent} /> ); }; diff --git a/src/library-authoring/library-info/LibraryInfo.tsx b/src/library-authoring/library-info/LibraryInfo.tsx index 755e2ec76..5269b798a 100644 --- a/src/library-authoring/library-info/LibraryInfo.tsx +++ b/src/library-authoring/library-info/LibraryInfo.tsx @@ -1,15 +1,24 @@ -import { Button, Stack, useToggle } from '@openedx/paragon'; +import { useCallback } from 'react'; +import { Button, Stack } from '@openedx/paragon'; import { FormattedDate, useIntl } from '@edx/frontend-platform/i18n'; import messages from './messages'; import LibraryPublishStatus from './LibraryPublishStatus'; import { LibraryTeamModal } from '../library-team'; import { useLibraryContext } from '../common/context/LibraryContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; const LibraryInfo = () => { const intl = useIntl(); const { libraryData, readOnly } = useLibraryContext(); - const [isLibraryTeamModalOpen, openLibraryTeamModal, closeLibraryTeamModal] = useToggle(); + const { sidebarAction, setSidebarAction, resetSidebarAction } = useSidebarContext(); + const isLibraryTeamModalOpen = (sidebarAction === SidebarActions.ManageTeam); + const openLibraryTeamModal = useCallback(() => { + setSidebarAction(SidebarActions.ManageTeam); + }, [setSidebarAction]); + const closeLibraryTeamModal = useCallback(() => { + resetSidebarAction(); + }, [resetSidebarAction]); return ( diff --git a/src/library-authoring/library-info/LibraryInfoHeader.tsx b/src/library-authoring/library-info/LibraryInfoHeader.tsx index 95d2f5fd2..cafeccf26 100644 --- a/src/library-authoring/library-info/LibraryInfoHeader.tsx +++ b/src/library-authoring/library-info/LibraryInfoHeader.tsx @@ -43,7 +43,7 @@ const LibraryInfoHeader = () => { setIsActive(true); }; - const hanldeOnKeyDown = (event) => { + const handleOnKeyDown = (event) => { if (event.key === 'Enter') { handleSaveTitle(event); } else if (event.key === 'Escape') { @@ -63,7 +63,7 @@ const LibraryInfoHeader = () => { aria-label="Title input" defaultValue={library.title} onBlur={handleSaveTitle} - onKeyDown={hanldeOnKeyDown} + onKeyDown={handleOnKeyDown} /> ) : ( diff --git a/src/library-authoring/routes.test.tsx b/src/library-authoring/routes.test.tsx new file mode 100644 index 000000000..488b7889d --- /dev/null +++ b/src/library-authoring/routes.test.tsx @@ -0,0 +1,289 @@ +import { useEffect } from 'react'; +import fetchMock from 'fetch-mock-jest'; +import { + mockContentLibrary, +} from './data/api.mocks'; +import { LibraryLayout } from '.'; +import { ContentType, useLibraryRoutes } from './routes'; +import mockResult from './__mocks__/library-search.json'; +import { initializeMocks, render } from '../testUtils'; +import { studioHomeMock } from '../studio-home/__mocks__'; +import { getStudioHomeApiUrl } from '../studio-home/data/api'; + +const mockNavigate = jest.fn(); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useNavigate: () => mockNavigate, +})); + +mockContentLibrary.applyMock(); + +const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; +describe('Library Authoring routes', () => { + beforeEach(async () => { + const { axiosMock } = initializeMocks(); + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock); + + // The Meilisearch client-side API uses fetch, not Axios. + fetchMock.mockReset(); + 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. + const newMockResult = { ...mockResult }; + newMockResult.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 + newMockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); + return newMockResult; + }); + }); + + test.each([ + // "All Content" tab + { + label: 'navigate from All Content tab to Components tab', + origin: { + path: '', + params: {}, + }, + destination: { + path: '/components', + params: { + contentType: ContentType.components, + }, + }, + }, + { + label: 'navigate from All Content tab to Collections tab', + origin: { + path: '', + params: {}, + }, + destination: { + params: { + contentType: ContentType.collections, + }, + path: '/collections', + }, + }, + { + label: 'from All Content tab, select a Component', + origin: { + path: '', + params: {}, + }, + destination: { + params: { + componentId: 'cmptId', + }, + path: '/component/cmptId', + }, + }, + { + label: 'from All Content tab > selected Component, select a different Component', + origin: { + path: '', + params: {}, + }, + destination: { + params: { + componentId: 'cmptId2', + }, + path: '/component/cmptId2', + }, + }, + { + label: 'from All Content tab, select a Collection', + origin: { + path: '', + params: {}, + }, + destination: { + params: { + collectionId: 'clctnId', + }, + path: '/clctnId', + }, + }, + { + label: 'navigate from All Content > selected Collection to the Collection page', + origin: { + params: { + collectionId: 'clctnId', + }, + path: '/clctnId', + }, + destination: { + params: { + collectionId: 'clctnId', + }, + /* + * Note: the MemoryRouter used by testUtils breaks this, but should be: + * path: '/collection/clctnId', + */ + path: '/clctnId', + }, + }, + { + label: 'from All Content > Collection, select a different Collection', + origin: { + params: { + collectionId: 'clctnId', + }, + path: '/clctnId', + }, + destination: { + params: { + collectionId: 'clctnId2', + }, + path: '/clctnId2', + }, + }, + // "Components" tab + { + label: 'navigate from Components tab to All Content tab', + origin: { + path: '/components', + params: {}, + }, + destination: { + path: '', + params: { + contentType: ContentType.home, + }, + }, + }, + { + label: 'navigate from Components tab to Collections tab', + origin: { + label: 'Components tab', + path: '/components', + params: {}, + }, + destination: { + label: 'Collections tab', + params: { + contentType: ContentType.collections, + }, + path: '/collections', + }, + }, + { + label: 'from Components tab, select a Component', + origin: { + path: '/components', + params: {}, + }, + destination: { + params: { + componentId: 'cmptId', + }, + path: '/components/cmptId', + }, + }, + // "Collections" tab + { + label: 'navigate from Collections tab to All Content tab', + origin: { + path: '/collections', + params: {}, + }, + destination: { + path: '', + params: { + contentType: ContentType.home, + }, + }, + }, + { + label: 'navigate from Collections tab to Components tab', + origin: { + path: '/collections', + params: {}, + }, + destination: { + path: '/components', + params: { + contentType: ContentType.components, + }, + }, + }, + { + label: 'from Collections tab, select a Collection', + origin: { + path: '/collections', + params: {}, + }, + destination: { + params: { + collectionId: 'clctnId', + }, + path: '/collections/clctnId', + }, + }, + { + label: 'from Collections tab > selected Collection, navigate to the Collection page', + origin: { + params: { + collectionId: 'clctnId', + }, + path: '/collections/clctnId', + }, + destination: { + params: { + collectionId: 'clctnId', + }, + /* + * Note: the MemoryRouter used by testUtils breaks this, but should be: + * path: '/collection/clctnId', + */ + path: '/collections/clctnId', + }, + }, + { + label: 'from Collections > selected Collection, select a different Collection', + origin: { + params: { + collectionId: 'clctnId', + }, + path: '/collections/clctnId', + }, + destination: { + params: { + collectionId: 'clctnId2', + }, + path: '/collections/clctnId2', + }, + }, + ])( + '$label', + async ({ origin, destination }) => { + const LibraryRouterTest = () => { + /* + * Note: we'd also like to test the insideComponent etc. flags returned here, + * but the MemoryRouter used by testUtils makes this impossible. + */ + const { navigateTo } = useLibraryRoutes(); + useEffect(() => navigateTo(destination.params), [destination.params]); + return ; + }; + + render(, { + path: `/library/:libraryId${origin.path}/*`, + params: { + libraryId: mockContentLibrary.libraryId, + collectionId: '', + ...origin.params, + }, + }); + + expect(mockNavigate).toBeCalledWith({ + pathname: `/library/${mockContentLibrary.libraryId}${destination.path}`, + search: '', + }); + }, + ); +}); diff --git a/src/library-authoring/routes.ts b/src/library-authoring/routes.ts new file mode 100644 index 000000000..fa96c7be0 --- /dev/null +++ b/src/library-authoring/routes.ts @@ -0,0 +1,128 @@ +/** + * Constants and utility hook for the Library Authoring routes. + */ +import { useCallback } from 'react'; +import { + generatePath, + matchPath, + useParams, + useLocation, + useNavigate, + useSearchParams, + type PathMatch, +} from 'react-router-dom'; + +export const BASE_ROUTE = '/library/:libraryId'; + +export const ROUTES = { + // LibraryAuthoringPage routes: + // * Components tab, with an optionally selected componentId in the sidebar. + COMPONENTS: '/components/:componentId?', + // * Collections tab, with an optionally selected collectionId in the sidebar. + COLLECTIONS: '/collections/:collectionId?', + // * All Content tab, with an optionally selected componentId in the sidebar. + COMPONENT: '/component/:componentId', + // * All Content tab, with an optionally selected collectionId in the sidebar. + HOME: '/:collectionId?', + // LibraryCollectionPage route: + // * with a selected collectionId and/or an optionally selected componentId. + COLLECTION: '/collection/:collectionId/:componentId?', +}; + +export enum ContentType { + home = '', + components = 'components', + collections = 'collections', +} + +export type NavigateToData = { + componentId?: string, + collectionId?: string, + contentType?: ContentType, +}; + +export type LibraryRoutesData = { + insideCollection: PathMatch | null; + insideCollections: PathMatch | null; + insideComponents: PathMatch | null; + + // Navigate using the best route from the current location for the given parameters. + navigateTo: (dict?: NavigateToData) => void; +}; + +export const useLibraryRoutes = (): LibraryRoutesData => { + const { pathname } = useLocation(); + const params = useParams(); + const [searchParams] = useSearchParams(); + const navigate = useNavigate(); + + const insideCollection = matchPath(BASE_ROUTE + ROUTES.COLLECTION, pathname); + const insideCollections = matchPath(BASE_ROUTE + ROUTES.COLLECTIONS, pathname); + const insideComponents = matchPath(BASE_ROUTE + ROUTES.COMPONENTS, pathname); + + const navigateTo = useCallback(({ + componentId, + collectionId, + contentType, + }: NavigateToData = {}) => { + const routeParams = { + ...params, + // Overwrite the current componentId/collectionId params if provided + ...((componentId !== undefined) && { componentId }), + ...((collectionId !== undefined) && { collectionId }), + }; + let route; + + // Providing contentType overrides the current route so we can change tabs. + if (contentType === ContentType.components) { + route = ROUTES.COMPONENTS; + } else if (contentType === ContentType.collections) { + route = ROUTES.COLLECTIONS; + } else if (contentType === ContentType.home) { + route = ROUTES.HOME; + } else if (insideCollections) { + // We're inside the Collections tab, + route = ( + (collectionId && collectionId === params.collectionId) + // now open the previously-selected collection, + ? ROUTES.COLLECTION + // or stay there to list all collections, or a selected collection. + : ROUTES.COLLECTIONS + ); + } else if (insideCollection) { + // We're viewing a Collection, so stay there, + // and optionally select a component in that collection. + route = ROUTES.COLLECTION; + } else if (insideComponents) { + // We're inside the Components tab, so stay there, + // optionally selecting a component. + route = ROUTES.COMPONENTS; + } else if (componentId) { + // We're inside the All Content tab, so stay there, + // and select a component. + route = ROUTES.COMPONENT; + } else { + // We're inside the All Content tab, + route = ( + (collectionId && collectionId === params.collectionId) + // now open the previously-selected collection + ? ROUTES.COLLECTION + // or stay there to list all content, or optionally select a collection. + : ROUTES.HOME + ); + } + + const newPath = generatePath(BASE_ROUTE + route, routeParams); + navigate({ + pathname: newPath, + search: searchParams.toString(), + }); + }, [navigate, params, searchParams, pathname]); + + return { + navigateTo, + insideCollection, + insideCollections, + insideComponents, + }; +}; diff --git a/src/search-manager/FilterByBlockType.tsx b/src/search-manager/FilterByBlockType.tsx index 95c8ee756..0b5160d37 100644 --- a/src/search-manager/FilterByBlockType.tsx +++ b/src/search-manager/FilterByBlockType.tsx @@ -18,7 +18,7 @@ import { useSearchContext } from './SearchManager'; interface ProblemFilterItemProps { count: number, - handleCheckboxChange: Function, + handleCheckboxChange: (e: any) => void; } interface FilterItemProps { blockType: string, @@ -29,11 +29,9 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr const blockType = 'problem'; const { - setBlockTypesFilter, problemTypes, - problemTypesFilter, - blockTypesFilter, - setProblemTypesFilter, + typesFilter, + setTypesFilter, } = useSearchContext(); const intl = useIntl(); @@ -45,65 +43,41 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr useEffect(() => { /* istanbul ignore next */ - if (problemTypesFilter.length !== 0 - && !blockTypesFilter.includes(blockType)) { + const selectedProblemTypes = typesFilter.problems.size; + if (!selectedProblemTypes || selectedProblemTypes === problemTypesLength) { + setIsProblemIndeterminate(false); + } else if (selectedProblemTypes) { setIsProblemIndeterminate(true); } - }, []); - - const handleCheckBoxChangeOnProblem = React.useCallback((e) => { - handleCheckboxChange(e); - setIsProblemIndeterminate(false); - if (e.target.checked) { - setProblemTypesFilter(Object.keys(problemTypes)); - } else { - setProblemTypesFilter([]); - } - }, [handleCheckboxChange, setProblemTypesFilter]); + }, [typesFilter, problemTypesLength, setIsProblemIndeterminate]); const handleProblemCheckboxChange = React.useCallback((e) => { - setProblemTypesFilter(currentFiltersProblem => { - let result; - if (currentFiltersProblem.includes(e.target.value)) { - result = currentFiltersProblem.filter(x => x !== e.target.value); + setTypesFilter((types) => { + if (e.target.checked) { + types.problems.add(e.target.value); } else { - result = [...currentFiltersProblem, e.target.value]; + types.problems.delete(e.target.value); } - if (e.target.checked) { - /* istanbul ignore next */ - if (result.length === problemTypesLength) { - // Add 'problem' to type filter if all problem types are selected. - setIsProblemIndeterminate(false); - setBlockTypesFilter(currentFilters => [...currentFilters, 'problem']); - } else { - setIsProblemIndeterminate(true); - } - } /* istanbul ignore next */ else { - // Delete 'problem' filter if a problem is deselected. - setBlockTypesFilter(currentFilters => { - /* istanbul ignore next */ - if (currentFilters.includes('problem')) { - return currentFilters.filter(x => x !== 'problem'); - } - return [...currentFilters]; - }); - setIsProblemIndeterminate(result.length !== 0); + + if (types.problems.size === problemTypesLength) { + // Add 'problem' to block type filter if all problem types are selected. + types.blocks.add(blockType); + } else { + // Delete 'problem' filter if its selected. + types.blocks.delete(blockType); } - return result; + + return types; }); - }, [ - setProblemTypesFilter, - problemTypesFilter, - setBlockTypesFilter, - problemTypesLength, - ]); + }, [setTypesFilter, problemTypesLength]); return (
@@ -135,7 +109,7 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr { Object.entries(problemTypes).map(([problemType, problemTypeCount]) => ( @@ -170,17 +144,28 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr const FilterItem = ({ blockType, count } : FilterItemProps) => { const { - setBlockTypesFilter, + problemTypes, + setTypesFilter, } = useSearchContext(); const handleCheckboxChange = React.useCallback((e) => { - setBlockTypesFilter(currentFilters => { - if (currentFilters.includes(e.target.value)) { - return currentFilters.filter(x => x !== e.target.value); + setTypesFilter((types) => { + if (e.target.checked) { + types.blocks.add(e.target.value); + } else { + types.blocks.delete(e.target.value); + } + // The "problem" block type also selects/clears all the problem types + if (blockType === 'problem') { + if (e.target.checked) { + types.union({ problems: Object.keys(problemTypes) }); + } else { + types.problems.clear(); + } } - return [...currentFilters, e.target.value]; + return types; }); - }, [setBlockTypesFilter]); + }, [setTypesFilter]); if (blockType === 'problem') { // Build Capa Problem types filter submenu @@ -206,44 +191,21 @@ 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 = ({ disabled = false }) => { +const FilterByBlockType: React.FC> = () => { const { blockTypes, - blockTypesFilter, - problemTypesFilter, - setBlockTypesFilter, - setProblemTypesFilter, + typesFilter, + setTypesFilter, } = useSearchContext(); const clearFilters = useCallback(/* istanbul ignore next */ () => { - setBlockTypesFilter([]); - setProblemTypesFilter([]); - }, []); - - useEffect(() => { - if (disabled) { - // Clear filters when disabled - const selectedBlockTypesFilter = blockTypesFilter; - const selectedProblemTypesFilter = problemTypesFilter; - clearFilters(); - - return () => { - // Restore filters when re-enabled - setBlockTypesFilter(selectedBlockTypesFilter); - setProblemTypesFilter(selectedProblemTypesFilter); - }; - } - return () => {}; - }, [disabled]); + setTypesFilter((types) => types.clear()); + }, [setTypesFilter]); // Sort blocktypes in order of hierarchy followed by alphabetically for components const sortedBlockTypeKeys = Object.keys(blockTypes).sort((a, b) => { @@ -278,14 +240,10 @@ const FilterByBlockType: React.FC = ({ disabled = false sortedBlockTypes[key] = blockTypes[key]; }); - const appliedFilters = [...blockTypesFilter, ...problemTypesFilter].map( + const appliedFilters = [...typesFilter.blocks, ...typesFilter.problems].map( blockType => ({ label: }), ); - if (disabled) { - return null; - } - return ( = ({ disabled = false { diff --git a/src/search-manager/SearchManager.ts b/src/search-manager/SearchManager.ts index 314c90020..334897cb9 100644 --- a/src/search-manager/SearchManager.ts +++ b/src/search-manager/SearchManager.ts @@ -5,24 +5,23 @@ * https://github.com/algolia/instantsearch/issues/1658 */ 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, } from './data/api'; +import { TypesFilterData, useStateOrUrlSearchParam } from './hooks'; import { useContentSearchConnection, useContentSearchResults } from './data/apiHooks'; +import { getBlockType } from '../generic/key-utils'; export interface SearchContextData { client?: MeiliSearch; indexName?: string; searchKeywords: string; setSearchKeywords: React.Dispatch>; - blockTypesFilter: string[]; - setBlockTypesFilter: React.Dispatch>; - problemTypesFilter: string[]; - setProblemTypesFilter: React.Dispatch>; + typesFilter: TypesFilterData; + setTypesFilter: React.Dispatch>; tagsFilter: string[]; setTagsFilter: React.Dispatch>; blockTypes: Record; @@ -47,64 +46,76 @@ export interface SearchContextData { const SearchContext = React.createContext(undefined); -/** - * Hook which lets you store state variables in the URL search parameters. - * - * It wraps useState with functions that get/set a query string - * search parameter when returning/setting the state variable. - * - */ -function useStateWithUrlSearchParam( - defaultValue: Type, - paramName: string, - // Returns the Type equivalent of the given string value, or - // undefined if value is invalid. - fromString: (value: string | null) => Type | undefined, - // Returns the string equivalent of the given Type value. - // Returning empty string/undefined will clear the url search paramName. - toString: (value: Type) => string | undefined, -): [value: Type, setter: React.Dispatch>] { - const [searchParams, setSearchParams] = useSearchParams(); - // The converted search parameter value takes precedence over the state value. - const returnValue: Type = fromString(searchParams.get(paramName)) ?? defaultValue; - // Function to update the url search parameter - const returnSetter: React.Dispatch> = React.useCallback((value: Type) => { - setSearchParams((prevParams) => { - const paramValue: string = toString(value) ?? ''; - const newSearchParams = new URLSearchParams(prevParams); - // If using the default paramValue, remove it from the search params. - if (paramValue === defaultValue) { - newSearchParams.delete(paramName); - } else { - newSearchParams.set(paramName, paramValue); - } - return newSearchParams; - }, { replace: true }); - }, [setSearchParams]); - - // Return the computed value and wrapped set state function - return [returnValue, returnSetter]; -} - 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 }) => { - const [searchKeywords, setSearchKeywords] = React.useState(''); - const [blockTypesFilter, setBlockTypesFilter] = React.useState([]); - const [problemTypesFilter, setProblemTypesFilter] = React.useState([]); - const [tagsFilter, setTagsFilter] = React.useState([]); - const [usageKey, setUsageKey] = useStateWithUrlSearchParam( + // Search parameters can be set via the query string + // E.g. ?q=draft+text + // TODO -- how to sanitize search terms? + const [searchKeywords, setSearchKeywords] = useStateOrUrlSearchParam( + '', + 'q', + (value: string) => value || '', + (value: string) => value || '', + skipUrlUpdate, + ); + + // Block + problem types use alphanumeric plus a few other characters. + // E.g ?type=html&type=video&type=p.multiplechoiceresponse + const [internalTypesFilter, setTypesFilter] = useStateOrUrlSearchParam( + 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. + // E.g ?tag=Skills+>+Abilities&tag=Skills+>+Knowledge + const sanitizeTag = (value: string | null | undefined): string | undefined => ( + (value && /^[^\t;]+$/.test(value)) ? value : undefined + ); + const [tagsFilter, setTagsFilter] = useStateOrUrlSearchParam( + [], + 'tag', + sanitizeTag, + sanitizeTag, + skipUrlUpdate, + ); + + // E.g ?usageKey=lb:OpenCraft:libA:problem:5714eb65-7c36-4eee-8ab9-a54ed5a95849 + const sanitizeUsageKey = (value: string): string | undefined => { + try { + if (getBlockType(value)) { + return value; + } + } catch (error) { + // Error thrown if value cannot be parsed into a library usage key. + // Pass through to return below. + } + return undefined; + }; + const [usageKey, setUsageKey] = useStateOrUrlSearchParam( '', 'usageKey', - (value: string) => value, - (value: string) => value, + sanitizeUsageKey, + sanitizeUsageKey, + skipUrlUpdate, ); let extraFilter: string[] = forceArray(props.extraFilter); @@ -112,21 +123,15 @@ export const SearchContextProvider: React.FC<{ extraFilter = union(extraFilter, [`usage_key = "${usageKey}"`]); } - // The search sort order can be set via the query string - // E.g. ?sort=display_name:desc maps to SearchSortOption.TITLE_ZA. // Default sort by Most Relevant if there's search keyword(s), else by Recently Modified. const defaultSearchSortOrder = searchKeywords ? SearchSortOption.RELEVANCE : SearchSortOption.RECENTLY_MODIFIED; - let sortStateManager = React.useState(defaultSearchSortOrder); - const sortUrlStateManager = useStateWithUrlSearchParam( + const [searchSortOrder, setSearchSortOrder] = useStateOrUrlSearchParam( defaultSearchSortOrder, 'sort', (value: string) => Object.values(SearchSortOption).find((enumValue) => value === enumValue), (value: SearchSortOption) => value.toString(), + skipUrlUpdate, ); - if (!skipUrlUpdate) { - sortStateManager = sortUrlStateManager; - } - const [searchSortOrder, setSearchSortOrder] = sortStateManager; // SearchSortOption.RELEVANCE is special, it means "no custom sorting", so we // send it to useContentSearchResults as an empty array. const searchSortOrderToUse = overrideSearchSortOrder ?? searchSortOrder; @@ -137,16 +142,14 @@ export const SearchContextProvider: React.FC<{ } const canClearFilters = ( - blockTypesFilter.length > 0 - || problemTypesFilter.length > 0 + !typesFilter.isEmpty() || tagsFilter.length > 0 || !!usageKey ); const isFiltered = canClearFilters || (searchKeywords !== ''); const clearFilters = React.useCallback(() => { - setBlockTypesFilter([]); + setTypesFilter((types) => types.clear()); setTagsFilter([]); - setProblemTypesFilter([]); if (usageKey !== '') { setUsageKey(''); } @@ -161,8 +164,8 @@ export const SearchContextProvider: React.FC<{ indexName, extraFilter, searchKeywords, - blockTypesFilter, - problemTypesFilter, + blockTypesFilter: [...typesFilter.blocks], + problemTypesFilter: [...typesFilter.problems], tagsFilter, sort, skipBlockTypeFetch, @@ -174,10 +177,8 @@ export const SearchContextProvider: React.FC<{ indexName, searchKeywords, setSearchKeywords, - blockTypesFilter, - setBlockTypesFilter, - problemTypesFilter, - setProblemTypesFilter, + typesFilter, + setTypesFilter, tagsFilter, setTagsFilter, extraFilter, diff --git a/src/search-manager/hooks.ts b/src/search-manager/hooks.ts new file mode 100644 index 000000000..4741c3469 --- /dev/null +++ b/src/search-manager/hooks.ts @@ -0,0 +1,133 @@ +import React from 'react'; +import { + type FromStringFn, + type ToStringFn, + useStateWithUrlSearchParam, +} from '../hooks'; + +/** + * Typed hook that returns useState if skipUrlUpdate, + * or useStateWithUrlSearchParam otherwise. + * + * Function is overloaded to accept simple Type or Type[] values. + * + * Provided here to reduce some code overhead in SearchManager. + */ +export function useStateOrUrlSearchParam( + defaultValue: Type[], + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, + skipUrlUpdate?: boolean, +): [value: Type[], setter: React.Dispatch>]; +export function useStateOrUrlSearchParam( + defaultValue: Type, + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, + skipUrlUpdate?: boolean, +): [value: Type, setter: React.Dispatch>]; +export function useStateOrUrlSearchParam( + defaultValue: Type | Type[], + paramName: string, + fromString: FromStringFn, + toString: ToStringFn, + skipUrlUpdate?: boolean, +): [value: Type | Type[], setter: React.Dispatch>] { + const useStateManager = React.useState(defaultValue); + const urlStateManager = useStateWithUrlSearchParam( + defaultValue, + paramName, + fromString, + toString, + ); + + return skipUrlUpdate ? useStateManager : urlStateManager; +} + +/** + * Helper class for managing Block + Problem type states. + * + * We use a class to store both Block and Problem types together because + * their behaviour is tightly intertwined: e.g if Block type "problem" is + * selected, that means all available Problem types are also selected. + * + */ +export class TypesFilterData { + #blocks = new Set(); + + #problems = new Set(); + + static #sanitizeType = (value: string | null | undefined): string | undefined => ( + (value && /^[a-z0-9._-]+$/.test(value)) + ? value + : undefined + ); + + static #sep1 = ','; // separates the individual types + + static #sep2 = '|'; // separates the block types from the problem types + + // Constructs a TypesFilterData from a string as generated from toString(). + constructor(value?: string | null) { + const [blocks, problems] = (value || '').split(TypesFilterData.#sep2); + this.union({ blocks, problems }); + } + + // Serialize the TypesFilterData to a string, or undefined if isEmpty(). + toString(): string | undefined { + if (this.isEmpty()) { + return undefined; + } + return [ + [...this.#blocks].join(TypesFilterData.#sep1), + [...this.#problems].join(TypesFilterData.#sep1), + ].join(TypesFilterData.#sep2); + } + + // Returns true if there are no block or problem types. + isEmpty(): boolean { + return !(this.#blocks.size || this.#problems.size); + } + + get blocks() : Set { + return this.#blocks; + } + + get problems(): Set { + return this.#problems; + } + + clear(): TypesFilterData { + this.#blocks.clear(); + this.#problems.clear(); + return this; + } + + union({ blocks, problems }: { + blocks?: string[] | Set | string | undefined, + problems?: string[] | Set | string | undefined, + }): void { + let newBlocks: string[]; + if (!blocks) { + newBlocks = []; + } else if (typeof blocks === 'string') { + newBlocks = blocks.split(TypesFilterData.#sep1) || []; + } else { + newBlocks = [...blocks]; + } + newBlocks = newBlocks.filter(TypesFilterData.#sanitizeType); + this.#blocks = new Set([...this.#blocks, ...newBlocks]); + + let newProblems: string[]; + if (!problems) { + newProblems = []; + } else if (typeof problems === 'string') { + newProblems = problems.split(TypesFilterData.#sep1) || []; + } else { + newProblems = [...problems]; + } + newProblems = newProblems.filter(TypesFilterData.#sanitizeType); + this.#problems = new Set([...this.#problems, ...newProblems]); + } +} diff --git a/src/search-manager/index.ts b/src/search-manager/index.ts index e2d4188be..278a53715 100644 --- a/src/search-manager/index.ts +++ b/src/search-manager/index.ts @@ -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'; diff --git a/src/search-modal/SearchUI.test.tsx b/src/search-modal/SearchUI.test.tsx index 2938e9fbd..fb8269413 100644 --- a/src/search-modal/SearchUI.test.tsx +++ b/src/search-modal/SearchUI.test.tsx @@ -165,7 +165,7 @@ describe('', () => { // Enter a keyword - search for 'giraffe': fireEvent.change(getByRole('searchbox'), { target: { value: 'giraffe' } }); // Wait for the new search request to load all the results: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // And make sure the request was limited to this course: expect(fetchMock).toHaveLastFetched((_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); @@ -353,7 +353,7 @@ describe('', () => { // Enter a keyword - search for 'giraffe': fireEvent.change(getByRole('searchbox'), { target: { value: 'giraffe' } }); // Wait for the new search request to load all the results and the filter options, based on the search so far: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // And make sure the request was limited to this course: expect(fetchMock).toHaveLastFetched((_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); @@ -367,6 +367,16 @@ describe('', () => { expect(getByText(mockResultDisplayName)).toBeInTheDocument(); }); + afterEach(async () => { + // Clear any search filters applied by the previous test. + // We need to do this because search filters are stored in the URL, and so they can leak between tests. + const { queryByRole } = rendered; + const clearFilters = await queryByRole('button', { name: /clear filters/i }); + if (clearFilters) { + fireEvent.click(clearFilters); + } + }); + it('can filter results by component/XBlock type', async () => { const { getByRole, getByText } = rendered; // Now open the filters menu: @@ -379,7 +389,7 @@ describe('', () => { expect(rendered.getByRole('button', { name: /type: problem/i, hidden: true })).toBeInTheDocument(); }); // Now wait for the filter to be applied and the new results to be fetched. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // Because we're mocking the results, there's no actual changes to the mock results, // but we can verify that the filter was sent in the request expect(fetchMock).toHaveLastFetched((_url, req) => { @@ -409,10 +419,10 @@ describe('', () => { await waitFor(() => { expect(getByLabelText(checkboxLabel)).toBeInTheDocument(); }); // In addition to the checkbox, there is another button to show the child tags: expect(getByLabelText(/Expand to show child tags of "ESDC Skills and Competencies"/i)).toBeInTheDocument(); - const competentciesCheckbox = getByLabelText(checkboxLabel); - fireEvent.click(competentciesCheckbox, {}); + const competenciesCheckbox = getByLabelText(checkboxLabel); + fireEvent.click(competenciesCheckbox, {}); // Now wait for the filter to be applied and the new results to be fetched. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // Because we're mocking the results, there's no actual changes to the mock results, // but we can verify that the filter was sent in the request expect(fetchMock).toBeDone((_url, req) => { @@ -447,7 +457,7 @@ describe('', () => { const abilitiesTagFilterCheckbox = getByLabelText(childTagLabel); fireEvent.click(abilitiesTagFilterCheckbox); // Now wait for the filter to be applied and the new results to be fetched. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // Because we're mocking the results, there's no actual changes to the mock results, // but we can verify that the filter was sent in the request expect(fetchMock).toBeDone((_url, req) => { diff --git a/src/testUtils.tsx b/src/testUtils.tsx index 6cfda2c0f..b9fdf8e61 100644 --- a/src/testUtils.tsx +++ b/src/testUtils.tsx @@ -16,6 +16,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { render, type RenderResult } from '@testing-library/react'; import MockAdapter from 'axios-mock-adapter'; import { + generatePath, MemoryRouter, MemoryRouterProps, Route, @@ -94,10 +95,7 @@ const RouterAndRoute: React.FC = ({ const newRouterProps = { ...routerProps }; if (!routerProps.initialEntries) { // Substitute the params into the URL so '/library/:libraryId' becomes '/library/lib:org:123' - let pathWithParams = path; - for (const [key, value] of Object.entries(params)) { - pathWithParams = pathWithParams.replaceAll(`:${key}`, value); - } + let pathWithParams = generatePath(path, params); if (pathWithParams.endsWith('/*')) { // Some routes (that contain child routes) need to end with /* in the but not in the router pathWithParams = pathWithParams.substring(0, pathWithParams.length - 1); From e6bce560bcdf0ab49efeffa0b2f96674eb82d07c Mon Sep 17 00:00:00 2001 From: Farhaan Bukhsh Date: Fri, 10 Jan 2025 23:52:49 +0530 Subject: [PATCH 06/18] feat: Adding human readable 403 error access restricted (#1569) Updated to have human-readable forbidden error (403) --- src/course-outline/constants.js | 1 + src/course-outline/data/slice.js | 4 +- src/course-outline/data/thunk.js | 25 +---- src/course-outline/page-alerts/PageAlerts.jsx | 33 ++++++- .../page-alerts/PageAlerts.test.jsx | 99 ++++++++++++------- src/course-outline/page-alerts/messages.js | 15 +++ src/course-outline/utils/getErrorDetails.js | 24 +++++ .../utils/getErrorDetails.test.js | 36 +++++++ 8 files changed, 173 insertions(+), 64 deletions(-) create mode 100644 src/course-outline/utils/getErrorDetails.js create mode 100644 src/course-outline/utils/getErrorDetails.test.js diff --git a/src/course-outline/constants.js b/src/course-outline/constants.js index 4430694a7..eda0522a0 100644 --- a/src/course-outline/constants.js +++ b/src/course-outline/constants.js @@ -87,4 +87,5 @@ export const API_ERROR_TYPES = /** @type {const} */ ({ networkError: 'networkError', serverError: 'serverError', unknown: 'unknown', + forbidden: 'forbidden', }); diff --git a/src/course-outline/data/slice.js b/src/course-outline/data/slice.js index 0bd146f95..027555526 100644 --- a/src/course-outline/data/slice.js +++ b/src/course-outline/data/slice.js @@ -104,7 +104,7 @@ const slice = createSlice({ ...payload, }; }, - fetchStatusBarSelPacedSuccess: (state, { payload }) => { + fetchStatusBarSelfPacedSuccess: (state, { payload }) => { state.statusBarData.isSelfPaced = payload.isSelfPaced; }, updateSavingStatus: (state, { payload }) => { @@ -206,7 +206,7 @@ export const { updateStatusBar, updateCourseActions, fetchStatusBarChecklistSuccess, - fetchStatusBarSelPacedSuccess, + fetchStatusBarSelfPacedSuccess, updateFetchSectionLoadingStatus, updateCourseLaunchQueryStatus, updateSavingStatus, diff --git a/src/course-outline/data/thunk.js b/src/course-outline/data/thunk.js index c555fcb97..457d03911 100644 --- a/src/course-outline/data/thunk.js +++ b/src/course-outline/data/thunk.js @@ -1,7 +1,7 @@ import { RequestStatus } from '../../data/constants'; import { updateClipboardData } from '../../generic/data/slice'; import { NOTIFICATION_MESSAGES } from '../../constants'; -import { API_ERROR_TYPES, COURSE_BLOCK_NAMES } from '../constants'; +import { COURSE_BLOCK_NAMES } from '../constants'; import { hideProcessingNotification, showProcessingNotification, @@ -10,6 +10,7 @@ import { getCourseBestPracticesChecklist, getCourseLaunchChecklist, } from '../utils/getChecklistForStatusBar'; +import { getErrorDetails } from '../utils/getErrorDetails'; import { addNewCourseItem, deleteCourseItem, @@ -41,7 +42,7 @@ import { updateStatusBar, updateCourseActions, fetchStatusBarChecklistSuccess, - fetchStatusBarSelPacedSuccess, + fetchStatusBarSelfPacedSuccess, updateSavingStatus, updateSectionList, updateFetchSectionLoadingStatus, @@ -54,24 +55,6 @@ import { updateCourseLaunchQueryStatus, } from './slice'; -const getErrorDetails = (error, dismissible = true) => { - const errorInfo = { dismissible }; - if (error.response?.data) { - const { data } = error.response; - if ((typeof data === 'string' && !data.includes('')) || typeof data === 'object') { - errorInfo.data = JSON.stringify(data); - } - errorInfo.status = error.response.status; - errorInfo.type = API_ERROR_TYPES.serverError; - } else if (error.request) { - errorInfo.type = API_ERROR_TYPES.networkError; - } else { - errorInfo.type = API_ERROR_TYPES.unknown; - errorInfo.data = error.message; - } - return errorInfo; -}; - export function fetchCourseOutlineIndexQuery(courseId) { return async (dispatch) => { dispatch(updateOutlineIndexLoadingStatus({ status: RequestStatus.IN_PROGRESS })); @@ -125,7 +108,7 @@ export function fetchCourseLaunchQuery({ const data = await getCourseLaunch({ courseId, gradedOnly, validateOras, all, }); - dispatch(fetchStatusBarSelPacedSuccess({ isSelfPaced: data.isSelfPaced })); + dispatch(fetchStatusBarSelfPacedSuccess({ isSelfPaced: data.isSelfPaced })); dispatch(fetchStatusBarChecklistSuccess(getCourseLaunchChecklist(data))); dispatch(updateCourseLaunchQueryStatus({ status: RequestStatus.SUCCESSFUL })); diff --git a/src/course-outline/page-alerts/PageAlerts.jsx b/src/course-outline/page-alerts/PageAlerts.jsx index 4b1299639..9d6eefb21 100644 --- a/src/course-outline/page-alerts/PageAlerts.jsx +++ b/src/course-outline/page-alerts/PageAlerts.jsx @@ -343,13 +343,38 @@ const PageAlerts = ({ const renderApiErrors = () => { let errorList = Object.entries(errors).filter(obj => obj[1] !== null).map(([k, v]) => { switch (v.type) { - case API_ERROR_TYPES.serverError: + case API_ERROR_TYPES.forbidden: { + const description = intl.formatMessage(messages.forbiddenAlertBody, { + LMS: ( + + {intl.formatMessage(messages.forbiddenAlertLmsUrl)} + + ), + }); return { key: k, - desc: v.data || intl.formatMessage(messages.serverErrorAlertBody), + desc: description, + title: intl.formatMessage(messages.forbiddenAlert), + dismissible: v.dismissible, + }; + } + case API_ERROR_TYPES.serverError: { + const description = ( + + {v.data || intl.formatMessage(messages.serverErrorAlertBody)} + + ); + return { + key: k, + desc: description, title: intl.formatMessage(messages.serverErrorAlert), dismissible: v.dismissible, }; + } case API_ERROR_TYPES.networkError: return { key: k, @@ -378,7 +403,7 @@ const PageAlerts = ({ dismissError={() => dispatch(dismissError(msgObj.key))} > {msgObj.title} - {msgObj.desc && {msgObj.desc}} + {msgObj.desc} ) : ( {msgObj.title} - {msgObj.desc && {msgObj.desc}} + {msgObj.desc} ) )) diff --git a/src/course-outline/page-alerts/PageAlerts.test.jsx b/src/course-outline/page-alerts/PageAlerts.test.jsx index 6d646f7cb..487a7e7ac 100644 --- a/src/course-outline/page-alerts/PageAlerts.test.jsx +++ b/src/course-outline/page-alerts/PageAlerts.test.jsx @@ -71,19 +71,19 @@ describe('', () => { }); it('renders null when no alerts are present', () => { - const { queryByTestId } = renderComponent(); - expect(queryByTestId('browser-router')).toBeEmptyDOMElement(); + renderComponent(); + expect(screen.queryByTestId('browser-router')).toBeEmptyDOMElement(); }); it('renders configuration alerts', async () => { - const { queryByText } = renderComponent({ + renderComponent({ ...pageAlertsData, notificationDismissUrl: 'some-url', handleDismissNotification, }); - expect(queryByText(messages.configurationErrorTitle.defaultMessage)).toBeInTheDocument(); - const dismissBtn = queryByText('Dismiss'); + expect(screen.queryByText(messages.configurationErrorTitle.defaultMessage)).toBeInTheDocument(); + const dismissBtn = screen.queryByText('Dismiss'); await act(async () => fireEvent.click(dismissBtn)); expect(handleDismissNotification).toBeCalled(); @@ -117,7 +117,7 @@ describe('', () => { }); it('renders deprecation warning alerts', async () => { - const { queryByText } = renderComponent({ + renderComponent({ ...pageAlertsData, deprecatedBlocksInfo: { blocks: [['url1', 'block1'], ['url2']], @@ -126,20 +126,20 @@ describe('', () => { }, }); - expect(queryByText(messages.deprecationWarningTitle.defaultMessage)).toBeInTheDocument(); - expect(queryByText(messages.deprecationWarningBlocksText.defaultMessage)).toBeInTheDocument(); - expect(queryByText('block1')).toHaveAttribute('href', 'url1'); - expect(queryByText(messages.deprecatedComponentName.defaultMessage)).toHaveAttribute('href', 'url2'); + expect(screen.queryByText(messages.deprecationWarningTitle.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.deprecationWarningBlocksText.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText('block1')).toHaveAttribute('href', 'url1'); + expect(screen.queryByText(messages.deprecatedComponentName.defaultMessage)).toHaveAttribute('href', 'url2'); - const feedbackLink = queryByText(messages.advancedSettingLinkText.defaultMessage); + const feedbackLink = screen.queryByText(messages.advancedSettingLinkText.defaultMessage); expect(feedbackLink).toBeInTheDocument(); expect(feedbackLink).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}/some-url`); - expect(queryByText('lti')).toBeInTheDocument(); - expect(queryByText('video')).toBeInTheDocument(); + expect(screen.queryByText('lti')).toBeInTheDocument(); + expect(screen.queryByText('video')).toBeInTheDocument(); }); it('renders proctoring alerts with mfe settings link', async () => { - const { queryByText } = renderComponent({ + renderComponent({ ...pageAlertsData, mfeProctoredExamSettingsUrl: 'mfe-url', proctoringErrors: [ @@ -148,15 +148,15 @@ describe('', () => { ], }); - expect(queryByText('error 1')).toBeInTheDocument(); - expect(queryByText('error 2')).toBeInTheDocument(); - expect(queryByText('message 1')).toBeInTheDocument(); - expect(queryByText('message 2')).toBeInTheDocument(); - expect(queryByText(messages.proctoredSettingsLinkText.defaultMessage)).toHaveAttribute('href', 'mfe-url'); + expect(screen.queryByText('error 1')).toBeInTheDocument(); + expect(screen.queryByText('error 2')).toBeInTheDocument(); + expect(screen.queryByText('message 1')).toBeInTheDocument(); + expect(screen.queryByText('message 2')).toBeInTheDocument(); + expect(screen.queryByText(messages.proctoredSettingsLinkText.defaultMessage)).toHaveAttribute('href', 'mfe-url'); }); it('renders proctoring alerts without mfe settings link', async () => { - const { queryByText } = renderComponent({ + renderComponent({ ...pageAlertsData, advanceSettingsUrl: '/some-url', proctoringErrors: [ @@ -165,11 +165,11 @@ describe('', () => { ], }); - expect(queryByText('error 1')).toBeInTheDocument(); - expect(queryByText('error 2')).toBeInTheDocument(); - expect(queryByText('message 1')).toBeInTheDocument(); - expect(queryByText('message 2')).toBeInTheDocument(); - expect(queryByText(messages.advancedSettingLinkText.defaultMessage)).toHaveAttribute( + expect(screen.queryByText('error 1')).toBeInTheDocument(); + expect(screen.queryByText('error 2')).toBeInTheDocument(); + expect(screen.queryByText('message 1')).toBeInTheDocument(); + expect(screen.queryByText('message 2')).toBeInTheDocument(); + expect(screen.queryByText(messages.advancedSettingLinkText.defaultMessage)).toHaveAttribute( 'href', `${getConfig().STUDIO_BASE_URL}/some-url`, ); @@ -181,10 +181,10 @@ describe('', () => { conflictingFiles: [], errorFiles: ['error.css'], }); - const { queryByText } = renderComponent(); - expect(queryByText(messages.newFileAlertTitle.defaultMessage)).toBeInTheDocument(); - expect(queryByText(messages.errorFileAlertTitle.defaultMessage)).toBeInTheDocument(); - expect(queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute( + renderComponent(); + expect(screen.queryByText(messages.newFileAlertTitle.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.errorFileAlertTitle.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute( 'href', `${getConfig().STUDIO_BASE_URL}/assets/course-id`, ); @@ -196,16 +196,16 @@ describe('', () => { conflictingFiles: ['some.css', 'some.js'], errorFiles: [], }); - const { queryByText } = renderComponent(); - expect(queryByText(messages.conflictingFileAlertTitle.defaultMessage)).toBeInTheDocument(); - expect(queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute( + renderComponent(); + expect(screen.queryByText(messages.conflictingFileAlertTitle.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute( 'href', `${getConfig().STUDIO_BASE_URL}/assets/course-id`, ); }); it('renders api error alerts', async () => { - const { queryByText } = renderComponent({ + renderComponent({ ...pageAlertsData, errors: { outlineIndexApi: { data: 'some error', status: 400, type: API_ERROR_TYPES.serverError }, @@ -213,9 +213,34 @@ describe('', () => { reindexApi: { type: API_ERROR_TYPES.unknown, data: 'some unknown error' }, }, }); - expect(queryByText(messages.networkErrorAlert.defaultMessage)).toBeInTheDocument(); - expect(queryByText(messages.serverErrorAlert.defaultMessage)).toBeInTheDocument(); - expect(queryByText('some error')).toBeInTheDocument(); - expect(queryByText('some unknown error')).toBeInTheDocument(); + expect(screen.queryByText(messages.networkErrorAlert.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.serverErrorAlert.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText('some error')).toBeInTheDocument(); + expect(screen.queryByText('some unknown error')).toBeInTheDocument(); + }); + + it('renders forbidden api error alerts', async () => { + renderComponent({ + ...pageAlertsData, + errors: { + outlineIndexApi: { + data: 'some error', status: 403, type: API_ERROR_TYPES.forbidden, dismissable: false, + }, + }, + }); + expect(screen.queryByText(messages.forbiddenAlert.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.forbiddenAlertBody.defaultMessage)).toBeInTheDocument(); + }); + + it('renders api error alerts when status is not 403', async () => { + renderComponent({ + ...pageAlertsData, + errors: { + outlineIndexApi: { + data: 'some error', status: 500, type: API_ERROR_TYPES.serverError, dismissable: true, + }, + }, + }); + expect(screen.queryByText('some error')).toBeInTheDocument(); }); }); diff --git a/src/course-outline/page-alerts/messages.js b/src/course-outline/page-alerts/messages.js index f9638398d..9aa6756a7 100644 --- a/src/course-outline/page-alerts/messages.js +++ b/src/course-outline/page-alerts/messages.js @@ -121,6 +121,21 @@ const messages = defineMessages({ defaultMessage: 'Network error', description: 'Generic network error alert.', }, + forbiddenAlert: { + id: 'course-authoring.course-outline.page-alert.forbidden.title', + defaultMessage: 'Access Restricted', + description: 'Forbidden(403) alert title', + }, + forbiddenAlertBody: { + id: 'course-authoring.course-outline.page-alert.forbidden.body', + defaultMessage: 'It looks like you’re trying to access a page you don’t have permission to view. Contact your admin if you think this is a mistake, or head back to the {LMS}.', + description: 'Forbidden(403) alert body', + }, + forbiddenAlertLmsUrl: { + id: 'course-authoring.course-outline.page-alert.lms', + defaultMessage: 'LMS', + description: 'LMS base redirection url', + }, }); export default messages; diff --git a/src/course-outline/utils/getErrorDetails.js b/src/course-outline/utils/getErrorDetails.js new file mode 100644 index 000000000..fa7e11145 --- /dev/null +++ b/src/course-outline/utils/getErrorDetails.js @@ -0,0 +1,24 @@ +import { API_ERROR_TYPES } from '../constants'; + +export const getErrorDetails = (error, dismissible = true) => { + const errorInfo = { dismissible }; + if (error.response?.status === 403) { + // For 403 status the error shouldn't be dismissible + errorInfo.dismissible = false; + errorInfo.type = API_ERROR_TYPES.forbidden; + errorInfo.status = error.response.status; + } else if (error.response?.data) { + const { data } = error.response; + if ((typeof data === 'string' && !data.includes('')) || typeof data === 'object') { + errorInfo.data = JSON.stringify(data); + } + errorInfo.status = error.response.status; + errorInfo.type = API_ERROR_TYPES.serverError; + } else if (error.request) { + errorInfo.type = API_ERROR_TYPES.networkError; + } else { + errorInfo.type = API_ERROR_TYPES.unknown; + errorInfo.data = error.message; + } + return errorInfo; +}; diff --git a/src/course-outline/utils/getErrorDetails.test.js b/src/course-outline/utils/getErrorDetails.test.js new file mode 100644 index 000000000..579485933 --- /dev/null +++ b/src/course-outline/utils/getErrorDetails.test.js @@ -0,0 +1,36 @@ +import { getErrorDetails } from './getErrorDetails'; +import { API_ERROR_TYPES } from '../constants'; + +describe('getErrorDetails', () => { + it('should handle 403 status error', () => { + const error = { response: { data: 'some data', status: 403 } }; + const result = getErrorDetails(error); + expect(result).toEqual({ dismissible: false, status: 403, type: API_ERROR_TYPES.forbidden }); + }); + + it('should handle response with data', () => { + const error = { response: { data: 'some data', status: 500 } }; + const result = getErrorDetails(error); + expect(result).toEqual({ + dismissible: true, data: '"some data"', status: 500, type: API_ERROR_TYPES.serverError, + }); + }); + + it('should handle response with HTML data', () => { + const error = { response: { data: 'error', status: 500 } }; + const result = getErrorDetails(error); + expect(result).toEqual({ dismissible: true, status: 500, type: API_ERROR_TYPES.serverError }); + }); + + it('should handle request error', () => { + const error = { request: {} }; + const result = getErrorDetails(error); + expect(result).toEqual({ dismissible: true, type: API_ERROR_TYPES.networkError }); + }); + + it('should handle unknown error', () => { + const error = { message: 'Unknown error' }; + const result = getErrorDetails(error); + expect(result).toEqual({ dismissible: true, type: API_ERROR_TYPES.unknown, data: 'Unknown error' }); + }); +}); From 8385c4e8edfa42d956cf59c5e5935afb6911e0ef Mon Sep 17 00:00:00 2001 From: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com> Date: Mon, 13 Jan 2025 11:44:25 -0500 Subject: [PATCH 07/18] Feat course optimizer page (#1533) Course Optimizer is a feature approved by the Openedx community that adds a "Course Optimizer" page to studio where users can run a scan of a course for broken links - links that point to pages that have a 404. Depends on backend: openedx/edx-platform#35887 - test together. This also requires adding a nav menu item to edx-platform legacy studio. That should be implemented before enabling the waffle flag on prod. Links: - [Internal JIRA ticket](https://2u-internal.atlassian.net/browse/TNL-11809) - [Course Optimizer Discovery](https://2u-internal.atlassian.net/wiki/spaces/TNL/pages/1426587703/TNL-11744+Course+Optimizer+Discovery) - [Openedx community proposal](https://github.com/openedx/platform-roadmap/issues/388) --- src/CourseAuthoringRoutes.jsx | 5 + src/data/constants.js | 7 + src/header/hooks.js | 4 + src/header/hooks.test.js | 29 ++- src/header/messages.js | 5 + src/index.scss | 1 + .../CourseOptimizerPage.test.js | 193 ++++++++++++++++++ src/optimizer-page/CourseOptimizerPage.tsx | 176 ++++++++++++++++ src/optimizer-page/data/api.test.js | 34 +++ src/optimizer-page/data/api.ts | 26 +++ src/optimizer-page/data/constants.ts | 40 ++++ src/optimizer-page/data/selectors.ts | 12 ++ src/optimizer-page/data/slice.test.ts | 111 ++++++++++ src/optimizer-page/data/slice.ts | 91 +++++++++ src/optimizer-page/data/thunks.test.js | 193 ++++++++++++++++++ src/optimizer-page/data/thunks.ts | 81 ++++++++ src/optimizer-page/messages.js | 71 +++++++ src/optimizer-page/mocks/mockApiResponse.js | 106 ++++++++++ .../scan-results/BrokenLinkTable.tsx | 117 +++++++++++ .../scan-results/LockedInfoIcon.jsx | 30 +++ .../scan-results/ScanResults.scss | 110 ++++++++++ .../scan-results/ScanResults.tsx | 89 ++++++++ .../scan-results/SectionCollapsible.tsx | 53 +++++ src/optimizer-page/scan-results/index.js | 3 + src/optimizer-page/scan-results/messages.js | 46 +++++ src/optimizer-page/types.ts | 27 +++ src/optimizer-page/utils.test.js | 44 ++++ src/optimizer-page/utils.ts | 26 +++ src/store.js | 2 + 29 files changed, 1724 insertions(+), 8 deletions(-) create mode 100644 src/optimizer-page/CourseOptimizerPage.test.js create mode 100644 src/optimizer-page/CourseOptimizerPage.tsx create mode 100644 src/optimizer-page/data/api.test.js create mode 100644 src/optimizer-page/data/api.ts create mode 100644 src/optimizer-page/data/constants.ts create mode 100644 src/optimizer-page/data/selectors.ts create mode 100644 src/optimizer-page/data/slice.test.ts create mode 100644 src/optimizer-page/data/slice.ts create mode 100644 src/optimizer-page/data/thunks.test.js create mode 100644 src/optimizer-page/data/thunks.ts create mode 100644 src/optimizer-page/messages.js create mode 100644 src/optimizer-page/mocks/mockApiResponse.js create mode 100644 src/optimizer-page/scan-results/BrokenLinkTable.tsx create mode 100644 src/optimizer-page/scan-results/LockedInfoIcon.jsx create mode 100644 src/optimizer-page/scan-results/ScanResults.scss create mode 100644 src/optimizer-page/scan-results/ScanResults.tsx create mode 100644 src/optimizer-page/scan-results/SectionCollapsible.tsx create mode 100644 src/optimizer-page/scan-results/index.js create mode 100644 src/optimizer-page/scan-results/messages.js create mode 100644 src/optimizer-page/types.ts create mode 100644 src/optimizer-page/utils.test.js create mode 100644 src/optimizer-page/utils.ts diff --git a/src/CourseAuthoringRoutes.jsx b/src/CourseAuthoringRoutes.jsx index ded2f07ea..4bef34268 100644 --- a/src/CourseAuthoringRoutes.jsx +++ b/src/CourseAuthoringRoutes.jsx @@ -20,6 +20,7 @@ import { CourseUpdates } from './course-updates'; import { CourseUnit, IframeProvider } from './course-unit'; import { Certificates } from './certificates'; import CourseExportPage from './export-page/CourseExportPage'; +import CourseOptimizerPage from './optimizer-page/CourseOptimizerPage'; import CourseImportPage from './import-page/CourseImportPage'; import { DECODED_ROUTES } from './constants'; import CourseChecklist from './course-checklist'; @@ -118,6 +119,10 @@ const CourseAuthoringRoutes = () => { path="export" element={} /> + } + /> } diff --git a/src/data/constants.js b/src/data/constants.js index 2c63f2628..7aa59f063 100644 --- a/src/data/constants.js +++ b/src/data/constants.js @@ -15,6 +15,13 @@ export const RequestStatus = /** @type {const} */ ({ NOT_FOUND: 'not-found', }); +export const RequestFailureStatuses = [ + RequestStatus.FAILED, + RequestStatus.DENIED, + RequestStatus.PARTIAL_FAILURE, + RequestStatus.NOT_FOUND, +]; + /** * Team sizes enum * @enum diff --git a/src/header/hooks.js b/src/header/hooks.js index 6758fbc27..d1e5e3a44 100644 --- a/src/header/hooks.js +++ b/src/header/hooks.js @@ -103,6 +103,10 @@ export const useToolsMenuItems = courseId => { href: `/course/${courseId}/checklists`, title: intl.formatMessage(messages['header.links.checklists']), }, + ...(waffleFlags.enableCourseOptimizer ? [{ + href: `/course/${courseId}/optimizer`, + title: intl.formatMessage(messages['header.links.optimizer']), + }] : []), ]; return items; }; diff --git a/src/header/hooks.test.js b/src/header/hooks.test.js index 9b9f1cbae..aa0829b33 100644 --- a/src/header/hooks.test.js +++ b/src/header/hooks.test.js @@ -1,6 +1,7 @@ import { useSelector } from 'react-redux'; import { getConfig, setConfig } from '@edx/frontend-platform'; import { renderHook } from '@testing-library/react-hooks'; +import messages from './messages'; import { useContentMenuItems, useToolsMenuItems, useSettingMenuItems } from './hooks'; jest.mock('@edx/frontend-platform/i18n', () => ({ @@ -17,7 +18,7 @@ jest.mock('react-redux', () => ({ describe('header utils', () => { describe('getContentMenuItems', () => { - it('should include Video Uploads option', () => { + it('when video upload page enabled should include Video Uploads option', () => { setConfig({ ...getConfig(), ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN: 'true', @@ -25,7 +26,7 @@ describe('header utils', () => { const actualItems = renderHook(() => useContentMenuItems('course-123')).result.current; expect(actualItems).toHaveLength(5); }); - it('should not include Video Uploads option', () => { + it('when video upload page disabled should not include Video Uploads option', () => { setConfig({ ...getConfig(), ENABLE_VIDEO_UPLOAD_PAGE_LINK_IN_CONTENT_DROPDOWN: 'false', @@ -38,7 +39,7 @@ describe('header utils', () => { describe('getSettingsMenuitems', () => { useSelector.mockReturnValue({ canAccessAdvancedSettings: true }); - it('should include certificates option', () => { + it('when certificate page enabled should include certificates option', () => { setConfig({ ...getConfig(), ENABLE_CERTIFICATE_PAGE: 'true', @@ -46,7 +47,7 @@ describe('header utils', () => { const actualItems = renderHook(() => useSettingMenuItems('course-123')).result.current; expect(actualItems).toHaveLength(6); }); - it('should not include certificates option', () => { + it('when certificate page disabled should not include certificates option', () => { setConfig({ ...getConfig(), ENABLE_CERTIFICATE_PAGE: 'false', @@ -54,11 +55,11 @@ describe('header utils', () => { const actualItems = renderHook(() => useSettingMenuItems('course-123')).result.current; expect(actualItems).toHaveLength(5); }); - it('should include advanced settings option', () => { + it('when user has access to advanced settings should include advanced settings option', () => { const actualItemsTitle = renderHook(() => useSettingMenuItems('course-123')).result.current.map((item) => item.title); expect(actualItemsTitle).toContain('Advanced Settings'); }); - it('should not include advanced settings option', () => { + it('when user has no access to advanced settings should not include advanced settings option', () => { useSelector.mockReturnValue({ canAccessAdvancedSettings: false }); const actualItemsTitle = renderHook(() => useSettingMenuItems('course-123')).result.current.map((item) => item.title); expect(actualItemsTitle).not.toContain('Advanced Settings'); @@ -66,7 +67,7 @@ describe('header utils', () => { }); describe('getToolsMenuItems', () => { - it('should include export tags option', () => { + it('when tags enabled should include export tags option', () => { setConfig({ ...getConfig(), ENABLE_TAGGING_TAXONOMY_PAGES: 'true', @@ -79,7 +80,7 @@ describe('header utils', () => { 'Checklists', ]); }); - it('should not include export tags option', () => { + it('when tags disabled should not include export tags option', () => { setConfig({ ...getConfig(), ENABLE_TAGGING_TAXONOMY_PAGES: 'false', @@ -91,5 +92,17 @@ describe('header utils', () => { 'Checklists', ]); }); + + it('when course optimizer enabled should include optimizer option', () => { + useSelector.mockReturnValue({ enableCourseOptimizer: true }); + const actualItemsTitle = renderHook(() => useToolsMenuItems('course-123')).result.current.map((item) => item.title); + expect(actualItemsTitle).toContain(messages['header.links.optimizer'].defaultMessage); + }); + + it('when course optimizer disabled should not include optimizer option', () => { + useSelector.mockReturnValue({ enableCourseOptimizer: false }); + const actualItemsTitle = renderHook(() => useToolsMenuItems('course-123')).result.current.map((item) => item.title); + expect(actualItemsTitle).not.toContain(messages['header.links.optimizer'].defaultMessage); + }); }); }); diff --git a/src/header/messages.js b/src/header/messages.js index 31d5f32fa..6c578790f 100644 --- a/src/header/messages.js +++ b/src/header/messages.js @@ -96,6 +96,11 @@ const messages = defineMessages({ defaultMessage: 'Export Course', description: 'Link to Studio Export page', }, + 'header.links.optimizer': { + id: 'header.links.optimizer', + defaultMessage: 'Optimize Course', + description: 'Fix broken links and other issues in your course', + }, 'header.links.exportTags': { id: 'header.links.exportTags', defaultMessage: 'Export Tags', diff --git a/src/index.scss b/src/index.scss index 69f9b8b34..31ffa2de8 100644 --- a/src/index.scss +++ b/src/index.scss @@ -31,6 +31,7 @@ @import "search-manager"; @import "certificates/scss/Certificates"; @import "group-configurations/GroupConfigurations"; +@import "optimizer-page/scan-results/ScanResults"; // To apply the glow effect to the selected Section/Subsection, in the Course Outline div.row:has(> div > div.highlight) { diff --git a/src/optimizer-page/CourseOptimizerPage.test.js b/src/optimizer-page/CourseOptimizerPage.test.js new file mode 100644 index 000000000..5f69e1840 --- /dev/null +++ b/src/optimizer-page/CourseOptimizerPage.test.js @@ -0,0 +1,193 @@ +/* eslint-disable @typescript-eslint/no-shadow */ +/* eslint-disable react/jsx-filename-extension */ +import { + fireEvent, render, waitFor, screen, +} from '@testing-library/react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { AppProvider } from '@edx/frontend-platform/react'; +import { initializeMockApp } from '@edx/frontend-platform'; + +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import MockAdapter from 'axios-mock-adapter'; +import initializeStore from '../store'; +import messages from './messages'; +import generalMessages from '../messages'; +import scanResultsMessages from './scan-results/messages'; +import CourseOptimizerPage, { pollLinkCheckDuringScan } from './CourseOptimizerPage'; +import { postLinkCheckCourseApiUrl, getLinkCheckStatusApiUrl } from './data/api'; +import mockApiResponse from './mocks/mockApiResponse'; +import * as thunks from './data/thunks'; + +let store; +let axiosMock; +const courseId = '123'; +const courseName = 'About Node JS'; + +jest.mock('../generic/model-store', () => ({ + useModel: jest.fn().mockReturnValue({ + name: courseName, + }), +})); + +const OptimizerPage = () => ( + + + + + +); + +describe('CourseOptimizerPage', () => { + describe('pollLinkCheckDuringScan', () => { + let mockFetchLinkCheckStatus; + beforeEach(() => { + mockFetchLinkCheckStatus = jest.fn(); + jest.spyOn(thunks, 'fetchLinkCheckStatus').mockImplementation(mockFetchLinkCheckStatus); + jest.useFakeTimers(); + jest.spyOn(global, 'setInterval').mockImplementation((cb) => { cb(); return true; }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it('should start polling if linkCheckInProgress has never been started (is null)', () => { + const linkCheckInProgress = null; + const interval = { current: null }; + const dispatch = jest.fn(); + const courseId = 'course-123'; + pollLinkCheckDuringScan(linkCheckInProgress, interval, dispatch, courseId); + expect(interval.current).toBeTruthy(); + expect(mockFetchLinkCheckStatus).toHaveBeenCalled(); + }); + + it('should start polling if link check is in progress', () => { + const linkCheckInProgress = true; + const interval = { current: null }; + const dispatch = jest.fn(); + const courseId = 'course-123'; + pollLinkCheckDuringScan(linkCheckInProgress, interval, dispatch, courseId); + expect(interval.current).toBeTruthy(); + }); + it('should not start polling if link check is not in progress', () => { + const linkCheckInProgress = false; + const interval = { current: null }; + const dispatch = jest.fn(); + const courseId = 'course-123'; + pollLinkCheckDuringScan(linkCheckInProgress, interval, dispatch, courseId); + expect(interval.current).toBeFalsy(); + }); + it('should clear the interval if link check is finished', () => { + const linkCheckInProgress = false; + const interval = { current: 1 }; + const dispatch = jest.fn(); + const courseId = 'course-123'; + pollLinkCheckDuringScan(linkCheckInProgress, interval, dispatch, courseId); + expect(interval.current).toBeUndefined(); + }); + }); + + describe('CourseOptimizerPage component', () => { + beforeEach(() => { + jest.useRealTimers(); + jest.clearAllMocks(); + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + store = initializeStore(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + axiosMock + .onPost(postLinkCheckCourseApiUrl(courseId)) + .reply(200, { LinkCheckStatus: 'In-Progress' }); + axiosMock + .onGet(getLinkCheckStatusApiUrl(courseId)) + .reply(200, mockApiResponse); + }); + + it('should render the component', () => { + const { getByText, queryByText } = render(); + expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(messages.buttonTitle.defaultMessage)).toBeInTheDocument(); + expect(queryByText(messages.preparingStepTitle)).not.toBeInTheDocument(); + }); + + it('should start scan after clicking the scan button', async () => { + const { getByText } = render(); + expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + await waitFor(() => { + expect(getByText(messages.preparingStepTitle.defaultMessage)).toBeInTheDocument(); + }); + }); + + it('should list broken links results', async () => { + const { + getByText, queryAllByText, getAllByText, container, + } = render(); + expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + await waitFor(() => { + expect(getByText('5 broken links')).toBeInTheDocument(); + expect(getByText('5 locked links')).toBeInTheDocument(); + }); + const collapsibleTrigger = container.querySelector('.collapsible-trigger'); + expect(collapsibleTrigger).toBeInTheDocument(); + fireEvent.click(collapsibleTrigger); + await waitFor(() => { + expect(getAllByText(scanResultsMessages.brokenLinkStatus.defaultMessage)[0]).toBeInTheDocument(); + expect(queryAllByText(scanResultsMessages.lockedLinkStatus.defaultMessage)[0]).toBeInTheDocument(); + }); + }); + + it('should not list locked links results when show locked links is unchecked', async () => { + const { + getByText, getAllByText, getByLabelText, queryAllByText, queryByText, container, + } = render(); + expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + await waitFor(() => { + expect(getByText('5 broken links')).toBeInTheDocument(); + }); + fireEvent.click(getByLabelText(scanResultsMessages.lockedCheckboxLabel.defaultMessage)); + const collapsibleTrigger = container.querySelector('.collapsible-trigger'); + expect(collapsibleTrigger).toBeInTheDocument(); + fireEvent.click(collapsibleTrigger); + await waitFor(() => { + expect(queryByText('5 locked links')).not.toBeInTheDocument(); + expect(getAllByText(scanResultsMessages.brokenLinkStatus.defaultMessage)[0]).toBeInTheDocument(); + expect(queryAllByText(scanResultsMessages.lockedLinkStatus.defaultMessage)?.[0]).toBeUndefined(); + }); + }); + + it('should show no broken links found message', async () => { + axiosMock + .onGet(getLinkCheckStatusApiUrl(courseId)) + .reply(200, { LinkCheckStatus: 'Succeeded' }); + const { getByText } = render(); + expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + await waitFor(() => { + expect(getByText(scanResultsMessages.noBrokenLinksCard.defaultMessage)).toBeInTheDocument(); + }); + }); + + it('should show error message if request does not go through', async () => { + axiosMock + .onPost(postLinkCheckCourseApiUrl(courseId)) + .reply(500); + render(); + expect(screen.getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); + fireEvent.click(screen.getByText(messages.buttonTitle.defaultMessage)); + await waitFor(() => { + expect(screen.getByText(generalMessages.supportText.defaultMessage)).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/src/optimizer-page/CourseOptimizerPage.tsx b/src/optimizer-page/CourseOptimizerPage.tsx new file mode 100644 index 000000000..f760e45fc --- /dev/null +++ b/src/optimizer-page/CourseOptimizerPage.tsx @@ -0,0 +1,176 @@ +/* eslint-disable no-param-reassign */ +import { + useEffect, useRef, FC, MutableRefObject, +} from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + Container, Layout, Button, Card, +} from '@openedx/paragon'; +import { Search as SearchIcon } from '@openedx/paragon/icons'; +import { Helmet } from 'react-helmet'; + +import CourseStepper from '../generic/course-stepper'; +import ConnectionErrorAlert from '../generic/ConnectionErrorAlert'; +import SubHeader from '../generic/sub-header/SubHeader'; +import { RequestFailureStatuses } from '../data/constants'; +import messages from './messages'; +import { + getCurrentStage, getError, getLinkCheckInProgress, getLoadingStatus, getSavingStatus, getLinkCheckResult, + getLastScannedAt, +} from './data/selectors'; +import { startLinkCheck, fetchLinkCheckStatus } from './data/thunks'; +import { useModel } from '../generic/model-store'; +import ScanResults from './scan-results'; + +const pollLinkCheckStatus = (dispatch: any, courseId: string, delay: number): number => { + const interval = setInterval(() => { + dispatch(fetchLinkCheckStatus(courseId)); + }, delay); + return interval as unknown as number; +}; + +export function pollLinkCheckDuringScan( + linkCheckInProgress: boolean | null, + interval: MutableRefObject, + dispatch: any, + courseId: string, +) { + if (linkCheckInProgress === null || linkCheckInProgress) { + clearInterval(interval.current as number | undefined); + interval.current = pollLinkCheckStatus(dispatch, courseId, 2000); + } else if (interval.current) { + clearInterval(interval.current); + interval.current = undefined; + } +} + +const CourseOptimizerPage: FC<{ courseId: string }> = ({ courseId }) => { + const dispatch = useDispatch(); + const linkCheckInProgress = useSelector(getLinkCheckInProgress); + const loadingStatus = useSelector(getLoadingStatus); + const savingStatus = useSelector(getSavingStatus); + const currentStage = useSelector(getCurrentStage); + const linkCheckResult = useSelector(getLinkCheckResult); + const lastScannedAt = useSelector(getLastScannedAt); + const { msg: errorMessage } = useSelector(getError); + const isShowExportButton = !linkCheckInProgress || errorMessage; + const isLoadingDenied = (RequestFailureStatuses as string[]).includes(loadingStatus); + const isSavingDenied = (RequestFailureStatuses as string[]).includes(savingStatus); + const interval = useRef(undefined); + const courseDetails = useModel('courseDetails', courseId); + const linkCheckPresent = !!currentStage; + const intl = useIntl(); + + const courseStepperSteps = [ + { + title: intl.formatMessage(messages.preparingStepTitle), + description: intl.formatMessage(messages.preparingStepDescription), + key: 'course-step-preparing', + }, + { + title: intl.formatMessage(messages.scanningStepTitle), + description: intl.formatMessage(messages.scanningStepDescription), + key: 'course-step-scanning', + }, + { + title: intl.formatMessage(messages.successStepTitle), + description: intl.formatMessage(messages.successStepDescription), + key: 'course-step-success', + }, + ]; + + useEffect(() => { + // when first entering the page, fetch any existing scan results + dispatch(fetchLinkCheckStatus(courseId)); + }, []); + + useEffect(() => { + // when a scan starts, start polling for the results as long as the scan status fetched + // signals it is still in progress + pollLinkCheckDuringScan(linkCheckInProgress, interval, dispatch, courseId); + + return () => { + if (interval.current) { clearInterval(interval.current); } + }; + }, [linkCheckInProgress, linkCheckResult]); + + if (isLoadingDenied || isSavingDenied) { + if (interval.current) { clearInterval(interval.current); } + + return ( + // + + // + ); + } + + return ( + <> + + + {intl.formatMessage(messages.pageTitle, { + headingTitle: intl.formatMessage(messages.headingTitle), + courseName: courseDetails?.name, + siteName: process.env.SITE_NAME, + })} + + + +
+ + +
+ +

{intl.formatMessage(messages.description1)}

+

{intl.formatMessage(messages.description2)}

+ + + {isShowExportButton && ( + + + + )} + {linkCheckPresent && ( + + + + )} + + {linkCheckPresent && } +
+ + +
+ + + ); +}; + +export default CourseOptimizerPage; diff --git a/src/optimizer-page/data/api.test.js b/src/optimizer-page/data/api.test.js new file mode 100644 index 000000000..155dc2d0b --- /dev/null +++ b/src/optimizer-page/data/api.test.js @@ -0,0 +1,34 @@ +import mockApiResponse from '../mocks/mockApiResponse'; +import { initializeMocks } from '../../testUtils'; +import * as api from './api'; +import { LINK_CHECK_STATUSES } from './constants'; + +describe('Course Optimizer API', () => { + describe('postLinkCheck', () => { + it('should get an affirmative response on starting a scan', async () => { + const { axiosMock } = initializeMocks(); + const courseId = 'course-123'; + const url = api.postLinkCheckCourseApiUrl(courseId); + axiosMock.onPost(url).reply(200, { LinkCheckStatus: LINK_CHECK_STATUSES.IN_PROGRESS }); + const data = await api.postLinkCheck(courseId); + + expect(data.linkCheckStatus).toEqual(LINK_CHECK_STATUSES.IN_PROGRESS); + expect(axiosMock.history.post[0].url).toEqual(url); + }); + }); + + describe('getLinkCheckStatus', () => { + it('should get the status of a scan', async () => { + const { axiosMock } = initializeMocks(); + const courseId = 'course-123'; + const url = api.getLinkCheckStatusApiUrl(courseId); + axiosMock.onGet(url).reply(200, mockApiResponse); + const data = await api.getLinkCheckStatus(courseId); + + expect(data.linkCheckOutput).toEqual(mockApiResponse.LinkCheckOutput); + expect(data.linkCheckStatus).toEqual(mockApiResponse.LinkCheckStatus); + expect(data.linkCheckCreatedAt).toEqual(mockApiResponse.LinkCheckCreatedAt); + expect(axiosMock.history.get[0].url).toEqual(url); + }); + }); +}); diff --git a/src/optimizer-page/data/api.ts b/src/optimizer-page/data/api.ts new file mode 100644 index 000000000..af88da3c5 --- /dev/null +++ b/src/optimizer-page/data/api.ts @@ -0,0 +1,26 @@ +import { camelCaseObject, getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { LinkCheckResult } from '../types'; +import { LinkCheckStatusTypes } from './constants'; + +export interface LinkCheckStatusApiResponseBody { + linkCheckStatus: LinkCheckStatusTypes; + linkCheckOutput: LinkCheckResult; + linkCheckCreatedAt: string; +} + +const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; +export const postLinkCheckCourseApiUrl = (courseId) => new URL(`api/contentstore/v0/link_check/${courseId}`, getApiBaseUrl()).href; +export const getLinkCheckStatusApiUrl = (courseId) => new URL(`api/contentstore/v0/link_check_status/${courseId}`, getApiBaseUrl()).href; + +export async function postLinkCheck(courseId: string): Promise<{ linkCheckStatus: LinkCheckStatusTypes }> { + const { data } = await getAuthenticatedHttpClient() + .post(postLinkCheckCourseApiUrl(courseId)); + return camelCaseObject(data); +} + +export async function getLinkCheckStatus(courseId: string): Promise { + const { data } = await getAuthenticatedHttpClient() + .get(getLinkCheckStatusApiUrl(courseId)); + return camelCaseObject(data); +} diff --git a/src/optimizer-page/data/constants.ts b/src/optimizer-page/data/constants.ts new file mode 100644 index 000000000..0ad3006d1 --- /dev/null +++ b/src/optimizer-page/data/constants.ts @@ -0,0 +1,40 @@ +export const LAST_EXPORT_COOKIE_NAME = 'lastexport'; +export const LINK_CHECK_STATUSES = { + UNINITIATED: 'Uninitiated', + PENDING: 'Pending', + IN_PROGRESS: 'In-Progress', + SUCCEEDED: 'Succeeded', + FAILED: 'Failed', + CANCELED: 'Canceled', + RETRYING: 'Retrying', +}; +export enum LinkCheckStatusTypes { + UNINITIATED = 'Uninitiated', + PENDING = 'Pending', + IN_PROGRESS = 'In-Progress', + SUCCEEDED = 'Succeeded', + FAILED = 'Failed', + CANCELED = 'Canceled', + RETRYING = 'Retrying', +} +export const SCAN_STAGES = { + [LINK_CHECK_STATUSES.UNINITIATED]: 0, + [LINK_CHECK_STATUSES.PENDING]: 1, + [LINK_CHECK_STATUSES.IN_PROGRESS]: 1, + [LINK_CHECK_STATUSES.RETRYING]: 1, + [LINK_CHECK_STATUSES.SUCCEEDED]: 2, + [LINK_CHECK_STATUSES.FAILED]: -1, + [LINK_CHECK_STATUSES.CANCELED]: -1, +}; + +export const LINK_CHECK_IN_PROGRESS_STATUSES = [ + LINK_CHECK_STATUSES.PENDING, + LINK_CHECK_STATUSES.IN_PROGRESS, + LINK_CHECK_STATUSES.RETRYING, +]; + +export const LINK_CHECK_FAILURE_STATUSES = [ + LINK_CHECK_STATUSES.FAILED, + LINK_CHECK_STATUSES.CANCELED, +]; +export const SUCCESS_DATE_FORMAT = 'MM/DD/yyyy'; diff --git a/src/optimizer-page/data/selectors.ts b/src/optimizer-page/data/selectors.ts new file mode 100644 index 000000000..7454157c5 --- /dev/null +++ b/src/optimizer-page/data/selectors.ts @@ -0,0 +1,12 @@ +import { RootState } from './slice'; + +export const getLinkCheckInProgress = (state: RootState) => state.courseOptimizer.linkCheckInProgress; +export const getCurrentStage = (state: RootState) => state.courseOptimizer.currentStage; +export const getDownloadPath = (state: RootState) => state.courseOptimizer.downloadPath; +export const getSuccessDate = (state: RootState) => state.courseOptimizer.successDate; +export const getError = (state: RootState) => state.courseOptimizer.error; +export const getIsErrorModalOpen = (state: RootState) => state.courseOptimizer.isErrorModalOpen; +export const getLoadingStatus = (state: RootState) => state.courseOptimizer.loadingStatus; +export const getSavingStatus = (state: RootState) => state.courseOptimizer.savingStatus; +export const getLinkCheckResult = (state: RootState) => state.courseOptimizer.linkCheckResult; +export const getLastScannedAt = (state: RootState) => state.courseOptimizer.lastScannedAt; diff --git a/src/optimizer-page/data/slice.test.ts b/src/optimizer-page/data/slice.test.ts new file mode 100644 index 000000000..14ea76d33 --- /dev/null +++ b/src/optimizer-page/data/slice.test.ts @@ -0,0 +1,111 @@ +import { AnyAction, configureStore, ThunkMiddleware } from '@reduxjs/toolkit'; +import { ToolkitStore } from '@reduxjs/toolkit/dist/configureStore'; +import { + CourseOptimizerState, + reducer, + updateLinkCheckInProgress, + updateLinkCheckResult, + updateLastScannedAt, + updateCurrentStage, + updateDownloadPath, + updateSuccessDate, + updateError, + updateIsErrorModalOpen, + reset, + updateLoadingStatus, + updateSavingStatus, +} from './slice'; + +describe('courseOptimizer slice', () => { + let store: ToolkitStore]>; + + beforeEach(() => { + store = configureStore({ reducer }); + }); + + it('should handle initial state', () => { + expect(store.getState()).toEqual({ + linkCheckInProgress: null, + linkCheckResult: null, + lastScannedAt: null, + currentStage: null, + error: { msg: null, unitUrl: null }, + downloadPath: null, + successDate: null, + isErrorModalOpen: false, + loadingStatus: '', + savingStatus: '', + }); + }); + + it('should handle updateLinkCheckInProgress', () => { + store.dispatch(updateLinkCheckInProgress(true)); + expect(store.getState().linkCheckInProgress).toBe(true); + }); + + it('should handle updateLinkCheckResult', () => { + const result = { valid: true }; + store.dispatch(updateLinkCheckResult(result)); + expect(store.getState().linkCheckResult).toEqual(result); + }); + + it('should handle updateLastScannedAt', () => { + const date = '2023-10-01'; + store.dispatch(updateLastScannedAt(date)); + expect(store.getState().lastScannedAt).toBe(date); + }); + + it('should handle updateCurrentStage', () => { + store.dispatch(updateCurrentStage(2)); + expect(store.getState().currentStage).toBe(2); + }); + + it('should handle updateDownloadPath', () => { + const path = '/path/to/download'; + store.dispatch(updateDownloadPath(path)); + expect(store.getState().downloadPath).toBe(path); + }); + + it('should handle updateSuccessDate', () => { + const date = '2023-10-01'; + store.dispatch(updateSuccessDate(date)); + expect(store.getState().successDate).toBe(date); + }); + + it('should handle updateError', () => { + const error = { msg: 'Error message', unitUrl: 'http://example.com' }; + store.dispatch(updateError(error)); + expect(store.getState().error).toEqual(error); + }); + + it('should handle updateIsErrorModalOpen', () => { + store.dispatch(updateIsErrorModalOpen(true)); + expect(store.getState().isErrorModalOpen).toBe(true); + }); + + it('should handle reset', () => { + store.dispatch(reset()); + expect(store.getState()).toEqual({ + linkCheckInProgress: null, + linkCheckResult: null, + lastScannedAt: null, + currentStage: null, + error: { msg: null, unitUrl: null }, + downloadPath: null, + successDate: null, + isErrorModalOpen: false, + loadingStatus: '', + savingStatus: '', + }); + }); + + it('should handle updateLoadingStatus', () => { + store.dispatch(updateLoadingStatus({ status: 'loading' })); + expect(store.getState().loadingStatus).toBe('loading'); + }); + + it('should handle updateSavingStatus', () => { + store.dispatch(updateSavingStatus({ status: 'saving' })); + expect(store.getState().savingStatus).toBe('saving'); + }); +}); diff --git a/src/optimizer-page/data/slice.ts b/src/optimizer-page/data/slice.ts new file mode 100644 index 000000000..7b3f81a8a --- /dev/null +++ b/src/optimizer-page/data/slice.ts @@ -0,0 +1,91 @@ +/* eslint-disable no-param-reassign */ +import { createSlice } from '@reduxjs/toolkit'; +import { LinkCheckResult } from '../types'; + +export interface CourseOptimizerState { + linkCheckInProgress: boolean | null; + linkCheckResult: LinkCheckResult | null; + lastScannedAt: string | null; + currentStage: number | null; + error: { msg: string | null; unitUrl: string | null }; + downloadPath: string | null; + successDate: string | null; + isErrorModalOpen: boolean; + loadingStatus: string; + savingStatus: string; +} + +export type RootState = { + [key: string]: any; +} & { + courseOptimizer: CourseOptimizerState; +}; + +const initialState: CourseOptimizerState = { + linkCheckInProgress: null, + linkCheckResult: null, + lastScannedAt: null, + currentStage: null, + error: { msg: null, unitUrl: null }, + downloadPath: null, + successDate: null, + isErrorModalOpen: false, + loadingStatus: '', + savingStatus: '', +}; + +const slice = createSlice({ + name: 'courseOptimizer', + initialState, + reducers: { + updateLinkCheckInProgress: (state, { payload }) => { + state.linkCheckInProgress = payload; + }, + updateLinkCheckResult: (state, { payload }) => { + state.linkCheckResult = payload; + }, + updateLastScannedAt: (state, { payload }) => { + state.lastScannedAt = payload; + }, + updateCurrentStage: (state, { payload }) => { + state.currentStage = payload; + }, + updateDownloadPath: (state, { payload }) => { + state.downloadPath = payload; + }, + updateSuccessDate: (state, { payload }) => { + state.successDate = payload; + }, + updateError: (state, { payload }) => { + state.error = payload; + }, + updateIsErrorModalOpen: (state, { payload }) => { + state.isErrorModalOpen = payload; + }, + reset: () => initialState, + updateLoadingStatus: (state, { payload }) => { + state.loadingStatus = payload.status; + }, + updateSavingStatus: (state, { payload }) => { + state.savingStatus = payload.status; + }, + }, +}); + +export const { + updateLinkCheckInProgress, + updateLinkCheckResult, + updateLastScannedAt, + updateCurrentStage, + updateDownloadPath, + updateSuccessDate, + updateError, + updateIsErrorModalOpen, + reset, + updateLoadingStatus, + updateSavingStatus, +} = slice.actions; + +export const { + reducer, +} = slice; diff --git a/src/optimizer-page/data/thunks.test.js b/src/optimizer-page/data/thunks.test.js new file mode 100644 index 000000000..208b8f452 --- /dev/null +++ b/src/optimizer-page/data/thunks.test.js @@ -0,0 +1,193 @@ +import { startLinkCheck, fetchLinkCheckStatus } from './thunks'; +import * as api from './api'; +import { LINK_CHECK_STATUSES } from './constants'; +import { RequestStatus } from '../../data/constants'; +import mockApiResponse from '../mocks/mockApiResponse'; + +describe('startLinkCheck thunk', () => { + const dispatch = jest.fn(); + const getState = jest.fn(); + const courseId = 'course-123'; + let mockGetStartLinkCheck; + + beforeEach(() => { + jest.clearAllMocks(); + + mockGetStartLinkCheck = jest.spyOn(api, 'postLinkCheck').mockResolvedValue({ + linkCheckStatus: LINK_CHECK_STATUSES.IN_PROGRESS, + }); + }); + + describe('successful request', () => { + it('should set link check stage and request statuses to their in-progress states', async () => { + const inProgressStageId = 1; + await startLinkCheck(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.PENDING }, + type: 'courseOptimizer/updateSavingStatus', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: true, + type: 'courseOptimizer/updateLinkCheckInProgress', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.SUCCESSFUL }, + type: 'courseOptimizer/updateSavingStatus', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: inProgressStageId, + type: 'courseOptimizer/updateCurrentStage', + }); + }); + }); + + describe('failed request should set stage and request ', () => { + it('should set request status to failed', async () => { + mockGetStartLinkCheck.mockRejectedValue(new Error('error')); + + await startLinkCheck(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.FAILED }, + type: 'courseOptimizer/updateSavingStatus', + }); + expect(dispatch).toHaveBeenCalledWith({ + payload: false, + type: 'courseOptimizer/updateLinkCheckInProgress', + }); + expect(dispatch).toHaveBeenCalledWith({ + payload: -1, + type: 'courseOptimizer/updateCurrentStage', + }); + }); + }); +}); + +describe('fetchLinkCheckStatus thunk', () => { + const dispatch = jest.fn(); + const getState = jest.fn(); + const courseId = 'course-123'; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('successful request', () => { + it('should return scan result', async () => { + jest + .spyOn(api, 'getLinkCheckStatus') + .mockResolvedValue({ + linkCheckStatus: mockApiResponse.LinkCheckStatus, + linkCheckOutput: mockApiResponse.LinkCheckOutput, + linkCheckCreatedAt: mockApiResponse.LinkCheckCreatedAt, + }); + + await fetchLinkCheckStatus(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: false, + type: 'courseOptimizer/updateLinkCheckInProgress', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: 2, + type: 'courseOptimizer/updateCurrentStage', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: mockApiResponse.LinkCheckOutput, + type: 'courseOptimizer/updateLinkCheckResult', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.SUCCESSFUL }, + type: 'courseOptimizer/updateLoadingStatus', + }); + }); + + it('with link check in progress should set current stage to 1', async () => { + jest + .spyOn(api, 'getLinkCheckStatus') + .mockResolvedValue({ + linkCheckStatus: LINK_CHECK_STATUSES.IN_PROGRESS, + }); + + await fetchLinkCheckStatus(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: 1, + type: 'courseOptimizer/updateCurrentStage', + }); + }); + }); + + describe('failed request', () => { + it('should set request status to failed', async () => { + jest + .spyOn(api, 'getLinkCheckStatus') + .mockRejectedValue(new Error('error')); + + await fetchLinkCheckStatus(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.FAILED }, + type: 'courseOptimizer/updateLoadingStatus', + }); + }); + }); + + describe('unauthorized request', () => { + it('should set request status to denied', async () => { + jest.spyOn(api, 'getLinkCheckStatus').mockRejectedValue({ response: { status: 403 } }); + await fetchLinkCheckStatus(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.DENIED }, + type: 'courseOptimizer/updateLoadingStatus', + }); + }); + }); + + describe('failed scan', () => { + it('should set error message', async () => { + jest + .spyOn(api, 'getLinkCheckStatus') + .mockResolvedValue({ + linkCheckStatus: LINK_CHECK_STATUSES.FAILED, + linkCheckOutput: mockApiResponse.LinkCheckOutput, + linkCheckCreatedAt: mockApiResponse.LinkCheckCreatedAt, + }); + + await fetchLinkCheckStatus(courseId)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: true, + type: 'courseOptimizer/updateIsErrorModalOpen', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { msg: 'Link Check Failed' }, + type: 'courseOptimizer/updateError', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { status: RequestStatus.SUCCESSFUL }, + type: 'courseOptimizer/updateLoadingStatus', + }); + + expect(dispatch).toHaveBeenCalledWith({ + payload: -1, + type: 'courseOptimizer/updateCurrentStage', + }); + + expect(dispatch).not.toHaveBeenCalledWith({ + payload: expect.anything(), + type: 'courseOptimizer/updateLinkCheckResult', + }); + }); + }); +}); diff --git a/src/optimizer-page/data/thunks.ts b/src/optimizer-page/data/thunks.ts new file mode 100644 index 000000000..1fb3c5720 --- /dev/null +++ b/src/optimizer-page/data/thunks.ts @@ -0,0 +1,81 @@ +import { RequestStatus } from '../../data/constants'; +import { + LINK_CHECK_FAILURE_STATUSES, + LINK_CHECK_IN_PROGRESS_STATUSES, + LINK_CHECK_STATUSES, + SCAN_STAGES, +} from './constants'; + +import { postLinkCheck, getLinkCheckStatus } from './api'; +import { + updateLinkCheckInProgress, + updateLinkCheckResult, + updateCurrentStage, + updateError, + updateIsErrorModalOpen, + updateLoadingStatus, + updateSavingStatus, + updateLastScannedAt, +} from './slice'; + +export function startLinkCheck(courseId: string) { + return async (dispatch) => { + dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); + dispatch(updateLinkCheckInProgress(true)); + dispatch(updateCurrentStage(SCAN_STAGES[LINK_CHECK_STATUSES.PENDING])); + try { + await postLinkCheck(courseId); + await dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); + return true; + } catch (error) { + dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); + dispatch(updateLinkCheckInProgress(false)); + dispatch(updateCurrentStage(SCAN_STAGES[LINK_CHECK_STATUSES.CANCELED])); + return false; + } + }; +} + +export function fetchLinkCheckStatus(courseId) { + return async (dispatch) => { + dispatch(updateLoadingStatus({ status: RequestStatus.IN_PROGRESS })); + + try { + const { linkCheckStatus, linkCheckOutput, linkCheckCreatedAt } = await getLinkCheckStatus( + courseId, + ); + + if (LINK_CHECK_IN_PROGRESS_STATUSES.includes(linkCheckStatus)) { + dispatch(updateLinkCheckInProgress(true)); + } else { + dispatch(updateLinkCheckInProgress(false)); + } + + dispatch(updateCurrentStage(SCAN_STAGES[linkCheckStatus])); + + if ( + linkCheckStatus === undefined + || linkCheckStatus === null + || LINK_CHECK_FAILURE_STATUSES.includes(linkCheckStatus) + ) { + dispatch(updateError({ msg: 'Link Check Failed' })); + dispatch(updateIsErrorModalOpen(true)); + } else if (linkCheckOutput) { + dispatch(updateLinkCheckResult(linkCheckOutput)); + dispatch(updateLastScannedAt(linkCheckCreatedAt)); + } + + dispatch(updateLoadingStatus({ status: RequestStatus.SUCCESSFUL })); + return true; + } catch (error: any) { + if (error?.response && error?.response.status === 403) { + dispatch(updateLoadingStatus({ status: RequestStatus.DENIED })); + } else { + dispatch( + updateLoadingStatus({ status: RequestStatus.FAILED }), + ); + } + return false; + } + }; +} diff --git a/src/optimizer-page/messages.js b/src/optimizer-page/messages.js new file mode 100644 index 000000000..d83108bd4 --- /dev/null +++ b/src/optimizer-page/messages.js @@ -0,0 +1,71 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + pageTitle: { + id: 'course-authoring.course-optimizer.page.title', + defaultMessage: '{headingTitle} | {courseName} | {siteName}', + }, + headingTitle: { + id: 'course-authoring.course-optimizer.heading.title', + defaultMessage: 'Course Optimizer', + }, + headingSubtitle: { + id: 'course-authoring.course-optimizer.heading.subtitle', + defaultMessage: 'Tools', + }, + description1: { + id: 'course-authoring.course-optimizer.description1', + defaultMessage: `This tool will scan your the published version of your course for broken links. + Unpublished changes will not be included in the scan. + Note that this process will take more time for larger courses. + To update the scan after you have published new changes to your course, + click the "Start Scanning" button again. + `, + }, + description2: { + id: 'course-authoring.course-optimizer.description2', + defaultMessage: 'Broken links are links pointing to external websites, images, or videos that do not exist or are no longer available. These links can cause issues for learners when they try to access the content.', + }, + card1Title: { + id: 'course-authoring.course-optimizer.card1.title', + defaultMessage: 'Scan my course for broken links', + }, + card2Title: { + id: 'course-authoring.course-optimizer.card2.title', + defaultMessage: 'Scan my course for broken links', + }, + buttonTitle: { + id: 'course-authoring.course-optimizer.button.title', + defaultMessage: 'Start Scanning', + }, + preparingStepTitle: { + id: 'course-authoring.course-optimizer.peparing-step.title', + defaultMessage: 'Preparing', + }, + preparingStepDescription: { + id: 'course-authoring.course-optimizer.peparing-step.description', + defaultMessage: 'Preparing to start the scan', + }, + scanningStepTitle: { + id: 'course-authoring.course-optimizer.scanning-step.title', + defaultMessage: 'Scanning', + }, + scanningStepDescription: { + id: 'course-authoring.course-optimizer.scanning-step.description', + defaultMessage: 'Scanning for broken links in your course (You can now leave this page safely, but avoid making drastic changes to content until the scan is complete)', + }, + successStepTitle: { + id: 'course-authoring.course-optimizer.success-step.title', + defaultMessage: 'Success', + }, + successStepDescription: { + id: 'course-authoring.course-optimizer.success-step.description', + defaultMessage: 'Your Scan is complete. You can view the list of results below.', + }, + lastScannedOn: { + id: 'course-authoring.course-optimizer.last-scanned-on', + defaultMessage: 'Last scanned on', + }, +}); + +export default messages; diff --git a/src/optimizer-page/mocks/mockApiResponse.js b/src/optimizer-page/mocks/mockApiResponse.js new file mode 100644 index 000000000..dd3b54e39 --- /dev/null +++ b/src/optimizer-page/mocks/mockApiResponse.js @@ -0,0 +1,106 @@ +const mockApiResponse = { + LinkCheckStatus: 'Succeeded', + LinkCheckCreatedAt: '2024-12-14T00:26:50.838350Z', + LinkCheckOutput: { + sections: [ + { + id: 'section-1', + displayName: 'Introduction to Programming', + subsections: [ + { + id: 'subsection-1-1', + displayName: 'Getting Started', + units: [ + { + id: 'unit-1-1-1', + displayName: 'Welcome Video', + blocks: [ + { + id: 'block-1-1-1-1', + url: 'https://example.com/welcome-video', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + { + id: 'block-1-1-1-2', + url: 'https://example.com/intro-guide', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + ], + }, + { + id: 'unit-1-1-2', + displayName: 'Course Overview', + blocks: [ + { + id: 'block-1-1-2-1', + url: 'https://example.com/course-overview', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + ], + }, + ], + }, + { + id: 'subsection-1-2', + displayName: 'Basic Concepts', + units: [ + { + id: 'unit-1-2-1', + displayName: 'Variables and Data Types', + blocks: [ + { + id: 'block-1-2-1-1', + url: 'https://example.com/variables', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + { + id: 'block-1-2-1-2', + url: 'https://example.com/broken-link', + brokenLinks: ['https://example.com/broken-link'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + ], + }, + ], + }, + ], + }, + { + id: 'section-2', + displayName: 'Advanced Topics', + subsections: [ + { + id: 'subsection-2-1', + displayName: 'Algorithms and Data Structures', + units: [ + { + id: 'unit-2-1-1', + displayName: 'Sorting Algorithms', + blocks: [ + { + id: 'block-2-1-1-1', + url: 'https://example.com/sorting-algorithms', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + { + id: 'block-2-1-1-2', + url: 'https://example.com/broken-link-algo', + brokenLinks: ['https://example.com/broken-link-algo'], + lockedLinks: ['https://example.com/locked-link-algo'], + }, + ], + }, + ], + }, + ], + }, + ], + }, +}; + +export default mockApiResponse; diff --git a/src/optimizer-page/scan-results/BrokenLinkTable.tsx b/src/optimizer-page/scan-results/BrokenLinkTable.tsx new file mode 100644 index 000000000..ebd177d12 --- /dev/null +++ b/src/optimizer-page/scan-results/BrokenLinkTable.tsx @@ -0,0 +1,117 @@ +import { Icon, Table } from '@openedx/paragon'; +import { OpenInNew, Lock, LinkOff } from '@openedx/paragon/icons'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { FC } from 'react'; +import { Unit } from '../types'; +import messages from './messages'; +import LockedInfoIcon from './LockedInfoIcon'; + +const BrokenLinkHref: FC<{ href: string }> = ({ href }) => ( + +); + +const GoToBlock: FC<{ block: { url: string } }> = ({ block }) => ( + + + + Go to Block + + +); + +interface BrokenLinkTableProps { + unit: Unit; + showLockedLinks: boolean; +} + +type TableData = { + blockLink: JSX.Element; + brokenLink: JSX.Element; + status: JSX.Element; +}[]; + +const BrokenLinkTable: FC = ({ + unit, + showLockedLinks, +}) => { + const intl = useIntl(); + return ( + <> +

{unit.displayName}

+ { + const blockBrokenLinks = block.brokenLinks.map((link) => ({ + blockLink: , + blockDisplayName: block.displayName || '', + brokenLink: , + status: ( + + + + {intl.formatMessage(messages.brokenLinkStatus)} + + + ), + })); + acc.push(...blockBrokenLinks); + if (!showLockedLinks) { + return acc; + } + + const blockLockedLinks = block.lockedLinks.map((link) => ({ + blockLink: , + blockDisplayName: block.displayName || '', + brokenLink: , + status: ( + + + {intl.formatMessage(messages.lockedLinkStatus)}{' '} + + + ), + })); + acc.push(...blockLockedLinks); + return acc; + }, + [], + )} + columns={[ + { + key: 'blockDisplayName', + columnSortable: false, + width: 'col-3', + hideHeader: true, + }, + { + key: 'blockLink', + columnSortable: false, + width: 'col-3', + hideHeader: true, + }, + { + key: 'brokenLink', + columnSortable: false, + width: 'col-6', + hideHeader: true, + }, + { + key: 'status', + columnSortable: false, + width: 'col-6', + hideHeader: true, + }, + ]} + /> + + ); +}; + +export default BrokenLinkTable; diff --git a/src/optimizer-page/scan-results/LockedInfoIcon.jsx b/src/optimizer-page/scan-results/LockedInfoIcon.jsx new file mode 100644 index 000000000..788dcb130 --- /dev/null +++ b/src/optimizer-page/scan-results/LockedInfoIcon.jsx @@ -0,0 +1,30 @@ +import { + Icon, + OverlayTrigger, + Tooltip, +} from '@openedx/paragon'; +import { + Question, +} from '@openedx/paragon/icons'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import messages from './messages'; + +const LockedInfoIcon = () => { + const intl = useIntl(); + + return ( + + {intl.formatMessage(messages.lockedInfoTooltip)} + + )} + > + + + ); +}; + +export default LockedInfoIcon; diff --git a/src/optimizer-page/scan-results/ScanResults.scss b/src/optimizer-page/scan-results/ScanResults.scss new file mode 100644 index 000000000..1918ad7b0 --- /dev/null +++ b/src/optimizer-page/scan-results/ScanResults.scss @@ -0,0 +1,110 @@ +.scan-results { + thead { + display: none; + } + + .red-italics { + color: $brand-500; + margin-left: 2rem; + font-weight: 400; + font-size: 80%; + font-style: italic; + } + + .yellow-italics { + color: $warning-800;; + margin-left: 2rem; + font-weight: 400; + font-size: 80%; + font-style: italic; + } + + .section { + &.is-open { + &:not(:first-child) { + margin-top: 1rem; + } + + margin-bottom: 1rem; + } + } + + .open-arrow { + transform: translate(-10px, 5px); + display: inline-block; + } + + /* Section Header */ + .subsection-header { + font-size: 16px; /* Slightly smaller */ + font-weight: 600; /* Reduced boldness */ + background-color: $dark-100; + padding: 10px; + margin-bottom: 10px; + } + + /* Subsection Header */ + .unit-header { + margin-left: .5rem; + margin-top: 10px; + font-size: 14px; + font-weight: 700; + margin-bottom: 5px; + color: $primary-500; + } + + /* Block Links */ + .broken-link-list li { + margin-bottom: 8px; /* Add breathing room */ + } + + .broken-link-list a { + text-decoration: none; + margin-left: 2rem; + } + + /* Broken Links Highlight */ + .broken-links-count { + color: red; + font-weight: bold; + } + + .unit { + padding: 0 3rem; + } + + .broken-link { + color: $brand-500; + text-decoration: none; + } + + .broken-link-container { + max-width: 18rem; + text-wrap: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + .locked-links-checkbox { + margin-top: .45rem; + } + + .locked-links-checkbox-wrapper { + display: flex; + gap: 1rem; + } + + .link-status-text { + display: flex; + align-items: center; + gap: .5rem; + } + + .broken-link-icon { + color: $brand-500; + } + + .lock-icon { + color: $warning-300; + } +} diff --git a/src/optimizer-page/scan-results/ScanResults.tsx b/src/optimizer-page/scan-results/ScanResults.tsx new file mode 100644 index 000000000..a409b1249 --- /dev/null +++ b/src/optimizer-page/scan-results/ScanResults.tsx @@ -0,0 +1,89 @@ +import { useState, useMemo, FC } from 'react'; +import { + Card, + CheckBox, +} from '@openedx/paragon'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import messages from './messages'; +import SectionCollapsible from './SectionCollapsible'; +import BrokenLinkTable from './BrokenLinkTable'; +import LockedInfoIcon from './LockedInfoIcon'; +import { LinkCheckResult } from '../types'; +import { countBrokenLinks } from '../utils'; + +const InfoCard: FC<{ text: string }> = ({ text }) => ( + +

+ {text} +

+ +); + +interface Props { + data: LinkCheckResult | null; +} + +const ScanResults: FC = ({ data }) => { + const intl = useIntl(); + const [showLockedLinks, setShowLockedLinks] = useState(true); + + const { brokenLinksCounts, lockedLinksCounts } = useMemo(() => countBrokenLinks(data), [data?.sections]); + + if (!data?.sections) { + return ; + } + + const { sections } = data; + + return ( +
+
+
+

{intl.formatMessage(messages.scanHeader)}

+ + { + setShowLockedLinks(!showLockedLinks); + }} + label={intl.formatMessage(messages.lockedCheckboxLabel)} + /> + + +
+
+ + {sections?.map((section, index) => ( + + {section.subsections.map((subsection) => ( + <> +

+ {subsection.displayName} +

+ {subsection.units.map((unit) => ( +
+ +
+ ))} + + ))} + + ))} +
+ ); +}; + +export default ScanResults; diff --git a/src/optimizer-page/scan-results/SectionCollapsible.tsx b/src/optimizer-page/scan-results/SectionCollapsible.tsx new file mode 100644 index 000000000..077f747cc --- /dev/null +++ b/src/optimizer-page/scan-results/SectionCollapsible.tsx @@ -0,0 +1,53 @@ +import { useState, FC } from 'react'; +import { + Collapsible, + Icon, +} from '@openedx/paragon'; +import { + ArrowRight, + ArrowDropDown, +} from '@openedx/paragon/icons'; + +interface Props { + title: string; + children: React.ReactNode; + redItalics?: string; + yellowItalics?: string; + className?: string; +} + +const SectionCollapsible: FC = ({ + title, children, redItalics = '', yellowItalics = '', className = '', +}) => { + const [isOpen, setIsOpen] = useState(false); + const styling = 'card-lg'; + const collapsibleTitle = ( +
+ + {title} + {redItalics} + {yellowItalics} +
+ ); + + return ( +
+ + {collapsibleTitle} +

+ )} + iconWhenClosed="" + iconWhenOpen="" + open={isOpen} + onToggle={() => setIsOpen(!isOpen)} + > + {children} + +
+ ); +}; + +export default SectionCollapsible; diff --git a/src/optimizer-page/scan-results/index.js b/src/optimizer-page/scan-results/index.js new file mode 100644 index 000000000..ab1d4b80b --- /dev/null +++ b/src/optimizer-page/scan-results/index.js @@ -0,0 +1,3 @@ +import ScanResults from './ScanResults'; + +export default ScanResults; diff --git a/src/optimizer-page/scan-results/messages.js b/src/optimizer-page/scan-results/messages.js new file mode 100644 index 000000000..7b388bfe9 --- /dev/null +++ b/src/optimizer-page/scan-results/messages.js @@ -0,0 +1,46 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + pageTitle: { + id: 'course-authoring.course-optimizer.page.title', + defaultMessage: '{headingTitle} | {courseName} | {siteName}', + }, + noDataCard: { + id: 'course-authoring.course-optimizer.noDataCard', + defaultMessage: 'No Scan data available', + }, + noBrokenLinksCard: { + id: 'course-authoring.course-optimizer.emptyResultsCard', + defaultMessage: 'No broken links found', + }, + scanHeader: { + id: 'course-authoring.course-optimizer.scanHeader', + defaultMessage: 'Broken Links Scan', + }, + lockedCheckboxLabel: { + id: 'course-authoring.course-optimizer.lockedCheckboxLabel', + defaultMessage: 'Show Locked Course Files', + }, + brokenLinksNumber: { + id: 'course-authoring.course-optimizer.brokenLinksNumber', + defaultMessage: '{count} broken links', + }, + lockedLinksNumber: { + id: 'course-authoring.course-optimizer.lockedLinksNumber', + defaultMessage: '{count} locked links', + }, + lockedInfoTooltip: { + id: 'course-authoring.course-optimizer.lockedInfoTooltip', + defaultMessage: 'These course files are "locked", so we cannot verify if the link can access the file.', + }, + brokenLinkStatus: { + id: 'course-authoring.course-optimizer.brokenLinkStatus', + defaultMessage: 'Status: Broken', + }, + lockedLinkStatus: { + id: 'course-authoring.course-optimizer.lockedLinkStatus', + defaultMessage: 'Status: Locked', + }, +}); + +export default messages; diff --git a/src/optimizer-page/types.ts b/src/optimizer-page/types.ts new file mode 100644 index 000000000..831c69281 --- /dev/null +++ b/src/optimizer-page/types.ts @@ -0,0 +1,27 @@ +export interface Unit { + id: string; + displayName: string; + blocks: { + id: string; + displayName?: string; + url: string; + brokenLinks: string[]; + lockedLinks: string[]; + }[]; +} + +export interface SubSection { + id: string; + displayName: string; + units: Unit[]; +} + +export interface Section { + id: string; + displayName: string; + subsections: SubSection[]; +} + +export interface LinkCheckResult { + sections: Section[]; +} diff --git a/src/optimizer-page/utils.test.js b/src/optimizer-page/utils.test.js new file mode 100644 index 000000000..0af97a146 --- /dev/null +++ b/src/optimizer-page/utils.test.js @@ -0,0 +1,44 @@ +import mockApiResponse from './mocks/mockApiResponse'; +import { countBrokenLinks } from './utils'; + +describe('countBrokenLinks', () => { + it('should return the count of broken links', () => { + const data = mockApiResponse.LinkCheckOutput; + expect(countBrokenLinks(data)).toStrictEqual({ brokenLinksCounts: [5, 2], lockedLinksCounts: [5, 2] }); + }); + + it('should return 0 if there are no broken links', () => { + const data = { + sections: [ + { + subsections: [ + { + units: [ + { + blocks: [ + { + brokenLinks: [], + }, + ], + }, + ], + }, + ], + }, + ], + }; + expect(countBrokenLinks(data)).toStrictEqual({ brokenLinksCounts: [0], lockedLinksCounts: [0] }); + }); + + it('should return [] if there is no data', () => { + const data = {}; + expect(countBrokenLinks(data)).toStrictEqual({ brokenLinksCounts: [], lockedLinksCounts: [] }); + }); + + it('should return [] if there are no sections', () => { + const data = { + sections: [], + }; + expect(countBrokenLinks(data)).toStrictEqual({ brokenLinksCounts: [], lockedLinksCounts: [] }); + }); +}); diff --git a/src/optimizer-page/utils.ts b/src/optimizer-page/utils.ts new file mode 100644 index 000000000..712cf61eb --- /dev/null +++ b/src/optimizer-page/utils.ts @@ -0,0 +1,26 @@ +/* eslint-disable import/prefer-default-export */ +import { LinkCheckResult } from './types'; + +export const countBrokenLinks = (data: LinkCheckResult | null): +{ brokenLinksCounts: number[], lockedLinksCounts: number[] } => { + if (!data?.sections) { + return { brokenLinksCounts: [], lockedLinksCounts: [] }; + } + const brokenLinksCounts: number[] = []; + const lockedLinksCounts: number[] = []; + data.sections.forEach((section) => { + let brokenLinks = 0; + let lockedLinks = 0; + section.subsections.forEach((subsection) => { + subsection.units.forEach((unit) => { + unit.blocks.forEach((block) => { + brokenLinks += block.brokenLinks?.length || 0; + lockedLinks += block.lockedLinks?.length || 0; + }); + }); + }); + brokenLinksCounts.push(brokenLinks); + lockedLinksCounts.push(lockedLinks); + }); + return { brokenLinksCounts, lockedLinksCounts }; +}; diff --git a/src/store.js b/src/store.js index bf761aadf..e979d8591 100644 --- a/src/store.js +++ b/src/store.js @@ -18,6 +18,7 @@ import { reducer as CourseUpdatesReducer } from './course-updates/data/slice'; import { reducer as processingNotificationReducer } from './generic/processing-notification/data/slice'; import { reducer as helpUrlsReducer } from './help-urls/data/slice'; import { reducer as courseExportReducer } from './export-page/data/slice'; +import { reducer as courseOptimizerReducer } from './optimizer-page/data/slice'; import { reducer as genericReducer } from './generic/data/slice'; import { reducer as courseImportReducer } from './import-page/data/slice'; import { reducer as videosReducer } from './files-and-videos/videos-page/data/slice'; @@ -47,6 +48,7 @@ export default function initializeStore(preloadedState = undefined) { processingNotification: processingNotificationReducer, helpUrls: helpUrlsReducer, courseExport: courseExportReducer, + courseOptimizer: courseOptimizerReducer, generic: genericReducer, courseImport: courseImportReducer, videos: videosReducer, From 45f6ef42a7dbc847315084f25b30906714edbd0e Mon Sep 17 00:00:00 2001 From: Jillian Date: Thu, 16 Jan 2025 04:14:30 +1030 Subject: [PATCH 08/18] fix: allow user provided value if can auto-create orgs [FC-0076] (#1582) Allows Content Authors to auto-create an organization when creating a library, if auto-creating orgs is allowed by the platform. --- .../create-library/CreateLibrary.test.tsx | 164 ++++++++++++------ .../create-library/CreateLibrary.tsx | 27 ++- src/studio-home/__mocks__/studioHomeMock.js | 1 + 3 files changed, 132 insertions(+), 60 deletions(-) diff --git a/src/library-authoring/create-library/CreateLibrary.test.tsx b/src/library-authoring/create-library/CreateLibrary.test.tsx index cf323b1ac..d129cfed1 100644 --- a/src/library-authoring/create-library/CreateLibrary.test.tsx +++ b/src/library-authoring/create-library/CreateLibrary.test.tsx @@ -1,18 +1,21 @@ import React from 'react'; -import MockAdapter from 'axios-mock-adapter'; -import { initializeMockApp } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { AppProvider } from '@edx/frontend-platform/react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { fireEvent, render, waitFor } from '@testing-library/react'; +import type MockAdapter from 'axios-mock-adapter'; import userEvent from '@testing-library/user-event'; -import initializeStore from '../../store'; +import { + act, + fireEvent, + initializeMocks, + render, + screen, + waitFor, +} from '../../testUtils'; +import { studioHomeMock } from '../../studio-home/__mocks__'; +import { getStudioHomeApiUrl } from '../../studio-home/data/api'; +import { getApiWaffleFlagsUrl } from '../../data/api'; import { CreateLibrary } from '.'; import { getContentLibraryV2CreateApiUrl } from './data/api'; -let store; const mockNavigate = jest.fn(); let axiosMock: MockAdapter; @@ -29,66 +32,41 @@ jest.mock('../../generic/data/apiHooks', () => ({ }), })); -const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, -}); - -const RootWrapper = () => ( - - - - - - - -); - describe('', () => { beforeEach(() => { - initializeMockApp({ - authenticatedUser: { - userId: 3, - username: 'abc123', - administrator: true, - roles: [], - }, - }); - store = initializeStore(); - - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + axiosMock = initializeMocks().axiosMock; + axiosMock + .onGet(getApiWaffleFlagsUrl(undefined)) + .reply(200, {}); }); afterEach(() => { jest.clearAllMocks(); axiosMock.restore(); - queryClient.clear(); }); test('call api data with correct data', async () => { + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock); axiosMock.onPost(getContentLibraryV2CreateApiUrl()).reply(200, { id: 'library-id', }); - const { getByRole } = render(); + render(); - const titleInput = getByRole('textbox', { name: /library name/i }); + const titleInput = await screen.findByRole('textbox', { name: /library name/i }); userEvent.click(titleInput); userEvent.type(titleInput, 'Test Library Name'); - const orgInput = getByRole('combobox', { name: /organization/i }); + const orgInput = await screen.findByRole('combobox', { name: /organization/i }); userEvent.click(orgInput); userEvent.type(orgInput, 'org1'); - userEvent.tab(); + act(() => userEvent.tab()); - const slugInput = getByRole('textbox', { name: /library id/i }); + const slugInput = await screen.findByRole('textbox', { name: /library id/i }); userEvent.click(slugInput); userEvent.type(slugInput, 'test_library_slug'); - fireEvent.click(getByRole('button', { name: /create/i })); + fireEvent.click(await screen.findByRole('button', { name: /create/i })); await waitFor(() => { expect(axiosMock.history.post.length).toBe(1); expect(axiosMock.history.post[0].data).toBe( @@ -98,41 +76,115 @@ describe('', () => { }); }); + test('cannot create new org unless allowed', async () => { + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock); + axiosMock.onPost(getContentLibraryV2CreateApiUrl()).reply(200, { + id: 'library-id', + }); + + render(); + + const titleInput = await screen.findByRole('textbox', { name: /library name/i }); + userEvent.click(titleInput); + userEvent.type(titleInput, 'Test Library Name'); + + // We cannot create a new org, and so we're restricted to the allowed list + const orgOptions = screen.getByTestId('autosuggest-iconbutton'); + userEvent.click(orgOptions); + expect(screen.getByText('org1')).toBeInTheDocument(); + ['org2', 'org3', 'org4', 'org5'].forEach((org) => expect(screen.queryByText(org)).not.toBeInTheDocument()); + + const orgInput = await screen.findByRole('combobox', { name: /organization/i }); + userEvent.click(orgInput); + userEvent.type(orgInput, 'NewOrg'); + act(() => userEvent.tab()); + + const slugInput = await screen.findByRole('textbox', { name: /library id/i }); + userEvent.click(slugInput); + userEvent.type(slugInput, 'test_library_slug'); + + fireEvent.click(await screen.findByRole('button', { name: /create/i })); + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(0); + }); + expect(await screen.findByText('Required field.')).toBeInTheDocument(); + }); + + test('can create new org if allowed', async () => { + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, { + ...studioHomeMock, + allow_to_create_new_org: true, + }); + axiosMock.onPost(getContentLibraryV2CreateApiUrl()).reply(200, { + id: 'library-id', + }); + + render(); + + const titleInput = await screen.findByRole('textbox', { name: /library name/i }); + userEvent.click(titleInput); + userEvent.type(titleInput, 'Test Library Name'); + + // We can create a new org, so we're also allowed to use any existing org + const orgOptions = screen.getByTestId('autosuggest-iconbutton'); + userEvent.click(orgOptions); + ['org1', 'org2', 'org3', 'org4', 'org5'].forEach((org) => expect(screen.queryByText(org)).toBeInTheDocument()); + + const orgInput = await screen.findByRole('combobox', { name: /organization/i }); + userEvent.click(orgInput); + userEvent.type(orgInput, 'NewOrg'); + act(() => userEvent.tab()); + + const slugInput = await screen.findByRole('textbox', { name: /library id/i }); + userEvent.click(slugInput); + userEvent.type(slugInput, 'test_library_slug'); + + fireEvent.click(await screen.findByRole('button', { name: /create/i })); + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(1); + expect(axiosMock.history.post[0].data).toBe( + '{"description":"","title":"Test Library Name","org":"NewOrg","slug":"test_library_slug"}', + ); + expect(mockNavigate).toHaveBeenCalledWith('/library/library-id'); + }); + }); + test('show api error', async () => { + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock); axiosMock.onPost(getContentLibraryV2CreateApiUrl()).reply(400, { field: 'Error message', }); - const { getByRole, getByTestId } = render(); + render(); - const titleInput = getByRole('textbox', { name: /library name/i }); + const titleInput = await screen.findByRole('textbox', { name: /library name/i }); userEvent.click(titleInput); userEvent.type(titleInput, 'Test Library Name'); - const orgInput = getByTestId('autosuggest-textbox-input'); + const orgInput = await screen.findByTestId('autosuggest-textbox-input'); userEvent.click(orgInput); userEvent.type(orgInput, 'org1'); - userEvent.tab(); + act(() => userEvent.tab()); - const slugInput = getByRole('textbox', { name: /library id/i }); + const slugInput = await screen.findByRole('textbox', { name: /library id/i }); userEvent.click(slugInput); userEvent.type(slugInput, 'test_library_slug'); - fireEvent.click(getByRole('button', { name: /create/i })); - await waitFor(() => { + fireEvent.click(await screen.findByRole('button', { name: /create/i })); + await waitFor(async () => { expect(axiosMock.history.post.length).toBe(1); expect(axiosMock.history.post[0].data).toBe( '{"description":"","title":"Test Library Name","org":"org1","slug":"test_library_slug"}', ); expect(mockNavigate).not.toHaveBeenCalled(); - expect(getByRole('alert')).toHaveTextContent('Request failed with status code 400'); - expect(getByRole('alert')).toHaveTextContent('{"field":"Error message"}'); + expect(await screen.findByRole('alert')).toHaveTextContent('Request failed with status code 400'); + expect(await screen.findByRole('alert')).toHaveTextContent('{"field":"Error message"}'); }); }); test('cancel creating library navigates to libraries page', async () => { - const { getByRole } = render(); + render(); - fireEvent.click(getByRole('button', { name: /cancel/i })); + fireEvent.click(await screen.findByRole('button', { name: /cancel/i })); await waitFor(() => { expect(mockNavigate).toHaveBeenCalledWith('/libraries'); }); diff --git a/src/library-authoring/create-library/CreateLibrary.tsx b/src/library-authoring/create-library/CreateLibrary.tsx index 1731984ec..c4f3695c7 100644 --- a/src/library-authoring/create-library/CreateLibrary.tsx +++ b/src/library-authoring/create-library/CreateLibrary.tsx @@ -19,6 +19,7 @@ import FormikErrorFeedback from '../../generic/FormikErrorFeedback'; import AlertError from '../../generic/alert-error'; import { useOrganizationListData } from '../../generic/data/apiHooks'; import SubHeader from '../../generic/sub-header/SubHeader'; +import { useStudioHome } from '../../studio-home/hooks'; import { useCreateLibraryV2 } from './data/apiHooks'; import messages from './messages'; @@ -38,10 +39,23 @@ const CreateLibrary = () => { } = useCreateLibraryV2(); const { - data: organizationListData, + data: allOrganizations, isLoading: isOrganizationListLoading, } = useOrganizationListData(); + const { + studioHomeData: { + allowedOrganizationsForLibraries, + allowToCreateNewOrg, + }, + } = useStudioHome(); + + const organizations = ( + allowToCreateNewOrg + ? allOrganizations + : allowedOrganizationsForLibraries + ) || []; + const handleOnClickCancel = () => { navigate('/libraries'); }; @@ -100,12 +114,17 @@ const CreateLibrary = () => { formikProps.setFieldValue('org', event.selectionId)} + onChange={(event) => formikProps.setFieldValue( + 'org', + allowToCreateNewOrg + ? (event.selectionId || event.userProvidedText) + : event.selectionId, + )} placeholder={intl.formatMessage(messages.orgPlaceholder)} > - {organizationListData ? organizationListData.map((org) => ( + {organizations.map((org) => ( {org} - )) : []} + ))} {intl.formatMessage(messages.orgHelp)} diff --git a/src/studio-home/__mocks__/studioHomeMock.js b/src/studio-home/__mocks__/studioHomeMock.js index dcd313c51..cc135f259 100644 --- a/src/studio-home/__mocks__/studioHomeMock.js +++ b/src/studio-home/__mocks__/studioHomeMock.js @@ -76,4 +76,5 @@ module.exports = { platformName: 'Your Platform Name Here', userIsActive: true, allowToCreateNewOrg: false, + allowedOrganizationsForLibraries: ['org1'], }; From 98fbcff842ed760337700e51204e17e940ee7603 Mon Sep 17 00:00:00 2001 From: Peter Kulko <93188219+PKulkoRaccoonGang@users.noreply.github.com> Date: Wed, 15 Jan 2025 20:28:43 +0200 Subject: [PATCH 09/18] feat: [FC-0070] listen to xblock interaction events (#1431) This is part of the effort to support the new embedded Studio Unit Page. It includes changes to the CourseUnit component and the functionality of interaction between xblocks in the iframe and the react page. The following events have been processed: * delete event * Manage Access event (opening and closing a modal window) * edit event for new xblock React editors * clipboard events * duplicate event --- src/constants.js | 4 + src/course-unit/CourseUnit.jsx | 2 + src/course-unit/CourseUnit.test.jsx | 495 +++++++++++++++++- src/course-unit/constants.js | 11 +- src/course-unit/data/slice.js | 18 - src/course-unit/data/thunk.js | 10 +- src/course-unit/header-title/HeaderTitle.jsx | 8 + .../header-title/HeaderTitle.test.jsx | 58 +- src/course-unit/sidebar/PublishControls.jsx | 9 +- .../xblock-container-iframe/hooks/index.ts | 5 + .../{ => hooks}/tests/hooks.test.tsx | 4 +- .../xblock-container-iframe/hooks/types.ts | 25 + .../useIFrameBehavior.tsx} | 57 +- .../hooks/useIframeContent.tsx | 33 ++ .../hooks/useIframeMessages.tsx | 20 + .../hooks/useLoadBearingHook.tsx | 33 ++ .../hooks/useMessageHandlers.tsx | 38 ++ .../xblock-container-iframe/index.tsx | 167 ++++-- .../xblock-container-iframe/messages.ts | 4 + .../tests/XblockContainerIframe.test.tsx | 47 -- .../xblock-container-iframe/types.ts | 106 ++++ .../xblock-container-iframe/utils.ts | 33 ++ 22 files changed, 994 insertions(+), 193 deletions(-) create mode 100644 src/course-unit/xblock-container-iframe/hooks/index.ts rename src/course-unit/xblock-container-iframe/{ => hooks}/tests/hooks.test.tsx (97%) create mode 100644 src/course-unit/xblock-container-iframe/hooks/types.ts rename src/course-unit/xblock-container-iframe/{hooks.tsx => hooks/useIFrameBehavior.tsx} (58%) create mode 100644 src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx create mode 100644 src/course-unit/xblock-container-iframe/hooks/useIframeMessages.tsx create mode 100644 src/course-unit/xblock-container-iframe/hooks/useLoadBearingHook.tsx create mode 100644 src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx delete mode 100644 src/course-unit/xblock-container-iframe/tests/XblockContainerIframe.test.tsx create mode 100644 src/course-unit/xblock-container-iframe/types.ts create mode 100644 src/course-unit/xblock-container-iframe/utils.ts diff --git a/src/constants.js b/src/constants.js index 163a16ef8..411d3f248 100644 --- a/src/constants.js +++ b/src/constants.js @@ -76,3 +76,7 @@ export const REGEX_RULES = { specialCharsRule: /^[a-zA-Z0-9_\-.'*~\s]+$/, noSpaceRule: /^\S*$/, }; + +export const IFRAME_FEATURE_POLICY = ( + 'microphone *; camera *; midi *; geolocation *; encrypted-media *; clipboard-write *' +); diff --git a/src/course-unit/CourseUnit.jsx b/src/course-unit/CourseUnit.jsx index 4c54d6775..b9cdff587 100644 --- a/src/course-unit/CourseUnit.jsx +++ b/src/course-unit/CourseUnit.jsx @@ -180,9 +180,11 @@ const CourseUnit = ({ courseId }) => { /> )} clipboardBroadcastChannelMock); +/** + * Simulates receiving a post message event for testing purposes. + * This can be used to mimic events like deletion or other actions + * sent from Backbone or other sources via postMessage. + * + * @param {string} type - The type of the message event (e.g., 'deleteXBlock'). + * @param {Object} payload - The payload data for the message event. + */ +function simulatePostMessageEvent(type, payload) { + const messageEvent = new MessageEvent('message', { + data: { type, payload }, + }); + + window.dispatchEvent(messageEvent); +} + const RootWrapper = () => ( @@ -175,6 +201,259 @@ describe('', () => { }); }); + it('renders the course unit iframe with correct attributes', async () => { + const { getByTitle } = render(); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute('src', `${getConfig().STUDIO_BASE_URL}/container_embed/${blockId}`); + expect(iframe).toHaveAttribute('allow', IFRAME_FEATURE_POLICY); + expect(iframe).toHaveAttribute('style', 'width: 100%; height: 0px;'); + expect(iframe).toHaveAttribute('scrolling', 'no'); + expect(iframe).toHaveAttribute('referrerpolicy', 'origin'); + expect(iframe).toHaveAttribute('loading', 'lazy'); + expect(iframe).toHaveAttribute('frameborder', '0'); + }); + }); + + it('adjusts iframe height dynamically based on courseXBlockDropdownHeight postMessage event', async () => { + const { getByTitle } = render(); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute('style', 'width: 100%; height: 0px;'); + simulatePostMessageEvent(messageTypes.toggleCourseXBlockDropdown, { + courseXBlockDropdownHeight: 200, + }); + expect(iframe).toHaveAttribute('style', 'width: 100%; height: 200px;'); + }); + }); + + it('checks whether xblock is removed when the corresponding delete button is clicked and the sidebar is the updated', async () => { + const { + getByTitle, getByText, queryByRole, getAllByRole, getByRole, + } = render(); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', courseVerticalChildrenMock.children.length), + ); + + simulatePostMessageEvent(messageTypes.deleteXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + + expect(getByText(/Delete this component?/i)).toBeInTheDocument(); + expect(getByText(/Deleting this component is permanent and cannot be undone./i)).toBeInTheDocument(); + + expect(getByRole('dialog')).toBeInTheDocument(); + + // Find the Cancel and Delete buttons within the iframe by their specific classes + const cancelButton = getAllByRole('button', { name: /Cancel/i }) + .find(({ classList }) => classList.contains('btn-tertiary')); + const deleteButton = getAllByRole('button', { name: /Delete/i }) + .find(({ classList }) => classList.contains('btn-primary')); + + userEvent.click(cancelButton); + + simulatePostMessageEvent(messageTypes.deleteXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + + expect(getByRole('dialog')).toBeInTheDocument(); + userEvent.click(deleteButton); + }); + + axiosMock + .onPost(getXBlockBaseApiUrl(blockId), { + publish: PUBLISH_TYPES.makePublic, + }) + .reply(200, { dummy: 'value' }); + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, { + ...courseUnitIndexMock, + visibility_state: UNIT_VISIBILITY_STATES.live, + has_changes: false, + published_by: userName, + }); + await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch); + + await waitFor(() => { + // check if the sidebar status is Published and Live + expect(getByText(sidebarMessages.sidebarTitlePublishedAndLive.defaultMessage)).toBeInTheDocument(); + expect(getByText( + sidebarMessages.publishLastPublished.defaultMessage + .replace('{publishedOn}', courseUnitIndexMock.published_on) + .replace('{publishedBy}', userName), + )).toBeInTheDocument(); + expect(queryByRole('button', { name: sidebarMessages.actionButtonPublishTitle.defaultMessage })).not.toBeInTheDocument(); + expect(getByText(unitDisplayName)).toBeInTheDocument(); + }); + + axiosMock + .onDelete(getXBlockBaseApiUrl(courseVerticalChildrenMock.children[0].block_id)) + .replyOnce(200, { dummy: 'value' }); + await executeThunk(deleteUnitItemQuery(courseId, blockId), store.dispatch); + + const updatedCourseVerticalChildren = courseVerticalChildrenMock.children.filter( + child => child.block_id !== courseVerticalChildrenMock.children[0].block_id, + ); + + axiosMock + .onGet(getCourseVerticalChildrenApiUrl(blockId)) + .reply(200, { + children: updatedCourseVerticalChildren, + isPublished: false, + canPasteComponent: true, + }); + await executeThunk(fetchCourseVerticalChildrenData(blockId), store.dispatch); + + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, courseUnitIndexMock); + await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', updatedCourseVerticalChildren.length), + ); + // after removing the xblock, the sidebar status changes to Draft (unpublished changes) + expect(getByText(sidebarMessages.sidebarTitleDraftUnpublishedChanges.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityStaffAndLearnersTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.releaseStatusTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.sidebarBodyNote.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityWillBeVisibleToTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityCheckboxTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.actionButtonPublishTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.actionButtonDiscardChangesTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(courseUnitIndexMock.release_date)).toBeInTheDocument(); + expect(getByText( + sidebarMessages.publishInfoDraftSaved.defaultMessage + .replace('{editedOn}', courseUnitIndexMock.edited_on) + .replace('{editedBy}', courseUnitIndexMock.edited_by), + )).toBeInTheDocument(); + expect(getByText( + sidebarMessages.releaseInfoWithSection.defaultMessage + .replace('{sectionName}', courseUnitIndexMock.release_date_from), + )).toBeInTheDocument(); + }); + }); + + it('checks if xblock is a duplicate when the corresponding duplicate button is clicked and if the sidebar status is updated', async () => { + const { + getByTitle, getByRole, getByText, queryByRole, + } = render(); + + simulatePostMessageEvent(messageTypes.duplicateXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + + axiosMock + .onPost(postXBlockBaseApiUrl({ + parent_locator: blockId, + duplicate_source_locator: courseVerticalChildrenMock.children[0].block_id, + })) + .replyOnce(200, { locator: '1234567890' }); + + const updatedCourseVerticalChildren = [ + ...courseVerticalChildrenMock.children, + { + ...courseVerticalChildrenMock.children[0], + name: 'New Cloned XBlock', + }, + ]; + + axiosMock + .onGet(getCourseVerticalChildrenApiUrl(blockId)) + .reply(200, { + ...courseVerticalChildrenMock, + children: updatedCourseVerticalChildren, + }); + + await waitFor(() => { + userEvent.click(getByRole('button', { name: sidebarMessages.actionButtonPublishTitle.defaultMessage })); + + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', courseVerticalChildrenMock.children.length), + ); + + simulatePostMessageEvent(messageTypes.duplicateXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + }); + + axiosMock + .onPost(getXBlockBaseApiUrl(blockId), { + publish: PUBLISH_TYPES.makePublic, + }) + .reply(200, { dummy: 'value' }); + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, { + ...courseUnitIndexMock, + visibility_state: UNIT_VISIBILITY_STATES.live, + has_changes: false, + published_by: userName, + }); + await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch); + + await waitFor(() => { + // check if the sidebar status is Published and Live + expect(getByText(sidebarMessages.sidebarTitlePublishedAndLive.defaultMessage)).toBeInTheDocument(); + expect(getByText( + sidebarMessages.publishLastPublished.defaultMessage + .replace('{publishedOn}', courseUnitIndexMock.published_on) + .replace('{publishedBy}', userName), + )).toBeInTheDocument(); + expect(queryByRole('button', { name: sidebarMessages.actionButtonPublishTitle.defaultMessage })).not.toBeInTheDocument(); + expect(getByText(unitDisplayName)).toBeInTheDocument(); + }); + + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, courseUnitIndexMock); + await executeThunk(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.makePublic, true), store.dispatch); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', updatedCourseVerticalChildren.length), + ); + + // after duplicate the xblock, the sidebar status changes to Draft (unpublished changes) + expect(getByText(sidebarMessages.sidebarTitleDraftUnpublishedChanges.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityStaffAndLearnersTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.releaseStatusTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.sidebarBodyNote.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityWillBeVisibleToTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.visibilityCheckboxTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.actionButtonPublishTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(sidebarMessages.actionButtonDiscardChangesTitle.defaultMessage)).toBeInTheDocument(); + expect(getByText(courseUnitIndexMock.release_date)).toBeInTheDocument(); + expect(getByText( + sidebarMessages.publishInfoDraftSaved.defaultMessage + .replace('{editedOn}', courseUnitIndexMock.edited_on) + .replace('{editedBy}', courseUnitIndexMock.edited_by), + )).toBeInTheDocument(); + expect(getByText( + sidebarMessages.releaseInfoWithSection.defaultMessage + .replace('{sectionName}', courseUnitIndexMock.release_date_from), + )).toBeInTheDocument(); + }); + }); + it('handles CourseUnit header action buttons', async () => { const { open } = window; window.open = jest.fn(); @@ -877,6 +1156,77 @@ describe('', () => { .toHaveBeenCalledWith(`/course/${courseId}/container/${blockId}/${updatedAncestorsChild.id}`, { replace: true }); }); + it('should increase the number of course XBlocks after copying and pasting a block', async () => { + const { getByRole, getByTitle } = render(); + + simulatePostMessageEvent(messageTypes.copyXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + + axiosMock + .onGet(getCourseSectionVerticalApiUrl(blockId)) + .reply(200, { + ...courseSectionVerticalMock, + user_clipboard: clipboardXBlock, + }); + axiosMock + .onGet(getCourseUnitApiUrl(courseId)) + .reply(200, { + ...courseUnitIndexMock, + enable_copy_paste_units: true, + }); + await executeThunk(fetchCourseUnitQuery(courseId), store.dispatch); + await executeThunk(fetchCourseSectionVerticalData(blockId), store.dispatch); + + userEvent.click(getByRole('button', { name: sidebarMessages.actionButtonCopyUnitTitle.defaultMessage })); + userEvent.click(getByRole('button', { name: messages.pasteButtonText.defaultMessage })); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', courseVerticalChildrenMock.children.length), + ); + + simulatePostMessageEvent(messageTypes.copyXBlock, { + id: courseVerticalChildrenMock.children[0].block_id, + }); + }); + + const updatedCourseVerticalChildren = [ + ...courseVerticalChildrenMock.children, + { + name: 'Copy XBlock', + block_id: '1234567890', + block_type: 'drag-and-drop-v2', + user_partition_info: { + selectable_partitions: [], + selected_partition_index: -1, + selected_groups_label: '', + }, + }, + ]; + + axiosMock + .onGet(getCourseVerticalChildrenApiUrl(blockId)) + .reply(200, { + ...courseVerticalChildrenMock, + children: updatedCourseVerticalChildren, + }); + + await executeThunk(fetchCourseVerticalChildrenData(blockId), store.dispatch); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toHaveAttribute( + 'aria-label', + xblockContainerIframeMessages.xblockIframeLabel.defaultMessage + .replace('{xblockCount}', updatedCourseVerticalChildren.length), + ); + }); + }); + it('displays a notification about new files after pasting a component', async () => { const { queryByTestId, getByTestId, getByRole, @@ -1324,4 +1674,147 @@ describe('', () => { ); }); }); + + describe('XBlock restrict access', () => { + it('opens xblock restrict access modal successfully', () => { + const { + getByTitle, getByTestId, + } = render(); + + const modalSubtitleText = configureModalMessages.restrictAccessTo.defaultMessage; + const modalCancelBtnText = configureModalMessages.cancelButton.defaultMessage; + const modalSaveBtnText = configureModalMessages.saveButton.defaultMessage; + + waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + const usageId = courseVerticalChildrenMock.children[0].block_id; + expect(iframe).toBeInTheDocument(); + + simulatePostMessageEvent(messageTypes.manageXBlockAccess, { + usageId, + }); + }); + + waitFor(() => { + const configureModal = getByTestId('configure-modal'); + + expect(within(configureModal).getByText(modalSubtitleText)).toBeInTheDocument(); + expect(within(configureModal).getByRole('button', { name: modalCancelBtnText })).toBeInTheDocument(); + expect(within(configureModal).getByRole('button', { name: modalSaveBtnText })).toBeInTheDocument(); + }); + }); + + it('closes xblock restrict access modal when cancel button is clicked', async () => { + const { + getByTitle, queryByTestId, getByTestId, + } = render(); + + waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.manageXBlockAccess, { + usageId: courseVerticalChildrenMock.children[0].block_id, + }); + }); + + waitFor(() => { + const configureModal = getByTestId('configure-modal'); + expect(configureModal).toBeInTheDocument(); + userEvent.click(within(configureModal).getByRole('button', { + name: configureModalMessages.cancelButton.defaultMessage, + })); + expect(handleConfigureSubmitMock).not.toHaveBeenCalled(); + }); + + expect(queryByTestId('configure-modal')).not.toBeInTheDocument(); + }); + + it('handles submit xblock restrict access data when save button is clicked', async () => { + axiosMock + .onPost(getXBlockBaseApiUrl(id), { + publish: PUBLISH_TYPES.republish, + metadata: { visible_to_staff_only: false, group_access: { 970807507: [1959537066] } }, + }) + .reply(200, { dummy: 'value' }); + + const { + getByTitle, getByRole, getByTestId, + } = render(); + + const accessGroupName1 = userPartitionInfoFormatted.selectablePartitions[0].groups[0].name; + const accessGroupName2 = userPartitionInfoFormatted.selectablePartitions[0].groups[1].name; + + waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.manageXBlockAccess, { + usageId: courseVerticalChildrenMock.children[0].block_id, + }); + }); + + waitFor(() => { + const configureModal = getByTestId('configure-modal'); + expect(configureModal).toBeInTheDocument(); + + expect(within(configureModal).queryByText(accessGroupName1)).not.toBeInTheDocument(); + expect(within(configureModal).queryByText(accessGroupName2)).not.toBeInTheDocument(); + + const restrictAccessSelect = getByRole('combobox', { + name: configureModalMessages.restrictAccessTo.defaultMessage, + }); + + userEvent.selectOptions(restrictAccessSelect, '0'); + + // eslint-disable-next-line array-callback-return + userPartitionInfoFormatted.selectablePartitions[0].groups.map((group) => { + expect(within(configureModal).getByRole('checkbox', { name: group.name })).not.toBeChecked(); + expect(within(configureModal).queryByText(group.name)).toBeInTheDocument(); + }); + + const group1Checkbox = within(configureModal).getByRole('checkbox', { name: accessGroupName1 }); + userEvent.click(group1Checkbox); + expect(group1Checkbox).toBeChecked(); + + const saveModalBtnText = within(configureModal).getByRole('button', { + name: configureModalMessages.saveButton.defaultMessage, + }); + expect(saveModalBtnText).toBeInTheDocument(); + + userEvent.click(saveModalBtnText); + expect(handleConfigureSubmitMock).toHaveBeenCalledTimes(1); + }); + }); + }); + + it('renders and navigates to the new HTML XBlock editor after xblock duplicating', async () => { + const { getByTitle } = render(); + const updatedCourseVerticalChildrenMock = JSON.parse(JSON.stringify(courseVerticalChildrenMock)); + const targetBlockId = updatedCourseVerticalChildrenMock.children[1].block_id; + + updatedCourseVerticalChildrenMock.children = updatedCourseVerticalChildrenMock.children + .map((child) => (child.block_id === targetBlockId + ? { ...child, block_type: 'html' } + : child)); + + axiosMock + .onGet(getCourseVerticalChildrenApiUrl(blockId)) + .reply(200, updatedCourseVerticalChildrenMock); + + await executeThunk(fetchCourseVerticalChildrenData(blockId), store.dispatch); + + await waitFor(() => { + const iframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(iframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.currentXBlockId, { + id: targetBlockId, + }); + }); + + waitFor(() => { + simulatePostMessageEvent(messageTypes.duplicateXBlock, {}); + simulatePostMessageEvent(messageTypes.newXBlockEditor, {}); + expect(mockedUsedNavigate) + .toHaveBeenCalledWith(`/course/${courseId}/editor/html/${targetBlockId}`, { replace: true }); + }); + }); }); diff --git a/src/course-unit/constants.js b/src/course-unit/constants.js index 129fa55d9..37c360900 100644 --- a/src/course-unit/constants.js +++ b/src/course-unit/constants.js @@ -55,8 +55,11 @@ export const messageTypes = { showMultipleComponentPicker: 'showMultipleComponentPicker', addSelectedComponentsToBank: 'addSelectedComponentsToBank', showXBlockLibraryChangesPreview: 'showXBlockLibraryChangesPreview', + copyXBlock: 'copyXBlock', + manageXBlockAccess: 'manageXBlockAccess', + deleteXBlock: 'deleteXBlock', + duplicateXBlock: 'duplicateXBlock', + refreshXBlockPositions: 'refreshPositions', + newXBlockEditor: 'newXBlockEditor', + toggleCourseXBlockDropdown: 'toggleCourseXBlockDropdown', }; - -export const IFRAME_FEATURE_POLICY = ( - 'microphone *; camera *; midi *; geolocation *; encrypted-media *, clipboard-write *' -); diff --git a/src/course-unit/data/slice.js b/src/course-unit/data/slice.js index aab66ea26..1755d0960 100644 --- a/src/course-unit/data/slice.js +++ b/src/course-unit/data/slice.js @@ -93,22 +93,6 @@ const slice = createSlice({ updateCourseVerticalChildrenLoadingStatus: (state, { payload }) => { state.loadingStatus.courseVerticalChildrenLoadingStatus = payload.status; }, - deleteXBlock: (state, { payload }) => { - state.courseVerticalChildren.children = state.courseVerticalChildren.children.filter( - (component) => component.id !== payload, - ); - }, - duplicateXBlock: (state, { payload }) => { - state.courseVerticalChildren = { - ...payload.newCourseVerticalChildren, - children: payload.newCourseVerticalChildren.children.map((component) => { - if (component.blockId === payload.newId) { - component.shouldScroll = true; - } - return component; - }), - }; - }, fetchStaticFileNoticesSuccess: (state, { payload }) => { state.staticFileNotices = payload; }, @@ -139,8 +123,6 @@ export const { updateLoadingCourseXblockStatus, updateCourseVerticalChildren, updateCourseVerticalChildrenLoadingStatus, - deleteXBlock, - duplicateXBlock, fetchStaticFileNoticesSuccess, updateCourseOutlineInfo, updateCourseOutlineInfoLoadingStatus, diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js index a0d421eea..8f560e75a 100644 --- a/src/course-unit/data/thunk.js +++ b/src/course-unit/data/thunk.js @@ -34,8 +34,6 @@ import { updateCourseVerticalChildren, updateCourseVerticalChildrenLoadingStatus, updateQueryPendingStatus, - deleteXBlock, - duplicateXBlock, fetchStaticFileNoticesSuccess, updateCourseOutlineInfo, updateCourseOutlineInfoLoadingStatus, @@ -229,7 +227,6 @@ export function deleteUnitItemQuery(itemId, xblockId) { try { await deleteUnitItem(xblockId); - dispatch(deleteXBlock(xblockId)); const { userClipboard } = await getCourseSectionVerticalData(itemId); dispatch(updateClipboardData(userClipboard)); const courseUnit = await getCourseUnitData(itemId); @@ -249,12 +246,7 @@ export function duplicateUnitItemQuery(itemId, xblockId) { dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.duplicating)); try { - const { locator } = await duplicateUnitItem(itemId, xblockId); - const newCourseVerticalChildren = await getCourseVerticalChildren(itemId); - dispatch(duplicateXBlock({ - newId: locator, - newCourseVerticalChildren, - })); + await duplicateUnitItem(itemId, xblockId); const courseUnit = await getCourseUnitData(itemId); dispatch(fetchCourseItemSuccess(courseUnit)); dispatch(hideProcessingNotification()); diff --git a/src/course-unit/header-title/HeaderTitle.jsx b/src/course-unit/header-title/HeaderTitle.jsx index 336d986fa..bb3cd3a72 100644 --- a/src/course-unit/header-title/HeaderTitle.jsx +++ b/src/course-unit/header-title/HeaderTitle.jsx @@ -11,6 +11,8 @@ import { import ConfigureModal from '../../generic/configure-modal/ConfigureModal'; import { getCourseUnitData } from '../data/selectors'; import { updateQueryPendingStatus } from '../data/slice'; +import { messageTypes } from '../constants'; +import { useIframe } from '../context/hooks'; import messages from './messages'; const HeaderTitle = ({ @@ -26,9 +28,15 @@ const HeaderTitle = ({ const currentItemData = useSelector(getCourseUnitData); const [isConfigureModalOpen, openConfigureModal, closeConfigureModal] = useToggle(false); const { selectedPartitionIndex, selectedGroupsLabel } = currentItemData.userPartitionInfo; + const { sendMessageToIframe } = useIframe(); const onConfigureSubmit = (...arg) => { handleConfigureSubmit(currentItemData.id, ...arg, closeConfigureModal); + // TODO: this artificial delay is a temporary solution + // to ensure the iframe content is properly refreshed. + setTimeout(() => { + sendMessageToIframe(messageTypes.refreshXBlock, null); + }, 1000); }; const getVisibilityMessage = () => { diff --git a/src/course-unit/header-title/HeaderTitle.test.jsx b/src/course-unit/header-title/HeaderTitle.test.jsx index 7e57c408e..4383fcf6c 100644 --- a/src/course-unit/header-title/HeaderTitle.test.jsx +++ b/src/course-unit/header-title/HeaderTitle.test.jsx @@ -1,6 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { render } from '@testing-library/react'; +import { render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { AppProvider } from '@edx/frontend-platform/react'; @@ -60,9 +60,11 @@ describe('', () => { it('render HeaderTitle component correctly', () => { const { getByText, getByRole } = renderComponent(); - expect(getByText(unitTitle)).toBeInTheDocument(); - expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument(); - expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument(); + waitFor(() => { + expect(getByText(unitTitle)).toBeInTheDocument(); + expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument(); + expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument(); + }); }); it('render HeaderTitle with open edit form', () => { @@ -70,18 +72,22 @@ describe('', () => { isTitleEditFormOpen: true, }); - expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toBeInTheDocument(); - expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toHaveValue(unitTitle); - expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument(); - expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument(); + waitFor(() => { + expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toBeInTheDocument(); + expect(getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage })).toHaveValue(unitTitle); + expect(getByRole('button', { name: messages.altButtonEdit.defaultMessage })).toBeInTheDocument(); + expect(getByRole('button', { name: messages.altButtonSettings.defaultMessage })).toBeInTheDocument(); + }); }); it('calls toggle edit title form by clicking on Edit button', () => { const { getByRole } = renderComponent(); - const editTitleButton = getByRole('button', { name: messages.altButtonEdit.defaultMessage }); - userEvent.click(editTitleButton); - expect(handleTitleEdit).toHaveBeenCalledTimes(1); + waitFor(() => { + const editTitleButton = getByRole('button', { name: messages.altButtonEdit.defaultMessage }); + userEvent.click(editTitleButton); + expect(handleTitleEdit).toHaveBeenCalledTimes(1); + }); }); it('calls saving title by clicking outside or press Enter key', async () => { @@ -89,16 +95,18 @@ describe('', () => { isTitleEditFormOpen: true, }); - const titleField = getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage }); - userEvent.type(titleField, ' 1'); - expect(titleField).toHaveValue(`${unitTitle} 1`); - userEvent.click(document.body); - expect(handleTitleEditSubmit).toHaveBeenCalledTimes(1); - - userEvent.click(titleField); - userEvent.type(titleField, ' 2[Enter]'); - expect(titleField).toHaveValue(`${unitTitle} 1 2`); - expect(handleTitleEditSubmit).toHaveBeenCalledTimes(2); + waitFor(() => { + const titleField = getByRole('textbox', { name: messages.ariaLabelButtonEdit.defaultMessage }); + userEvent.type(titleField, ' 1'); + expect(titleField).toHaveValue(`${unitTitle} 1`); + userEvent.click(document.body); + expect(handleTitleEditSubmit).toHaveBeenCalledTimes(1); + + userEvent.click(titleField); + userEvent.type(titleField, ' 2[Enter]'); + expect(titleField).toHaveValue(`${unitTitle} 1 2`); + expect(handleTitleEditSubmit).toHaveBeenCalledTimes(2); + }); }); it('displays a visibility message with the selected groups for the unit', async () => { @@ -117,7 +125,9 @@ describe('', () => { const visibilityMessage = messages.definedVisibilityMessage.defaultMessage .replace('{selectedGroupsLabel}', 'Visibility group 1'); - expect(getByText(visibilityMessage)).toBeInTheDocument(); + waitFor(() => { + expect(getByText(visibilityMessage)).toBeInTheDocument(); + }); }); it('displays a visibility message with the selected groups for some of xblock', async () => { @@ -130,6 +140,8 @@ describe('', () => { await executeThunk(fetchCourseUnitQuery(blockId), store.dispatch); const { getByText } = renderComponent(); - expect(getByText(messages.commonVisibilityMessage.defaultMessage)).toBeInTheDocument(); + waitFor(() => { + expect(getByText(messages.someVisibilityMessage.defaultMessage)).toBeInTheDocument(); + }); }); }); diff --git a/src/course-unit/sidebar/PublishControls.jsx b/src/course-unit/sidebar/PublishControls.jsx index 424594f35..0ef08baf2 100644 --- a/src/course-unit/sidebar/PublishControls.jsx +++ b/src/course-unit/sidebar/PublishControls.jsx @@ -4,9 +4,10 @@ import { useToggle } from '@openedx/paragon'; import { InfoOutline as InfoOutlineIcon } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import useCourseUnitData from './hooks'; +import { useIframe } from '../context/hooks'; import { editCourseUnitVisibilityAndData } from '../data/thunk'; import { SidebarBody, SidebarFooter, SidebarHeader } from './components'; -import { PUBLISH_TYPES } from '../constants'; +import { PUBLISH_TYPES, messageTypes } from '../constants'; import { getCourseUnitData } from '../data/selectors'; import messages from './messages'; import ModalNotification from '../../generic/modal-notification'; @@ -20,6 +21,7 @@ const PublishControls = ({ blockId }) => { visibleToStaffOnly, } = useCourseUnitData(useSelector(getCourseUnitData)); const intl = useIntl(); + const { sendMessageToIframe } = useIframe(); const [isDiscardModalOpen, openDiscardModal, closeDiscardModal] = useToggle(false); const [isVisibleModalOpen, openVisibleModal, closeVisibleModal] = useToggle(false); @@ -34,6 +36,11 @@ const PublishControls = ({ blockId }) => { const handleCourseUnitDiscardChanges = () => { closeDiscardModal(); dispatch(editCourseUnitVisibilityAndData(blockId, PUBLISH_TYPES.discardChanges)); + // TODO: this artificial delay is a temporary solution + // to ensure the iframe content is properly refreshed. + setTimeout(() => { + sendMessageToIframe(messageTypes.refreshXBlock, null); + }, 1000); }; const handleCourseUnitPublish = () => { diff --git a/src/course-unit/xblock-container-iframe/hooks/index.ts b/src/course-unit/xblock-container-iframe/hooks/index.ts new file mode 100644 index 000000000..c49993dc1 --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/index.ts @@ -0,0 +1,5 @@ +export { useIframeMessages } from './useIframeMessages'; +export { useIframeContent } from './useIframeContent'; +export { useMessageHandlers } from './useMessageHandlers'; +export { useIFrameBehavior } from './useIFrameBehavior'; +export { useLoadBearingHook } from './useLoadBearingHook'; diff --git a/src/course-unit/xblock-container-iframe/tests/hooks.test.tsx b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx similarity index 97% rename from src/course-unit/xblock-container-iframe/tests/hooks.test.tsx rename to src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx index 13b546762..8883efb04 100644 --- a/src/course-unit/xblock-container-iframe/tests/hooks.test.tsx +++ b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx @@ -3,8 +3,8 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { useKeyedState } from '@edx/react-unit-test-utils'; import { logError } from '@edx/frontend-platform/logging'; -import { stateKeys, messageTypes } from '../../constants'; -import { useIFrameBehavior, useLoadBearingHook } from '../hooks'; +import { stateKeys, messageTypes } from '../../../constants'; +import { useLoadBearingHook, useIFrameBehavior } from '..'; jest.mock('@edx/react-unit-test-utils', () => ({ useKeyedState: jest.fn(), diff --git a/src/course-unit/xblock-container-iframe/hooks/types.ts b/src/course-unit/xblock-container-iframe/hooks/types.ts new file mode 100644 index 000000000..3974656c4 --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/types.ts @@ -0,0 +1,25 @@ +export type UseMessageHandlersTypes = { + courseId: string; + navigate: (path: string) => void; + dispatch: (action: any) => void; + setIframeOffset: (height: number) => void; + handleDeleteXBlock: (usageId: string) => void; + handleRefetchXBlocks: () => void; + handleDuplicateXBlock: (blockType: string, usageId: string) => void; + handleManageXBlockAccess: (usageId: string) => void; +}; + +export type MessageHandlersTypes = Record void>; + +export interface UseIFrameBehaviorTypes { + id: string; + iframeUrl: string; + onLoaded?: boolean; +} + +export interface UseIFrameBehaviorReturnTypes { + iframeHeight: number; + handleIFrameLoad: () => void; + showError: boolean; + hasLoaded: boolean; +} diff --git a/src/course-unit/xblock-container-iframe/hooks.tsx b/src/course-unit/xblock-container-iframe/hooks/useIFrameBehavior.tsx similarity index 58% rename from src/course-unit/xblock-container-iframe/hooks.tsx rename to src/course-unit/xblock-container-iframe/hooks/useIFrameBehavior.tsx index 1a81e7852..832ac94cd 100644 --- a/src/course-unit/xblock-container-iframe/hooks.tsx +++ b/src/course-unit/xblock-container-iframe/hooks/useIFrameBehavior.tsx @@ -1,57 +1,12 @@ -import { - useState, useLayoutEffect, useCallback, useEffect, -} from 'react'; +import { useCallback, useEffect } from 'react'; import { logError } from '@edx/frontend-platform/logging'; // eslint-disable-next-line import/no-extraneous-dependencies import { useKeyedState } from '@edx/react-unit-test-utils'; -import { useEventListener } from '../../generic/hooks'; -import { stateKeys, messageTypes } from '../constants'; - -interface UseIFrameBehaviorParams { - id: string; - iframeUrl: string; - onLoaded?: boolean; -} - -interface UseIFrameBehaviorReturn { - iframeHeight: number; - handleIFrameLoad: () => void; - showError: boolean; - hasLoaded: boolean; -} - -/** - * We discovered an error in Firefox where - upon iframe load - React would cease to call any - * useEffect hooks until the user interacts with the page again. This is particularly confusing - * when navigating between sequences, as the UI partially updates leaving the user in a nebulous - * state. - * - * We were able to solve this error by using a layout effect to update some component state, which - * executes synchronously on render. Somehow this forces React to continue it's lifecycle - * immediately, rather than waiting for user interaction. This layout effect could be anywhere in - * the parent tree, as far as we can tell - we chose to add a conspicuously 'load bearing' (that's - * a joke) one here so it wouldn't be accidentally removed elsewhere. - * - * If we remove this hook when one of these happens: - * 1. React figures out that there's an issue here and fixes a bug. - * 2. We cease to use an iframe for unit rendering. - * 3. Firefox figures out that there's an issue in their iframe loading and fixes a bug. - * 4. We stop supporting Firefox. - * 5. An enterprising engineer decides to create a repo that reproduces the problem, submits it to - * Firefox/React for review, and they kindly help us figure out what in the world is happening - * so we can fix it. - * - * This hook depends on the unit id just to make sure it re-evaluates whenever the ID changes. If - * we change whether or not the Unit component is re-mounted when the unit ID changes, this may - * become important, as this hook will otherwise only evaluate the useLayoutEffect once. - */ -export const useLoadBearingHook = (id: string): void => { - const setValue = useState(0)[1]; - useLayoutEffect(() => { - setValue(currentValue => currentValue + 1); - }, [id]); -}; +import { useEventListener } from '../../../generic/hooks'; +import { stateKeys, messageTypes } from '../../constants'; +import { useLoadBearingHook } from './useLoadBearingHook'; +import { UseIFrameBehaviorTypes, UseIFrameBehaviorReturnTypes } from './types'; /** * Custom hook to manage iframe behavior. @@ -70,7 +25,7 @@ export const useIFrameBehavior = ({ id, iframeUrl, onLoaded = true, -}: UseIFrameBehaviorParams): UseIFrameBehaviorReturn => { +}: UseIFrameBehaviorTypes): UseIFrameBehaviorReturnTypes => { // Do not remove this hook. See function description. useLoadBearingHook(id); diff --git a/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx b/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx new file mode 100644 index 000000000..abbc98b21 --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/useIframeContent.tsx @@ -0,0 +1,33 @@ +import { useEffect, useCallback, RefObject } from 'react'; + +import { messageTypes } from '../../constants'; + +/** + * Hook for managing iframe content and providing utilities to interact with the iframe. + * + * @param {React.RefObject} iframeRef - A React ref for the iframe element. + * @param {(ref: React.RefObject) => void} setIframeRef - + * A function to associate the iframeRef with the parent context. + * @param {(type: string, payload: any) => void} sendMessageToIframe - A function to send messages to the iframe. + * + * @returns {Object} - An object containing utility functions. + * @returns {() => void} return.refreshIframeContent - + * A function to refresh the iframe content by sending a specific message. + */ +export const useIframeContent = ( + iframeRef: RefObject, + setIframeRef: (ref: RefObject) => void, + sendMessageToIframe: (type: string, payload: any) => void, +): { refreshIframeContent: () => void } => { + useEffect(() => { + setIframeRef(iframeRef); + }, [setIframeRef, iframeRef]); + + // TODO: this artificial delay is a temporary solution + // to ensure the iframe content is properly refreshed. + const refreshIframeContent = useCallback(() => { + setTimeout(() => sendMessageToIframe(messageTypes.refreshXBlock, null), 1000); + }, [sendMessageToIframe]); + + return { refreshIframeContent }; +}; diff --git a/src/course-unit/xblock-container-iframe/hooks/useIframeMessages.tsx b/src/course-unit/xblock-container-iframe/hooks/useIframeMessages.tsx new file mode 100644 index 000000000..6f192615d --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/useIframeMessages.tsx @@ -0,0 +1,20 @@ +import { useEffect } from 'react'; + +/** + * Hook for managing and handling messages received by the iframe. + * + * @param {Record void>} messageHandlers - + * A mapping of message types to their corresponding handler functions. + */ +export const useIframeMessages = (messageHandlers: Record void>) => { + useEffect(() => { + const handleMessage = (event: MessageEvent) => { + const { type, payload } = event.data || {}; + if (type in messageHandlers) { + messageHandlers[type](payload); + } + }; + window.addEventListener('message', handleMessage); + return () => window.removeEventListener('message', handleMessage); + }, [messageHandlers]); +}; diff --git a/src/course-unit/xblock-container-iframe/hooks/useLoadBearingHook.tsx b/src/course-unit/xblock-container-iframe/hooks/useLoadBearingHook.tsx new file mode 100644 index 000000000..73a38b022 --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/useLoadBearingHook.tsx @@ -0,0 +1,33 @@ +import { useLayoutEffect, useState } from 'react'; + +/** + * We discovered an error in Firefox where - upon iframe load - React would cease to call any + * useEffect hooks until the user interacts with the page again. This is particularly confusing + * when navigating between sequences, as the UI partially updates leaving the user in a nebulous + * state. + * + * We were able to solve this error by using a layout effect to update some component state, which + * executes synchronously on render. Somehow this forces React to continue it's lifecycle + * immediately, rather than waiting for user interaction. This layout effect could be anywhere in + * the parent tree, as far as we can tell - we chose to add a conspicuously 'load bearing' (that's + * a joke) one here so it wouldn't be accidentally removed elsewhere. + * + * If we remove this hook when one of these happens: + * 1. React figures out that there's an issue here and fixes a bug. + * 2. We cease to use an iframe for unit rendering. + * 3. Firefox figures out that there's an issue in their iframe loading and fixes a bug. + * 4. We stop supporting Firefox. + * 5. An enterprising engineer decides to create a repo that reproduces the problem, submits it to + * Firefox/React for review, and they kindly help us figure out what in the world is happening + * so we can fix it. + * + * This hook depends on the unit id just to make sure it re-evaluates whenever the ID changes. If + * we change whether or not the Unit component is re-mounted when the unit ID changes, this may + * become important, as this hook will otherwise only evaluate the useLayoutEffect once. + */ +export const useLoadBearingHook = (id: string): void => { + const setValue = useState(0)[1]; + useLayoutEffect(() => { + setValue(currentValue => currentValue + 1); + }, [id]); +}; diff --git a/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx new file mode 100644 index 000000000..974f7bf0c --- /dev/null +++ b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx @@ -0,0 +1,38 @@ +import { useMemo } from 'react'; + +import { copyToClipboard } from '../../../generic/data/thunks'; +import { messageTypes } from '../../constants'; +import { MessageHandlersTypes, UseMessageHandlersTypes } from './types'; + +/** + * Hook for creating message handlers used to handle iframe messages. + * + * @param params - The parameters required to create message handlers. + * @returns {MessageHandlersTypes} - An object mapping message types to their handler functions. + */ +export const useMessageHandlers = ({ + courseId, + navigate, + dispatch, + setIframeOffset, + handleDeleteXBlock, + handleRefetchXBlocks, + handleDuplicateXBlock, + handleManageXBlockAccess, +}: UseMessageHandlersTypes): MessageHandlersTypes => useMemo(() => ({ + [messageTypes.copyXBlock]: ({ usageId }) => dispatch(copyToClipboard(usageId)), + [messageTypes.deleteXBlock]: ({ usageId }) => handleDeleteXBlock(usageId), + [messageTypes.newXBlockEditor]: ({ blockType, usageId }) => navigate(`/course/${courseId}/editor/${blockType}/${usageId}`), + [messageTypes.duplicateXBlock]: ({ blockType, usageId }) => handleDuplicateXBlock(blockType, usageId), + [messageTypes.manageXBlockAccess]: ({ usageId }) => handleManageXBlockAccess(usageId), + [messageTypes.refreshXBlockPositions]: handleRefetchXBlocks, + [messageTypes.toggleCourseXBlockDropdown]: ({ + courseXBlockDropdownHeight, + }: { courseXBlockDropdownHeight: number }) => setIframeOffset(courseXBlockDropdownHeight), +}), [ + courseId, + handleDeleteXBlock, + handleRefetchXBlocks, + handleDuplicateXBlock, + handleManageXBlockAccess, +]); diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx index 761d63775..9b47a32d0 100644 --- a/src/course-unit/xblock-container-iframe/index.tsx +++ b/src/course-unit/xblock-container-iframe/index.tsx @@ -1,57 +1,150 @@ -import { useRef, useEffect, FC } from 'react'; -import PropTypes from 'prop-types'; +import { + useRef, FC, useEffect, useState, useMemo, useCallback, +} from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { getConfig } from '@edx/frontend-platform'; +import { useToggle } from '@openedx/paragon'; +import { useDispatch } from 'react-redux'; +import { useNavigate } from 'react-router-dom'; -import { IFRAME_FEATURE_POLICY } from '../constants'; +import DeleteModal from '../../generic/delete-modal/DeleteModal'; +import ConfigureModal from '../../generic/configure-modal/ConfigureModal'; +import { IFRAME_FEATURE_POLICY } from '../../constants'; +import supportedEditors from '../../editors/supportedEditors'; +import { fetchCourseUnitQuery } from '../data/thunk'; import { useIframe } from '../context/hooks'; -import { useIFrameBehavior } from './hooks'; +import { + useMessageHandlers, + useIframeContent, + useIframeMessages, + useIFrameBehavior, +} from './hooks'; +import { formatAccessManagedXBlockData, getIframeUrl } from './utils'; import messages from './messages'; -/** - * This offset is necessary to fully display the dropdown actions of the XBlock - * in case the XBlock does not have content inside. - */ -const IFRAME_BOTTOM_OFFSET = 220; +import { + XBlockContainerIframeProps, + AccessManagedXBlockDataTypes, +} from './types'; -interface XBlockContainerIframeProps { - blockId: string; -} - -const XBlockContainerIframe: FC = ({ blockId }) => { +const XBlockContainerIframe: FC = ({ + courseId, blockId, unitXBlockActions, courseVerticalChildren, handleConfigureSubmit, +}) => { const intl = useIntl(); const iframeRef = useRef(null); - const { setIframeRef } = useIframe(); + const dispatch = useDispatch(); + const navigate = useNavigate(); - const iframeUrl = `${getConfig().STUDIO_BASE_URL}/container_embed/${blockId}`; + const [isDeleteModalOpen, openDeleteModal, closeDeleteModal] = useToggle(false); + const [isConfigureModalOpen, openConfigureModal, closeConfigureModal] = useToggle(false); + const [accessManagedXBlockData, setAccessManagedXBlockData] = useState({}); + const [iframeOffset, setIframeOffset] = useState(0); + const [deleteXBlockId, setDeleteXBlockId] = useState(null); + const [configureXBlockId, setConfigureXBlockId] = useState(null); - const { iframeHeight } = useIFrameBehavior({ - id: blockId, - iframeUrl, - }); + const iframeUrl = useMemo(() => getIframeUrl(blockId), [blockId]); + + const { setIframeRef, sendMessageToIframe } = useIframe(); + const { iframeHeight } = useIFrameBehavior({ id: blockId, iframeUrl }); + const { refreshIframeContent } = useIframeContent(iframeRef, setIframeRef, sendMessageToIframe); useEffect(() => { setIframeRef(iframeRef); }, [setIframeRef]); - return ( -