From 7a4c9a36b60beda726231b2f91b6196beb8781ce Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Mon, 18 Dec 2023 08:46:22 +0300 Subject: [PATCH] feat: Search Content Tags (#737) This change adds the ability to search content tags in the content tags drawer, in order to filter tags. This change also refactors the way data is loaded from the server, handling pre-loaded data and pagination. --- package-lock.json | 47 +++ package.json | 1 + .../ContentTagsCollapsible.jsx | 59 ++-- .../ContentTagsCollapsible.scss | 1 + .../ContentTagsCollapsible.test.jsx | 306 ++++++++++-------- .../ContentTagsCollapsibleHelper.jsx | 3 +- src/content-tags-drawer/ContentTagsDrawer.jsx | 15 +- .../ContentTagsDrawer.test.jsx | 16 +- .../ContentTagsDropDownSelector.jsx | 181 ++++++----- .../ContentTagsDropDownSelector.scss | 4 + .../ContentTagsDropDownSelector.test.jsx | 246 +++++++++++++- src/content-tags-drawer/ContentTagsTree.jsx | 13 +- src/content-tags-drawer/TagBubble.test.jsx | 6 +- src/content-tags-drawer/data/api.js | 40 ++- src/content-tags-drawer/data/api.test.js | 29 +- src/content-tags-drawer/data/apiHooks.jsx | 98 +++++- .../data/apiHooks.test.jsx | 99 +++++- src/content-tags-drawer/data/types.mjs | 94 ++---- src/content-tags-drawer/messages.js | 4 + src/taxonomy/data/types.mjs | 14 +- src/taxonomy/tag-list/data/api.js | 6 +- src/taxonomy/tag-list/data/types.mjs | 10 +- src/taxonomy/taxonomy-detail/data/api.js | 7 +- .../taxonomy-detail/data/apiHooks.jsx | 2 +- src/taxonomy/taxonomy-detail/data/types.mjs | 19 -- 25 files changed, 910 insertions(+), 410 deletions(-) delete mode 100644 src/taxonomy/taxonomy-detail/data/types.mjs diff --git a/package-lock.json b/package-lock.json index 728a799bc..ed8c8ff89 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,6 +57,7 @@ "@edx/typescript-config": "^1.0.1", "@testing-library/jest-dom": "5.17.0", "@testing-library/react": "12.1.5", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "^13.2.1", "@wojtekmaj/enzyme-adapter-react-17": "0.8.0", "axios-mock-adapter": "1.22.0", @@ -5651,6 +5652,36 @@ "react-dom": "<18.0.0" } }, + "node_modules/@testing-library/react-hooks": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz", + "integrity": "sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g==", + "dev": true, + "dependencies": { + "@babel/runtime": "^7.12.5", + "react-error-boundary": "^3.1.0" + }, + "engines": { + "node": ">=12" + }, + "peerDependencies": { + "@types/react": "^16.9.0 || ^17.0.0", + "react": "^16.9.0 || ^17.0.0", + "react-dom": "^16.9.0 || ^17.0.0", + "react-test-renderer": "^16.9.0 || ^17.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "react-dom": { + "optional": true + }, + "react-test-renderer": { + "optional": true + } + } + }, "node_modules/@testing-library/user-event": { "version": "13.5.0", "dev": true, @@ -23363,6 +23394,22 @@ "react-is": "^16.13.1" } }, + "node_modules/react-error-boundary": { + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-3.1.4.tgz", + "integrity": "sha512-uM9uPzZJTF6wRQORmSrvOIgt4lJ9MC1sNgEOj2XGsDTRE4kmpWxg7ENK9EWNKJRMAOY9z0MuF4yIfl6gp4sotA==", + "dev": true, + "dependencies": { + "@babel/runtime": "^7.12.5" + }, + "engines": { + "node": ">=10", + "npm": ">=6" + }, + "peerDependencies": { + "react": ">=16.13.1" + } + }, "node_modules/react-error-overlay": { "version": "6.0.11", "license": "MIT" diff --git a/package.json b/package.json index 6232884cc..46e7048e0 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "@edx/typescript-config": "^1.0.1", "@testing-library/jest-dom": "5.17.0", "@testing-library/react": "12.1.5", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "^13.2.1", "@wojtekmaj/enzyme-adapter-react-17": "0.8.0", "axios-mock-adapter": "1.22.0", diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 2337af8db..574e1617f 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -1,3 +1,4 @@ +// @ts-check import React from 'react'; import { Badge, @@ -6,10 +7,12 @@ import { Button, ModalPopup, useToggle, + SearchField, } from '@edx/paragon'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; +import { debounce } from 'lodash'; import messages from './messages'; import './ContentTagsCollapsible.scss'; @@ -19,6 +22,9 @@ import ContentTagsTree from './ContentTagsTree'; import useContentTagsCollapsibleHelper from './ContentTagsCollapsibleHelper'; +/** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ +/** @typedef {import("./data/types.mjs").Tag} ContentTagData */ + /** * Collapsible component that holds a Taxonomy along with Tags that belong to it. * This includes both applied tags and tags that are available to select @@ -89,22 +95,11 @@ import useContentTagsCollapsibleHelper from './ContentTagsCollapsibleHelper'; * Here is an example of what the value of the "Virology" tag would be: * * "Science%20and%20Research,Molecular%2C%20Cellular%2C%20and%20Microbiology,Virology" - * @param {string} contentId - Id of the content object - * @param {Object} taxonomyAndTagsData - Object containing Taxonomy meta data along with applied tags - * @param {number} taxonomyAndTagsData.id - id of Taxonomy - * @param {string} taxonomyAndTagsData.name - name of Taxonomy - * @param {string} taxonomyAndTagsData.description - description of Taxonomy - * @param {boolean} taxonomyAndTagsData.enabled - Whether Taxonomy is enabled/disabled - * @param {boolean} taxonomyAndTagsData.allowMultiple - Whether Taxonomy allows multiple tags to be applied - * @param {boolean} taxonomyAndTagsData.allowFreeText - Whether Taxonomy allows free text tags - * @param {boolean} taxonomyAndTagsData.systemDefined - Whether Taxonomy is system defined or authored by user - * @param {boolean} taxonomyAndTagsData.visibleToAuthors - Whether Taxonomy should be visible to object authors - * @param {string[]} taxonomyAndTagsData.orgs - Array of orgs this Taxonomy belongs to - * @param {boolean} taxonomyAndTagsData.allOrgs - Whether Taxonomy belongs to all orgs - * @param {Object[]} taxonomyAndTagsData.contentTags - Array of taxonomy tags that are applied to the content - * @param {string} taxonomyAndTagsData.contentTags.value - Value of applied Tag - * @param {string} taxonomyAndTagsData.contentTags.lineage - Array of Tag's ancestors sorted (ancestor -> tag) - * @param {boolean} editable - Whether the tags can be edited + * + * @param {Object} props - The component props. + * @param {string} props.contentId - Id of the content object + * @param {TaxonomyData & {contentTags: ContentTagData[]}} props.taxonomyAndTagsData - Taxonomy metadata & applied tags + * @param {boolean} props.editable - Whether the tags can be edited */ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) => { const intl = useIntl(); @@ -117,9 +112,30 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) => const [isOpen, open, close] = useToggle(false); const [addTagsButtonRef, setAddTagsButtonRef] = React.useState(null); + const [searchTerm, setSearchTerm] = React.useState(''); + const handleSelectableBoxChange = React.useCallback((e) => { tagChangeHandler(e.target.value, e.target.checked); - }); + }, []); + + const handleSearch = debounce((term) => { + setSearchTerm(term.trim()); + }, 500); // Perform search after 500ms + + const handleSearchChange = React.useCallback((value) => { + if (value === '') { + // No need to debounce when search term cleared + setSearchTerm(''); + } else { + handleSearch(value); + } + }, []); + + const modalPopupOnCloseHandler = React.useCallback((event) => { + close(event); + // Clear search term + setSearchTerm(''); + }, []); return (
@@ -145,7 +161,7 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) => placement="bottom" positionRef={addTagsButtonRef} isOpen={isOpen} - onClose={close} + onClose={modalPopupOnCloseHandler} >
@@ -158,11 +174,18 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) => onChange={handleSelectableBoxChange} value={checkedTags} > + {}} + onChange={handleSearchChange} + className="mb-2" + /> +
diff --git a/src/content-tags-drawer/ContentTagsCollapsible.scss b/src/content-tags-drawer/ContentTagsCollapsible.scss index e55d597af..3123eebbf 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.scss +++ b/src/content-tags-drawer/ContentTagsCollapsible.scss @@ -19,6 +19,7 @@ .taxonomy-tags-selectable-box-set { grid-auto-rows: unset !important; + grid-gap: unset !important; overflow-y: scroll; max-height: 20rem; } diff --git a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx index 096eb2745..49a810837 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx @@ -4,8 +4,8 @@ import { act, render, fireEvent, - waitFor, } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import PropTypes from 'prop-types'; import ContentTagsCollapsible from './ContentTagsCollapsible'; @@ -18,8 +18,12 @@ jest.mock('./data/apiHooks', () => ({ mutate: jest.fn(), })), useTaxonomyTagsData: jest.fn(() => ({ - isSuccess: false, - data: {}, + hasMorePages: false, + tagPages: [{ + isLoading: true, + isError: false, + data: [], + }], })), })); @@ -66,153 +70,195 @@ ContentTagsCollapsibleComponent.propTypes = { }; describe('', () => { - it('should render taxonomy tags data along content tags number badge', async () => { - await act(async () => { - const { container, getByText } = render( - , - ); - expect(getByText('Taxonomy 1')).toBeInTheDocument(); - expect(container.getElementsByClassName('badge').length).toBe(1); - expect(getByText('3')).toBeInTheDocument(); + beforeAll(() => { + jest.useFakeTimers(); // To account for debounce timer + }); + + afterAll(() => { + jest.useRealTimers(); // Restore real timers after the tests + }); + + async function getComponent(updatedData) { + const componentData = (!updatedData ? data : updatedData); + + return render( + , + ); + } + + function setupTaxonomyMock() { + useTaxonomyTagsData.mockReturnValue({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 1', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: null, + }, { + value: 'Tag 2', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12346, + subTagsUrl: null, + }, { + value: 'Tag 3', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12347, + subTagsUrl: null, + }], + }], }); + } + + it('should render taxonomy tags data along content tags number badge', async () => { + const { container, getByText } = await getComponent(); + expect(getByText('Taxonomy 1')).toBeInTheDocument(); + expect(container.getElementsByClassName('badge').length).toBe(1); + expect(getByText('3')).toBeInTheDocument(); }); it('should render new tags as they are checked in the dropdown', async () => { - useTaxonomyTagsData.mockReturnValue({ - isSuccess: true, - data: { - results: [{ - value: 'Tag 1', - subTagsUrl: null, - }, { - value: 'Tag 2', - subTagsUrl: null, - }, { - value: 'Tag 3', - subTagsUrl: null, - }], - }, - }); + setupTaxonomyMock(); + const { container, getByText, getAllByText } = await getComponent(); - await act(async () => { - const { container, getByText, getAllByText } = render( - , - ); + // Expand the Taxonomy to view applied tags and "Add tags" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + fireEvent.click(expandToggle); - // Expand the Taxonomy to view applied tags and "Add tags" button - const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; - await act(async () => { - fireEvent.click(expandToggle); - }); + // Click on "Add tags" button to open dropdown to select new tags + const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); + fireEvent.click(addTagsButton); - // Click on "Add tags" button to open dropdown to select new tags - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - await act(async () => { - fireEvent.click(addTagsButton); - }); + // Wait for the dropdown selector for tags to open, + // Tag 3 should only appear there + expect(getByText('Tag 3')).toBeInTheDocument(); + expect(getAllByText('Tag 3').length === 1); - // Wait for the dropdown selector for tags to open, - // Tag 3 should only appear there - await waitFor(() => { - expect(getByText('Tag 3')).toBeInTheDocument(); - expect(getAllByText('Tag 3').length === 1); - }); + const tag3 = getByText('Tag 3'); - const tag3 = getByText('Tag 3'); - await act(async () => { - fireEvent.click(tag3); - }); + fireEvent.click(tag3); - // After clicking on Tag 3, it should also appear in amongst - // the tag bubbles in the tree - await waitFor(() => { - expect(getAllByText('Tag 3').length === 2); - }); - }); + // After clicking on Tag 3, it should also appear in amongst + // the tag bubbles in the tree + expect(getAllByText('Tag 3').length === 2); }); it('should remove tag when they are unchecked in the dropdown', async () => { - useTaxonomyTagsData.mockReturnValue({ - isSuccess: true, - data: { - results: [{ - value: 'Tag 1', - subTagsUrl: null, - }, { - value: 'Tag 2', - subTagsUrl: null, - }, { - value: 'Tag 3', - subTagsUrl: null, - }], - }, - }); + setupTaxonomyMock(); + const { container, getByText, getAllByText } = await getComponent(); + + // Expand the Taxonomy to view applied tags and "Add tags" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Check that Tag 2 appears in tag bubbles + expect(getByText('Tag 2')).toBeInTheDocument(); + + // Click on "Add tags" button to open dropdown to select new tags + const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); + fireEvent.click(addTagsButton); + + // Wait for the dropdown selector for tags to open, + // Tag 3 should only appear there, (i.e. the dropdown is open, since Tag 3 is not applied) + expect(getByText('Tag 3')).toBeInTheDocument(); + + // Get the Tag 2 checkbox and click on it + const tag2 = getAllByText('Tag 2')[1]; + fireEvent.click(tag2); + + // After clicking on Tag 2, it should be removed from + // the tag bubbles in so only the one in the dropdown appears + expect(getAllByText('Tag 2').length === 1); + }); + + it('should handle search term change', async () => { + const { + container, getByText, getByRole, getByDisplayValue, + } = await getComponent(); + + // Expand the Taxonomy to view applied tags and "Add tags" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + fireEvent.click(expandToggle); + + // Click on "Add tags" button to open dropdown + const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); + fireEvent.click(addTagsButton); + + // Get the search field + const searchField = getByRole('searchbox'); + + const searchTerm = 'memo'; + + // Trigger a change in the search field + userEvent.type(searchField, searchTerm); await act(async () => { - const { container, getByText, getAllByText } = render( - , - ); - - // Expand the Taxonomy to view applied tags and "Add tags" button - const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; - await act(async () => { - fireEvent.click(expandToggle); - }); - - // Check that Tag 2 appears in tag bubbles - await waitFor(() => { - expect(getByText('Tag 2')).toBeInTheDocument(); - }); - - // Click on "Add tags" button to open dropdown to select new tags - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - await act(async () => { - fireEvent.click(addTagsButton); - }); - - // Wait for the dropdown selector for tags to open, - // Tag 3 should only appear there, (i.e. the dropdown is open, since Tag 3 is not applied) - await waitFor(() => { - expect(getByText('Tag 3')).toBeInTheDocument(); - }); - - // Get the Tag 2 checkbox and click on it - const tag2 = getAllByText('Tag 2')[1]; - await act(async () => { - fireEvent.click(tag2); - }); - - // After clicking on Tag 2, it should be removed from - // the tag bubbles in so only the one in the dropdown appears - expect(getAllByText('Tag 2').length === 1); + // Fast-forward time by 500 milliseconds (for the debounce delay) + jest.advanceTimersByTime(500); }); + + // Check that the search term has been set + expect(searchField).toHaveValue(searchTerm); + expect(getByDisplayValue(searchTerm)).toBeInTheDocument(); + + // Clear search + userEvent.clear(searchField); + + // Check that the search term has been cleared + expect(searchField).toHaveValue(''); + }); + + it('should close dropdown selector when clicking away', async () => { + setupTaxonomyMock(); + const { container, getByText, queryByText } = await getComponent(); + + // Expand the Taxonomy to view applied tags and "Add tags" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on "Add tags" button to open dropdown + const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); + fireEvent.click(addTagsButton); + + // Wait for the dropdown selector for tags to open, Tag 3 should appear + // since it is not applied + expect(queryByText('Tag 3')).toBeInTheDocument(); + + // Simulate clicking outside the dropdown remove focus + userEvent.click(document.body); + + // Simulate clicking outside the dropdown again to close it + userEvent.click(document.body); + + // Wait for the dropdown selector for tags to close, Tag 3 is no longer on + // the page + expect(queryByText('Tag 3')).not.toBeInTheDocument(); }); it('should render taxonomy tags data without tags number badge', async () => { const updatedData = { ...data }; + updatedData.taxonomyAndTagsData = { ...updatedData.taxonomyAndTagsData }; updatedData.taxonomyAndTagsData.contentTags = []; - await act(async () => { - const { container, getByText } = render( - , - ); - expect(getByText('Taxonomy 1')).toBeInTheDocument(); - expect(container.getElementsByClassName('invisible').length).toBe(1); - }); + const { container, getByText } = await getComponent(updatedData); + + expect(getByText('Taxonomy 1')).toBeInTheDocument(); + expect(container.getElementsByClassName('invisible').length).toBe(1); }); }); diff --git a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx index 3692e1549..44a06ad50 100644 --- a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx @@ -1,3 +1,4 @@ +// @ts-check import React from 'react'; import { useCheckboxSetValues } from '@edx/paragon'; import { cloneDeep } from 'lodash'; @@ -196,7 +197,7 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { setAddedContentTags(addedTree); setUpdatingTags(true); - }); + }, []); return { tagChangeHandler, tagsTree, contentTagsCount, checkedTags, diff --git a/src/content-tags-drawer/ContentTagsDrawer.jsx b/src/content-tags-drawer/ContentTagsDrawer.jsx index 071c8d407..3d2cd52c0 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.jsx @@ -1,3 +1,4 @@ +// @ts-check import React, { useMemo, useEffect } from 'react'; import { Container, @@ -16,9 +17,12 @@ import { import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from '../taxonomy/data/apiHooks'; import Loading from '../generic/Loading'; +/** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ +/** @typedef {import("./data/types.mjs").Tag} ContentTagData */ + const ContentTagsDrawer = () => { const intl = useIntl(); - const { contentId } = useParams(); + const { contentId } = /** @type {{contentId: string}} */(useParams()); const org = extractOrgFromContentId(contentId); @@ -58,11 +62,10 @@ const ContentTagsDrawer = () => { const taxonomies = useMemo(() => { if (taxonomyListData && contentTaxonomyTagsData) { // Initialize list of content tags in taxonomies to populate - const taxonomiesList = taxonomyListData.results.map((taxonomy) => { - // eslint-disable-next-line no-param-reassign - taxonomy.contentTags = []; - return taxonomy; - }); + const taxonomiesList = taxonomyListData.results.map((taxonomy) => ({ + ...taxonomy, + contentTags: /** @type {ContentTagData[]} */([]), + })); const contentTaxonomies = contentTaxonomyTagsData.taxonomies; diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index c722fb48b..3ffbadd9c 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -140,9 +140,7 @@ describe('', () => { // Find the CloseButton element by its test ID and trigger a click event const closeButton = getByTestId('drawer-close-button'); - await act(async () => { - fireEvent.click(closeButton); - }); + fireEvent.click(closeButton); expect(postMessageSpy).toHaveBeenCalledWith('closeManageTagsDrawer', '*'); @@ -154,10 +152,8 @@ describe('', () => { const { container } = render(); - act(() => { - fireEvent.keyDown(container, { - key: 'Escape', - }); + fireEvent.keyDown(container, { + key: 'Escape', }); expect(postMessageSpy).toHaveBeenCalledWith('closeManageTagsDrawer', '*'); @@ -175,10 +171,8 @@ describe('', () => { selectableBox.setAttribute('data-selectable-box', 'taxonomy-tags'); document.body.appendChild(selectableBox); - act(() => { - fireEvent.keyDown(container, { - key: 'Escape', - }); + fireEvent.keyDown(container, { + key: 'Escape', }); expect(postMessageSpy).not.toHaveBeenCalled(); diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx index bde6d5e96..bd9df6af2 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx @@ -1,4 +1,5 @@ -import React, { useState, useEffect, useCallback } from 'react'; +// @ts-check +import React, { useState, useCallback } from 'react'; import { SelectableBox, Icon, @@ -14,137 +15,144 @@ import './ContentTagsDropDownSelector.scss'; import { useTaxonomyTagsData } from './data/apiHooks'; const ContentTagsDropDownSelector = ({ - taxonomyId, level, subTagsUrl, lineage, tagsTree, + taxonomyId, level, lineage, tagsTree, searchTerm, }) => { const intl = useIntl(); + // This object represents the states of the dropdowns on this level // The keys represent the index of the dropdown with // the value true (open) false (closed) const [dropdownStates, setDropdownStates] = useState({}); + const isOpen = (tagValue) => dropdownStates[tagValue]; - const [tags, setTags] = useState([]); - const [nextPage, setNextPage] = useState(null); + const [numPages, setNumPages] = useState(1); + const parentTagValue = lineage.length ? decodeURIComponent(lineage[lineage.length - 1]) : null; + const { hasMorePages, tagPages } = useTaxonomyTagsData(taxonomyId, parentTagValue, numPages, searchTerm); - // `fetchUrl` is initially `subTagsUrl` to fetch the initial data, - // however if it is null that means it is the root, and the apiHooks - // would automatically handle it. Later this url is set to the next - // page of results (if any) - // - // TODO: In the future we may need to refactor this to keep track - // of the count for how many times the user clicked on "load more" then - // use useQueries to load all the pages based on that. - const [fetchUrl, setFetchUrl] = useState(subTagsUrl); + const [prevSearchTerm, setPrevSearchTerm] = useState(searchTerm); - const isOpen = (i) => dropdownStates[i]; + // Reset the page and tags state when search term changes + // and store search term to compare + if (prevSearchTerm !== searchTerm) { + setPrevSearchTerm(searchTerm); + setNumPages(1); + } - const clickAndEnterHandler = (i) => { + const clickAndEnterHandler = (tagValue) => { // This flips the state of the dropdown at index false (closed) -> true (open) // and vice versa. Initially they are undefined which is falsy. - setDropdownStates({ ...dropdownStates, [i]: !dropdownStates[i] }); + setDropdownStates({ ...dropdownStates, [tagValue]: !dropdownStates[tagValue] }); }; - const { data: taxonomyTagsData, isSuccess: isTaxonomyTagsLoaded } = useTaxonomyTagsData(taxonomyId, fetchUrl); - const isImplicit = (tag) => { // Traverse the tags tree using the lineage let traversal = tagsTree; lineage.forEach(t => { - // We need to decode the tag to traverse the tree since the lineage value is encoded - traversal = traversal[decodeURIComponent(t)]?.children || {}; + traversal = traversal[t]?.children || {}; }); return (traversal[tag.value] && !traversal[tag.value].explicit) || false; }; - useEffect(() => { - if (isTaxonomyTagsLoaded && taxonomyTagsData) { - setTags([...tags, ...taxonomyTagsData.results]); - setNextPage(taxonomyTagsData.next); - } - }, [isTaxonomyTagsLoaded, taxonomyTagsData]); - const loadMoreTags = useCallback(() => { - setFetchUrl(nextPage); - }, [nextPage]); + setNumPages((x) => x + 1); + }, []); return ( - <> - {tags.map((taxonomyTag, i) => ( -
-
- - {taxonomyTag.value} - - { taxonomyTag.subTagsUrl - && ( -
- clickAndEnterHandler(i)} - tabIndex="0" - onKeyPress={(event) => (event.key === 'Enter' ? clickAndEnterHandler(i) : null)} - /> +
+ {tagPages.map((tagPage, pageNum) => ( + // Array index represents the page number + // eslint-disable-next-line react/no-array-index-key + + {tagPage.isLoading ? ( +
+ +
+ ) : null } + {tagPage.isError ? 'Error...' : null /* TODO: show a proper error message */} + + {tagPage.data?.map((tagData) => ( + +
+
+ encodeURIComponent(t)).join(',')} + isIndeterminate={isImplicit(tagData)} + disabled={isImplicit(tagData)} + > + {tagData.value} + + { tagData.childCount > 0 + && ( +
+ clickAndEnterHandler(tagData.value)} + tabIndex="0" + onKeyPress={(event) => (event.key === 'Enter' ? clickAndEnterHandler(tagData.value) : null)} + /> +
+ )}
+ +
+ + { tagData.childCount > 0 && isOpen(tagData.value) && ( + )} -
- { taxonomyTag.subTagsUrl && isOpen(i) && ( - - )} + + ))} -
+ ))} - { nextPage && isTaxonomyTagsLoaded + { hasMorePages ? ( - +
+ +
) : null} - { !isTaxonomyTagsLoaded ? ( -
- -
- ) : null} - +
); }; ContentTagsDropDownSelector.defaultProps = { - subTagsUrl: undefined, lineage: [], + searchTerm: '', }; ContentTagsDropDownSelector.propTypes = { taxonomyId: PropTypes.number.isRequired, level: PropTypes.number.isRequired, - subTagsUrl: PropTypes.string, lineage: PropTypes.arrayOf(PropTypes.string), tagsTree: PropTypes.objectOf( PropTypes.shape({ @@ -152,6 +160,7 @@ ContentTagsDropDownSelector.propTypes = { children: PropTypes.shape({}).isRequired, }).isRequired, ).isRequired, + searchTerm: PropTypes.string, }; export default ContentTagsDropDownSelector; diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.scss b/src/content-tags-drawer/ContentTagsDropDownSelector.scss index a6b72affc..4a3541e10 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.scss +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.scss @@ -2,6 +2,10 @@ cursor: pointer; } +.taxonomy-tags-load-more-button { + flex: 1; +} + .pgn__selectable_box.taxonomy-tags-selectable-box { box-shadow: none; padding: 0; diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx index 5823feaac..ac24a39e9 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx @@ -1,6 +1,11 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { act, render, waitFor } from '@testing-library/react'; +import { + act, + render, + waitFor, + fireEvent, +} from '@testing-library/react'; import PropTypes from 'prop-types'; import ContentTagsDropDownSelector from './ContentTagsDropDownSelector'; @@ -8,8 +13,12 @@ import { useTaxonomyTagsData } from './data/apiHooks'; jest.mock('./data/apiHooks', () => ({ useTaxonomyTagsData: jest.fn(() => ({ - isSuccess: false, - data: {}, + hasMorePages: false, + tagPages: [{ + isLoading: true, + isError: false, + data: [], + }], })), })); @@ -20,28 +29,27 @@ const data = { }; const ContentTagsDropDownSelectorComponent = ({ - taxonomyId, level, subTagsUrl, lineage, tagsTree, + taxonomyId, level, lineage, tagsTree, searchTerm, }) => ( ); ContentTagsDropDownSelectorComponent.defaultProps = { - subTagsUrl: undefined, lineage: [], + searchTerm: '', }; ContentTagsDropDownSelectorComponent.propTypes = { taxonomyId: PropTypes.number.isRequired, level: PropTypes.number.isRequired, - subTagsUrl: PropTypes.string, lineage: PropTypes.arrayOf(PropTypes.string), tagsTree: PropTypes.objectOf( PropTypes.shape({ @@ -49,9 +57,14 @@ ContentTagsDropDownSelectorComponent.propTypes = { children: PropTypes.shape({}).isRequired, }).isRequired, ).isRequired, + searchTerm: PropTypes.string, }; describe('', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('should render taxonomy tags drop down selector loading with spinner', async () => { await act(async () => { const { getByRole } = render( @@ -68,14 +81,22 @@ describe('', () => { it('should render taxonomy tags drop down selector with no sub tags', async () => { useTaxonomyTagsData.mockReturnValue({ - isSuccess: true, - data: { - results: [{ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ value: 'Tag 1', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12345, subTagsUrl: null, }], - }, + }], }); + await act(async () => { const { container, getByText } = render( ', () => { it('should render taxonomy tags drop down selector with sub tags', async () => { useTaxonomyTagsData.mockReturnValue({ - isSuccess: true, - data: { - results: [{ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ value: 'Tag 2', - subTagsUrl: 'https://example.com', + externalId: null, + childCount: 1, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', }], - }, + }], }); + await act(async () => { const { container, getByText } = render( ', () => { }); }); }); + + it('should expand on click taxonomy tags drop down selector with sub tags', async () => { + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 2', + externalId: null, + childCount: 1, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', + }], + }], + }); + + await act(async () => { + const dataWithTagsTree = { + ...data, + tagsTree: { + 'Tag 3': { + explicit: false, + children: {}, + }, + }, + }; + const { container, getByText } = render( + , + ); + await waitFor(() => { + expect(getByText('Tag 2')).toBeInTheDocument(); + expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(1); + }); + + // Mock useTaxonomyTagsData again since it gets called in the recursive call + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 3', + externalId: null, + childCount: 0, + depth: 1, + parentValue: 'Tag 2', + id: 12346, + subTagsUrl: null, + }], + }], + }); + + // Expand the dropdown to see the subtags selectors + const expandToggle = container.querySelector('.taxonomy-tags-arrow-drop-down span'); + fireEvent.click(expandToggle); + + await waitFor(() => { + expect(getByText('Tag 3')).toBeInTheDocument(); + }); + }); + }); + + it('should expand on enter key taxonomy tags drop down selector with sub tags', async () => { + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 2', + externalId: null, + childCount: 1, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=Tag%202', + }], + }], + }); + + await act(async () => { + const dataWithTagsTree = { + ...data, + tagsTree: { + 'Tag 3': { + explicit: false, + children: {}, + }, + }, + }; + const { container, getByText } = render( + , + ); + await waitFor(() => { + expect(getByText('Tag 2')).toBeInTheDocument(); + expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(1); + }); + + // Mock useTaxonomyTagsData again since it gets called in the recursive call + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 3', + externalId: null, + childCount: 0, + depth: 1, + parentValue: 'Tag 2', + id: 12346, + subTagsUrl: null, + }], + }], + }); + + // Expand the dropdown to see the subtags selectors + const expandToggle = container.querySelector('.taxonomy-tags-arrow-drop-down span'); + fireEvent.keyPress(expandToggle, { key: 'Enter', charCode: 13 }); + + await waitFor(() => { + expect(getByText('Tag 3')).toBeInTheDocument(); + }); + }); + }); + + it('should render taxonomy tags drop down selector and change search term', async () => { + useTaxonomyTagsData.mockReturnValueOnce({ + hasMorePages: false, + tagPages: [{ + isLoading: false, + isError: false, + data: [{ + value: 'Tag 1', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: null, + }], + }], + }); + + const initalSearchTerm = 'test 1'; + await act(async () => { + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, initalSearchTerm); + }); + + const updatedSearchTerm = 'test 2'; + rerender(); + + await waitFor(() => { + expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, updatedSearchTerm); + }); + }); + }); }); diff --git a/src/content-tags-drawer/ContentTagsTree.jsx b/src/content-tags-drawer/ContentTagsTree.jsx index 08ae7e145..7066ca869 100644 --- a/src/content-tags-drawer/ContentTagsTree.jsx +++ b/src/content-tags-drawer/ContentTagsTree.jsx @@ -1,3 +1,4 @@ +// @ts-check import React from 'react'; import PropTypes from 'prop-types'; @@ -34,9 +35,13 @@ import TagBubble from './TagBubble'; * } * }; * - * @param {Object} tagsTree - Array of taxonomy tags that are applied to the content - * @param {Func} removeTagHandler - Function that is called when removing tags from tree - * @param {boolean} editable - Whether the tags appear with an 'x' allowing the user to remove them + * @param {Object} props - The component props. + * @param {Object} props.tagsTree - Array of taxonomy tags that are applied to the content. + * @param {( + * tagSelectableBoxValue: string, + * checked: boolean + * ) => void} props.removeTagHandler - Function that is called when removing tags from the tree. + * @param {boolean} props.editable - Whether the tags appear with an 'x' allowing the user to remove them. */ const ContentTagsTree = ({ tagsTree, removeTagHandler, editable }) => { const renderTagsTree = (tag, level, lineage) => Object.keys(tag).map((key) => { @@ -60,7 +65,7 @@ const ContentTagsTree = ({ tagsTree, removeTagHandler, editable }) => { return null; }); - return renderTagsTree(tagsTree, 0, []); + return <>{renderTagsTree(tagsTree, 0, [])}; }; ContentTagsTree.propTypes = { diff --git a/src/content-tags-drawer/TagBubble.test.jsx b/src/content-tags-drawer/TagBubble.test.jsx index 48fe71ecf..e03fe1872 100644 --- a/src/content-tags-drawer/TagBubble.test.jsx +++ b/src/content-tags-drawer/TagBubble.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { act, render, fireEvent } from '@testing-library/react'; +import { render, fireEvent } from '@testing-library/react'; import PropTypes from 'prop-types'; import TagBubble from './TagBubble'; @@ -90,9 +90,7 @@ describe('', () => { ); const xButton = container.getElementsByClassName('pgn__chip__icon-after')[0]; - await act(async () => { - fireEvent.click(xButton); - }); + fireEvent.click(xButton); expect(data.removeTagHandler).toHaveBeenCalled(); }); }); diff --git a/src/content-tags-drawer/data/api.js b/src/content-tags-drawer/data/api.js index a6082b33d..5bd30772b 100644 --- a/src/content-tags-drawer/data/api.js +++ b/src/content-tags-drawer/data/api.js @@ -3,21 +3,43 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; -export const getTaxonomyTagsApiUrl = (taxonomyId) => new URL(`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/`, getApiBaseUrl()).href; + +/** + * Get the URL used to fetch tags data from the "taxonomy tags" REST API + * @param {number} taxonomyId + * @param {{page?: number, searchTerm?: string, parentTag?: string}} options + * @returns {string} the URL + */ +export const getTaxonomyTagsApiUrl = (taxonomyId, options = {}) => { + const url = new URL(`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/`, getApiBaseUrl()); + if (options.parentTag) { + url.searchParams.append('parent_tag', options.parentTag); + } + if (options.page) { + url.searchParams.append('page', String(options.page)); + } + if (options.searchTerm) { + url.searchParams.append('search_term', options.searchTerm); + } + + // Load in the full tree if children at once, if we can: + // Note: do not combine this with page_size (we currently aren't using page_size) + url.searchParams.append('full_depth_threshold', '1000'); + + return url.href; +}; export const getContentTaxonomyTagsApiUrl = (contentId) => new URL(`api/content_tagging/v1/object_tags/${contentId}/`, getApiBaseUrl()).href; export const getContentDataApiUrl = (contentId) => new URL(`/xblock/outline/${contentId}`, getApiBaseUrl()).href; /** * Get all tags that belong to taxonomy. * @param {number} taxonomyId The id of the taxonomy to fetch tags for - * @param {string} fullPathProvided Optional param that contains the full URL to fetch data - * If provided, we use it instead of generating the URL. This is usually for fetching subTags - * @returns {Promise} + * @param {{page?: number, searchTerm?: string, parentTag?: string}} options + * @returns {Promise} */ -export async function getTaxonomyTagsData(taxonomyId, fullPathProvided) { - const { data } = await getAuthenticatedHttpClient().get( - fullPathProvided ? new URL(`${fullPathProvided}`) : getTaxonomyTagsApiUrl(taxonomyId), - ); +export async function getTaxonomyTagsData(taxonomyId, options = {}) { + const url = getTaxonomyTagsApiUrl(taxonomyId, options); + const { data } = await getAuthenticatedHttpClient().get(url); return camelCaseObject(data); } @@ -46,7 +68,7 @@ export async function getContentData(contentId) { * @param {string} contentId The id of the content object (unit/component) * @param {number} taxonomyId The id of the taxonomy the tags belong to * @param {string[]} tags The list of tags (values) to set on content object - * @returns {Promise} + * @returns {Promise} */ export async function updateContentTaxonomyTags(contentId, taxonomyId, tags) { let url = getContentTaxonomyTagsApiUrl(contentId); diff --git a/src/content-tags-drawer/data/api.test.js b/src/content-tags-drawer/data/api.test.js index 0d474ee0c..b007e12c9 100644 --- a/src/content-tags-drawer/data/api.test.js +++ b/src/content-tags-drawer/data/api.test.js @@ -1,3 +1,4 @@ +// @ts-check import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; @@ -47,13 +48,33 @@ describe('content tags drawer api calls', () => { expect(result).toEqual(taxonomyTagsMock); }); - it('should get taxonomy tags data with fullPathProvided', async () => { + it('should get taxonomy tags data with parentTag', async () => { const taxonomyId = 123; - const fullPathProvided = 'http://example.com/'; + const options = { parentTag: 'Sample Tag' }; axiosMock.onGet().reply(200, taxonomyTagsMock); - const result = await getTaxonomyTagsData(taxonomyId, fullPathProvided); + const result = await getTaxonomyTagsData(taxonomyId, options); - expect(axiosMock.history.get[0].url).toEqual(new URL(`${fullPathProvided}`)); + expect(axiosMock.history.get[0].url).toContain('parent_tag=Sample+Tag'); + expect(result).toEqual(taxonomyTagsMock); + }); + + it('should get taxonomy tags data with page', async () => { + const taxonomyId = 123; + const options = { page: 2 }; + axiosMock.onGet().reply(200, taxonomyTagsMock); + const result = await getTaxonomyTagsData(taxonomyId, options); + + expect(axiosMock.history.get[0].url).toContain('page=2'); + expect(result).toEqual(taxonomyTagsMock); + }); + + it('should get taxonomy tags data with searchTerm', async () => { + const taxonomyId = 123; + const options = { searchTerm: 'memo' }; + axiosMock.onGet().reply(200, taxonomyTagsMock); + const result = await getTaxonomyTagsData(taxonomyId, options); + + expect(axiosMock.history.get[0].url).toContain('search_term=memo'); expect(result).toEqual(taxonomyTagsMock); }); diff --git a/src/content-tags-drawer/data/apiHooks.jsx b/src/content-tags-drawer/data/apiHooks.jsx index 97ffde2fe..44151727a 100644 --- a/src/content-tags-drawer/data/apiHooks.jsx +++ b/src/content-tags-drawer/data/apiHooks.jsx @@ -1,5 +1,11 @@ // @ts-check -import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; +import { useMemo } from 'react'; +import { + useQuery, + useQueries, + useMutation, + useQueryClient, +} from '@tanstack/react-query'; import { getTaxonomyTagsData, getContentTaxonomyTagsData, @@ -7,22 +13,94 @@ import { updateContentTaxonomyTags, } from './api'; +/** @typedef {import("../../taxonomy/tag-list/data/types.mjs").TagListData} TagListData */ +/** @typedef {import("../../taxonomy/tag-list/data/types.mjs").TagData} TagData */ + /** * Builds the query to get the taxonomy tags * @param {number} taxonomyId The id of the taxonomy to fetch tags for - * @param {string} fullPathProvided Optional param that contains the full URL to fetch data - * If provided, we use it instead of generating the URL. This is usually for fetching subTags + * @param {string|null} parentTag The tag whose children we're loading, if any + * @param {string} searchTerm The term passed in to perform search on tags + * @param {number} numPages How many pages of tags to load at this level + * @returns {{ + * hasMorePages: boolean, + * tagPages: { + * isLoading: boolean, + * isError: boolean, + * data: TagListData[], + * }[], + * }} */ -export const useTaxonomyTagsData = (taxonomyId, fullPathProvided) => ( - useQuery({ - queryKey: [`taxonomyTags${ fullPathProvided || taxonomyId }`], - queryFn: () => getTaxonomyTagsData(taxonomyId, fullPathProvided), - }) -); +export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1, searchTerm = '') => { + const queryClient = useQueryClient(); + + const queryFn = async ({ queryKey }) => { + const page = queryKey[3]; + return getTaxonomyTagsData(taxonomyId, { parentTag: parentTag || '', searchTerm, page }); + }; + + /** @type {{queryKey: any[], queryFn: typeof queryFn, staleTime: number}[]} */ + const queries = []; + for (let page = 1; page <= numPages; page++) { + queries.push( + { queryKey: ['taxonomyTags', taxonomyId, parentTag, page, searchTerm], queryFn, staleTime: Infinity }, + ); + } + + const dataPages = useQueries({ queries }); + + const totalPages = dataPages[0]?.data?.numPages || 1; + const hasMorePages = numPages < totalPages; + + const tagPages = useMemo(() => { + /** @type { { isLoading: boolean, isError: boolean, data: TagListData[] }[] } */ + const newTags = []; + + // Pre-load desendants if possible + const preLoadedData = new Map(); + + dataPages.forEach(result => { + /** @type {TagListData[]} */ + const simplifiedTagsList = []; + + result.data?.results?.forEach((tag) => { + if (tag.parentValue === parentTag) { + simplifiedTagsList.push(tag); + } else if (!preLoadedData.has(tag.parentValue)) { + preLoadedData.set(tag.parentValue, [tag]); + } else { + preLoadedData.get(tag.parentValue).push(tag); + } + }); + + newTags.push({ ...result, data: simplifiedTagsList }); + }); + + // Store the pre-loaded descendants into the query cache: + preLoadedData.forEach((tags, parentValue) => { + const queryKey = ['taxonomyTags', taxonomyId, parentValue, 1, searchTerm]; + /** @type {TagData} */ + const cachedData = { + next: '', + previous: '', + count: tags.length, + numPages: 1, + currentPage: 1, + start: 0, + results: tags, + }; + queryClient.setQueryData(queryKey, cachedData); + }); + + return newTags; + }, [dataPages]); + + return { hasMorePages, tagPages }; +}; /** * Builds the query to get the taxonomy tags applied to the content object - * @param {string} contentId The id of the content object to fetch the applied tags for + * @param {string} contentId The ID of the content object to fetch the applied tags for (e.g. an XBlock usage key) */ export const useContentTaxonomyTagsData = (contentId) => ( useQuery({ diff --git a/src/content-tags-drawer/data/apiHooks.test.jsx b/src/content-tags-drawer/data/apiHooks.test.jsx index 3abf33a22..8000a424a 100644 --- a/src/content-tags-drawer/data/apiHooks.test.jsx +++ b/src/content-tags-drawer/data/apiHooks.test.jsx @@ -1,5 +1,6 @@ -import { useQuery, useMutation } from '@tanstack/react-query'; +import { useQuery, useMutation, useQueries } from '@tanstack/react-query'; import { act } from '@testing-library/react'; +import { renderHook } from '@testing-library/react-hooks'; import { useTaxonomyTagsData, useContentTaxonomyTagsData, @@ -12,7 +13,10 @@ import { updateContentTaxonomyTags } from './api'; jest.mock('@tanstack/react-query', () => ({ useQuery: jest.fn(), useMutation: jest.fn(), - useQueryClient: jest.fn(), + useQueryClient: jest.fn(() => ({ + setQueryData: jest.fn(), + })), + useQueries: jest.fn(), })); jest.mock('./api', () => ({ @@ -20,20 +24,91 @@ jest.mock('./api', () => ({ })); describe('useTaxonomyTagsData', () => { - it('should return success response', () => { - useQuery.mockReturnValueOnce({ isSuccess: true, data: 'data' }); + it('should call useQueries with the correct arguments', () => { const taxonomyId = 123; - const result = useTaxonomyTagsData(taxonomyId); + const mockData = { + results: [ + { + value: 'tag 1', + externalId: null, + childCount: 16, + depth: 0, + parentValue: null, + id: 635951, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=tag%201', + }, + { + value: 'tag 2', + externalId: null, + childCount: 1, + depth: 0, + parentValue: null, + id: 636992, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=tag%202', + }, + { + value: 'tag 3', + externalId: null, + childCount: 0, + depth: 1, + parentValue: 'tag 2', + id: 636993, + subTagsUrl: null, + }, + { + value: 'tag 4', + externalId: null, + childCount: 0, + depth: 1, + parentValue: 'tag 2', + id: 636994, + subTagsUrl: null, + }, + ], + }; - expect(result).toEqual({ isSuccess: true, data: 'data' }); - }); + useQueries.mockReturnValue([ + { data: mockData, isLoading: false, isError: false }, + ]); - it('should return failure response', () => { - useQuery.mockReturnValueOnce({ isSuccess: false }); - const taxonomyId = 123; - const result = useTaxonomyTagsData(taxonomyId); + const { result } = renderHook(() => useTaxonomyTagsData(taxonomyId)); - expect(result).toEqual({ isSuccess: false }); + // Assert that useQueries was called with the correct arguments + expect(useQueries).toHaveBeenCalledWith({ + queries: [ + { queryKey: ['taxonomyTags', taxonomyId, null, 1, ''], queryFn: expect.any(Function), staleTime: Infinity }, + ], + }); + + expect(result.current.hasMorePages).toEqual(false); + // Only includes the first 2 tags because the other 2 would be + // in the nested dropdown + expect(result.current.tagPages).toEqual([ + { + isLoading: false, + isError: false, + data: [ + { + value: 'tag 1', + externalId: null, + childCount: 16, + depth: 0, + parentValue: null, + id: 635951, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=tag%201', + }, + { + value: 'tag 2', + externalId: null, + childCount: 1, + depth: 0, + parentValue: null, + id: 636992, + subTagsUrl: 'http://localhost:18010/api/content_tagging/v1/taxonomies/4/tags/?parent_tag=tag%202', + }, + ], + }, + ]); }); }); diff --git a/src/content-tags-drawer/data/types.mjs b/src/content-tags-drawer/data/types.mjs index c16cb45ea..2145f41b9 100644 --- a/src/content-tags-drawer/data/types.mjs +++ b/src/content-tags-drawer/data/types.mjs @@ -1,21 +1,21 @@ // @ts-check /** - * @typedef {Object} Tag - * @property {string} value - * @property {string[]} lineage + * @typedef {Object} Tag A tag that has been applied to some content. + * @property {string} value The value of the tag, also its ID. e.g. "Biology" + * @property {string[]} lineage The values of the tag and its parent(s) in the hierarchy */ /** - * @typedef {Object} ContentTaxonomyTagData + * @typedef {Object} ContentTaxonomyTagData A list of the tags from one taxonomy that are applied to a content object. * @property {string} name - * @property {number} taxonomy_id + * @property {number} taxonomyId * @property {boolean} editable * @property {Tag[]} tags */ /** - * @typedef {Object} ContentTaxonomyTagsData + * @typedef {Object} ContentTaxonomyTagsData A list of all the tags applied to some content object, grouped by taxonomy. * @property {ContentTaxonomyTagData[]} taxonomies */ @@ -30,73 +30,29 @@ /** * @typedef {Object} ContentData * @property {string} id - * @property {string} display_name + * @property {string} displayName * @property {string} category - * @property {boolean} has_children - * @property {string} edited_on + * @property {boolean} hasChildren + * @property {string} editedOn * @property {boolean} published - * @property {string} published_on - * @property {string} studio_url - * @property {boolean} released_to_students - * @property {string} release_date - * @property {string} visibility_state - * @property {boolean} has_explicit_staff_lock + * @property {string} publishedOn + * @property {string} studioUrl + * @property {boolean} releasedToStudents + * @property {string|null} releaseDate + * @property {string} visibilityState + * @property {boolean} hasExplicitStaffLock * @property {string} start * @property {boolean} graded - * @property {string} due_date + * @property {string} dueDate * @property {string} due - * @property {string} relative_weeks_due - * @property {string} format - * @property {boolean} has_changes + * @property {string|null} relativeWeeksDue + * @property {string|null} format + * @property {boolean} hasChanges * @property {ContentActions} actions - * @property {string} explanatory_message - * @property {string} show_correctness - * @property {boolean} discussion_enabled - * @property {boolean} ancestor_has_staff_lock - * @property {boolean} staff_only_message - * @property {boolean} enable_copy_paste_units - * @property {boolean} has_partition_group_components + * @property {string} explanatoryMessage + * @property {string} showCorrectness + * @property {boolean} discussionEnabled + * @property {boolean} ancestorHasStaffLock + * @property {boolean} staffOnlyMessage + * @property {boolean} hasPartitionGroupComponents */ - -/** - * @typedef {Object} TaxonomyTagData - * @property {string} id - * @property {string} display_name - * @property {string} category - * @property {boolean} has_children - * @property {string} edited_on - * @property {boolean} published - * @property {string} published_on - * @property {string} studio_url - * @property {boolean} released_to_students - * @property {string} release_date - * @property {string} visibility_state - * @property {boolean} has_explicit_staff_lock - * @property {string} start - * @property {boolean} graded - * @property {string} due_date - * @property {string} due - * @property {string} relative_weeks_due - * @property {string} format - * @property {boolean} has_changes - * @property {ContentActions} actions - * @property {string} explanatory_message - * @property {string} show_correctness - * @property {boolean} discussion_enabled - * @property {boolean} ancestor_has_staff_lock - * @property {boolean} staff_only_message - * @property {boolean} enable_copy_paste_units - * @property {boolean} has_partition_group_components - */ - -/** - * @typedef {Object} TaxonomyTagsData - * @property {string} next - * @property {string} previous - * @property {number} count - * @property {number} num_pages - * @property {number} current_page - * @property {number} start - * @property {TaxonomyTagData[]} results - */ - diff --git a/src/content-tags-drawer/messages.js b/src/content-tags-drawer/messages.js index 2fe9deb99..62d9e1549 100644 --- a/src/content-tags-drawer/messages.js +++ b/src/content-tags-drawer/messages.js @@ -21,6 +21,10 @@ const messages = defineMessages({ id: 'course-authoring.content-tags-drawer.tags-dropdown-selector.load-more-tags.button', defaultMessage: 'Load more', }, + taxonomyTagsCheckboxAriaLabel: { + id: 'course-authoring.content-tags-drawer.tags-dropdown-selector.selectable-box.aria.label', + defaultMessage: '{tag} checkbox', + }, taxonomyTagsAriaLabel: { id: 'course-authoring.content-tags-drawer.content-tags-collapsible.selectable-box.selection.aria.label', defaultMessage: 'taxonomy tags selection', diff --git a/src/taxonomy/data/types.mjs b/src/taxonomy/data/types.mjs index e2c1026fd..140829c64 100644 --- a/src/taxonomy/data/types.mjs +++ b/src/taxonomy/data/types.mjs @@ -5,10 +5,11 @@ * @property {number} id * @property {string} name * @property {boolean} enabled - * @property {boolean} allow_multiple - * @property {boolean} allow_free_text - * @property {boolean} system_defined - * @property {boolean} visible_to_authors + * @property {boolean} allowMultiple + * @property {boolean} allowFreeText + * @property {boolean} systemDefined + * @property {boolean} visibleToAuthors + * @property {string[]} orgs */ /** @@ -16,10 +17,9 @@ * @property {string} next * @property {string} previous * @property {number} count - * @property {number} num_pages - * @property {number} current_page + * @property {number} numPages + * @property {number} currentPage * @property {number} start * @property {function} refetch * @property {TaxonomyData[]} results */ - diff --git a/src/taxonomy/tag-list/data/api.js b/src/taxonomy/tag-list/data/api.js index 6561256e9..a039223ee 100644 --- a/src/taxonomy/tag-list/data/api.js +++ b/src/taxonomy/tag-list/data/api.js @@ -1,4 +1,9 @@ // @ts-check + +// TODO: this file needs to be merged into src/taxonomy/data/api.js +// We are creating a mess with so many different /data/[api|types].js files in subfolders. +// There is only one tagging/taxonomy API, and it should be implemented via a single types.mjs and api.js file. + import { useQuery } from '@tanstack/react-query'; import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; @@ -9,7 +14,6 @@ const getTagListApiUrl = (taxonomyId, page) => new URL( getApiBaseUrl(), ).href; -// ToDo: fix types /** * @param {number} taxonomyId * @param {import('./types.mjs').QueryOptions} options diff --git a/src/taxonomy/tag-list/data/types.mjs b/src/taxonomy/tag-list/data/types.mjs index 63f6550c2..bd22d27a5 100644 --- a/src/taxonomy/tag-list/data/types.mjs +++ b/src/taxonomy/tag-list/data/types.mjs @@ -1,10 +1,15 @@ // @ts-check +// TODO: this file needs to be merged into src/taxonomy/data/types.mjs +// We are creating a mess with so many different /data/[api|types].js files in subfolders. +// There is only one tagging/taxonomy API, and it should be implemented via a single types.mjs and api.js file. + /** * @typedef {Object} QueryOptions * @property {number} pageIndex */ +// FIXME: this should be renamed to TagData /** * @typedef {Object} TagListData * @property {number} childCount @@ -13,9 +18,12 @@ * @property {number} id * @property {string | null} parentValue * @property {string | null} subTagsUrl - * @property {string} value + * @property {string} value Unique ID for this tag, also its display text + * @property {number?} usageCount + * @property {string?} _id Database ID. Don't rely on this, as it is not present for free-text tags. */ +// FIXME: this should be renamed to TagListData /** * @typedef {Object} TagData * @property {number} count diff --git a/src/taxonomy/taxonomy-detail/data/api.js b/src/taxonomy/taxonomy-detail/data/api.js index 81b7929ec..3cc912605 100644 --- a/src/taxonomy/taxonomy-detail/data/api.js +++ b/src/taxonomy/taxonomy-detail/data/api.js @@ -1,4 +1,9 @@ // @ts-check + +// TODO: this file needs to be merged into src/taxonomy/data/api.js +// We are creating a mess with so many different /data/[api|types].js files in subfolders. +// There is only one tagging/taxonomy API, and it should be implemented via a single types.mjs and api.js file. + import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { useQuery } from '@tanstack/react-query'; @@ -11,7 +16,7 @@ const getTaxonomyDetailApiUrl = (taxonomyId) => new URL( /** * @param {number} taxonomyId - * @returns {import('@tanstack/react-query').UseQueryResult} + * @returns {import('@tanstack/react-query').UseQueryResult} */ // eslint-disable-next-line import/prefer-default-export export const useTaxonomyDetailData = (taxonomyId) => ( useQuery({ diff --git a/src/taxonomy/taxonomy-detail/data/apiHooks.jsx b/src/taxonomy/taxonomy-detail/data/apiHooks.jsx index 31f3361a9..7a06b3361 100644 --- a/src/taxonomy/taxonomy-detail/data/apiHooks.jsx +++ b/src/taxonomy/taxonomy-detail/data/apiHooks.jsx @@ -24,7 +24,7 @@ export const useTaxonomyDetailDataStatus = (taxonomyId) => { /** * @param {number} taxonomyId - * @returns {import("./types.mjs").TaxonomyData | undefined} + * @returns {import("../../data/types.mjs").TaxonomyData | undefined} */ export const useTaxonomyDetailDataResponse = (taxonomyId) => { const { isSuccess, data } = useTaxonomyDetailData(taxonomyId); diff --git a/src/taxonomy/taxonomy-detail/data/types.mjs b/src/taxonomy/taxonomy-detail/data/types.mjs deleted file mode 100644 index 90b2c07ac..000000000 --- a/src/taxonomy/taxonomy-detail/data/types.mjs +++ /dev/null @@ -1,19 +0,0 @@ -// @ts-check - -/** - * @typedef {Object} TaxonomyData - * @property {number} id - * @property {string} name - * @property {boolean} enabled - * @property {boolean} allowMultiple - * @property {boolean} allowFreeText - * @property {boolean} systemDefined - * @property {boolean} visibleToAuthors - * @property {string[]} orgs - */ - -/** - * @typedef {Object} UseQueryResult - * @property {Object} data - * @property {string} status - */