Skip to content

Commit

Permalink
feat: shareable URLs for library components and searches [FC-0076] (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
pomegranited authored Jan 10, 2025
1 parent f586b09 commit 811be22
Show file tree
Hide file tree
Showing 31 changed files with 1,140 additions and 393 deletions.
2 changes: 1 addition & 1 deletion src/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
117 changes: 115 additions & 2 deletions src/hooks.ts
Original file line number Diff line number Diff line change
@@ -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<string | null>(null);
Expand Down Expand Up @@ -77,3 +84,109 @@ export const useLoadOnScroll = (
return () => { };
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);
};

/**
* Types used by the useStateWithUrlSearchParam hook.
*/
export type FromStringFn<Type> = (value: string | null) => Type | undefined;
export type ToStringFn<Type> = (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<Type> 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<Type>(
defaultValue: Type[],
paramName: string,
fromString: FromStringFn<Type>,
toString: ToStringFn<Type>,
): [value: Type[], setter: Dispatch<SetStateAction<Type[]>>];
export function useStateWithUrlSearchParam<Type>(
defaultValue: Type,
paramName: string,
fromString: FromStringFn<Type>,
toString: ToStringFn<Type>,
): [value: Type, setter: Dispatch<SetStateAction<Type>>];
export function useStateWithUrlSearchParam<Type>(
defaultValue: Type | Type[],
paramName: string,
fromString: FromStringFn<Type>,
toString: ToStringFn<Type>,
): [value: Type | Type[], setter: Dispatch<SetStateAction<Type | Type[]>>] {
// 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<SetStateAction<Type | Type[]>> = useCallback<ReturnSetterParams>((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];
}
65 changes: 48 additions & 17 deletions src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('<LibraryAuthoringPage />', () => {
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 });
Expand All @@ -151,7 +151,7 @@ describe('<LibraryAuthoringPage />', () => {
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);
Expand Down Expand Up @@ -196,15 +196,15 @@ describe('<LibraryAuthoringPage />', () => {
// 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
Expand Down Expand Up @@ -431,27 +431,23 @@ describe('<LibraryAuthoringPage />', () => {
// 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 () => {
Expand Down Expand Up @@ -505,6 +501,15 @@ describe('<LibraryAuthoringPage />', () => {
// 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];
Expand All @@ -528,15 +533,13 @@ describe('<LibraryAuthoringPage />', () => {
});

// 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(),
});
Expand Down Expand Up @@ -706,6 +709,34 @@ describe('<LibraryAuthoringPage />', () => {
});
});

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

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

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

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

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

it('Shows an error if libraries V2 is disabled', async () => {
const { axiosMock } = initializeMocks();
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, {
Expand Down
Loading

0 comments on commit 811be22

Please sign in to comment.