From eb1ea744132dd5fa18fc7c2d0804a27f00f74c52 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 20 Nov 2023 12:17:56 -0800 Subject: [PATCH] feat: display all child tags in the "bare bones" taxonomy detail page Also includes: - feat: set on taxonomy list page and taxonomy detail page - fix: display all taxonomies on the list page, even if > 10 - refactor: separate out loading spinner component --- src/generic/Loading.jsx | 37 +++--- src/generic/utils.js | 12 +- src/taxonomy/TaxonomyListPage.jsx | 6 + src/taxonomy/data/api.js | 1 + src/taxonomy/tag-list/TagListTable.jsx | 55 +++++++- src/taxonomy/tag-list/TagListTable.test.jsx | 124 +++++++++++++----- src/taxonomy/tag-list/data/api.js | 19 +++ src/taxonomy/tag-list/messages.js | 8 ++ .../taxonomy-detail/TaxonomyDetailPage.jsx | 12 +- 9 files changed, 214 insertions(+), 60 deletions(-) diff --git a/src/generic/Loading.jsx b/src/generic/Loading.jsx index 7f109a245c..9c696b013c 100644 --- a/src/generic/Loading.jsx +++ b/src/generic/Loading.jsx @@ -2,27 +2,24 @@ import React from 'react'; import { Spinner } from '@edx/paragon'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; +export const LoadingSpinner = () => ( + <Spinner + animation="border" + role="status" + variant="primary" + screenReaderText={( + <FormattedMessage + id="authoring.loading" + defaultMessage="Loading..." + description="Screen-reader message for when a page is loading." + /> + )} + /> +); + const Loading = () => ( - <div - className="d-flex justify-content-center align-items-center flex-column" - style={{ - height: '100vh', - }} - > - <Spinner - animation="border" - role="status" - variant="primary" - screenReaderText={( - <span className="sr-only"> - <FormattedMessage - id="authoring.loading" - defaultMessage="Loading..." - description="Screen-reader message for when a page is loading." - /> - </span> - )} - /> + <div className="d-flex justify-content-center align-items-center flex-column vh-100"> + <LoadingSpinner /> </div> ); diff --git a/src/generic/utils.js b/src/generic/utils.js index 27236e8aa8..c24e5a1646 100644 --- a/src/generic/utils.js +++ b/src/generic/utils.js @@ -1,10 +1,16 @@ import { isEmpty } from 'lodash'; -const getPageHeadTitle = (courseName, pageName) => { - if (isEmpty(courseName)) { +/** + * Generate the string for the page <title> + * @param {string} courseOrSectionName The name of the course, or the section of the MFE that the user is in currently + * @param {string} pageName The name of the current page + * @returns {string} The combined title + */ +const getPageHeadTitle = (courseOrSectionName, pageName) => { + if (isEmpty(courseOrSectionName)) { return `${pageName} | ${process.env.SITE_NAME}`; } - return `${pageName} | ${courseName} | ${process.env.SITE_NAME}`; + return `${pageName} | ${courseOrSectionName} | ${process.env.SITE_NAME}`; }; export default getPageHeadTitle; diff --git a/src/taxonomy/TaxonomyListPage.jsx b/src/taxonomy/TaxonomyListPage.jsx index 79ca9982aa..20287b693f 100644 --- a/src/taxonomy/TaxonomyListPage.jsx +++ b/src/taxonomy/TaxonomyListPage.jsx @@ -6,7 +6,10 @@ import { Spinner, } from '@edx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { Helmet } from 'react-helmet'; + import SubHeader from '../generic/sub-header/SubHeader'; +import getPageHeadTitle from '../generic/utils'; import messages from './messages'; import TaxonomyCard from './taxonomy-card'; import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from './data/apiHooks'; @@ -35,6 +38,9 @@ const TaxonomyListPage = () => { return ( <> + <Helmet> + <title>{getPageHeadTitle('', intl.formatMessage(messages.headerTitle))} +
getConfig().STUDIO_BASE_URL; export const getTaxonomyListApiUrl = (org) => { const url = new URL('api/content_tagging/v1/taxonomies/', getApiBaseUrl()); url.searchParams.append('enabled', 'true'); + url.searchParams.append('page_size', '500'); // For the tagging MVP, we don't paginate the taxonomy list if (org !== undefined) { url.searchParams.append('org', org); } diff --git a/src/taxonomy/tag-list/TagListTable.jsx b/src/taxonomy/tag-list/TagListTable.jsx index 09a6a26d63..2fa3c042a0 100644 --- a/src/taxonomy/tag-list/TagListTable.jsx +++ b/src/taxonomy/tag-list/TagListTable.jsx @@ -1,14 +1,46 @@ // ts-check -import { useIntl } from '@edx/frontend-platform/i18n'; -import { - DataTable, -} from '@edx/paragon'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { DataTable } from '@edx/paragon'; import _ from 'lodash'; import Proptypes from 'prop-types'; import { useState } from 'react'; +import { LoadingSpinner } from '../../generic/Loading'; import messages from './messages'; import { useTagListDataResponse, useTagListDataStatus } from './data/apiHooks'; +import { useSubTags } from './data/api'; + +const SubTagsExpanded = ({ taxonomyId, parentTagValue }) => { + const subTagsData = useSubTags(taxonomyId, parentTagValue); + + if (subTagsData.isLoading) { + return ; + } + if (subTagsData.isError) { + return ; + } + + return ( +
    + {subTagsData.data.results.map(tagData => ( +
  • + {tagData.value} {tagData.childCount > 0 ? `(${tagData.childCount})` : null} +
    • + ))} +
+ ); +}; + +SubTagsExpanded.propTypes = { + taxonomyId: Proptypes.number.isRequired, + parentTagValue: Proptypes.string.isRequired, +}; + +/** + * An "Expand" toggle to show/hide subtags, but one which is hidden if the given tag row has no subtags. + */ +const OptionalExpandLink = ({ row }) => (row.values.childCount > 0 ? : null); +OptionalExpandLink.propTypes = DataTable.ExpandRow.propTypes; const TagListTable = ({ taxonomyId }) => { const intl = useIntl(); @@ -34,11 +66,24 @@ const TagListTable = ({ taxonomyId }) => { itemCount={tagList?.count || 0} pageCount={tagList?.numPages || 0} initialState={options} + isExpandable + // This is a temporary "bare bones" solution for brute-force loading all the child tags. In future we'll match + // the Figma design and do something more sophisticated. + renderRowSubComponent={({ row }) => } columns={[ { Header: intl.formatMessage(messages.tagListColumnValueHeader), accessor: 'value', }, + { + id: 'expander', + Header: DataTable.ExpandAll, + Cell: OptionalExpandLink, + }, + { + Header: intl.formatMessage(messages.tagListColumnChildCountHeader), + accessor: 'childCount', + }, ]} > @@ -50,7 +95,7 @@ const TagListTable = ({ taxonomyId }) => { }; TagListTable.propTypes = { - taxonomyId: Proptypes.string.isRequired, + taxonomyId: Proptypes.number.isRequired, }; export default TagListTable; diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index e9d5015dd4..dd79c107fa 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -1,29 +1,84 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { initializeMockApp } from '@edx/frontend-platform'; import { AppProvider } from '@edx/frontend-platform/react'; -import { render } from '@testing-library/react'; +import { render, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import MockAdapter from 'axios-mock-adapter'; -import { useTagListData } from './data/api'; import initializeStore from '../../store'; import TagListTable from './TagListTable'; let store; - -jest.mock('./data/api', () => ({ - useTagListData: jest.fn(), -})); +let axiosMock; +const queryClient = new QueryClient(); const RootWrapper = () => ( - + + + ); -describe('', async () => { - beforeEach(async () => { +const tagDefaults = { depth: 0, external_id: null, parent_value: null }; +const mockTagsResponse = { + next: null, + previous: null, + count: 3, + num_pages: 1, + current_page: 1, + start: 0, + results: [ + { + ...tagDefaults, + value: 'two level tag 1', + child_count: 1, + _id: 1001, + sub_tags_url: '/request/to/load/subtags/1', + }, + { + ...tagDefaults, + value: 'two level tag 2', + child_count: 1, + _id: 1002, + sub_tags_url: '/request/to/load/subtags/2', + }, + { + ...tagDefaults, + value: 'two level tag 3', + child_count: 1, + _id: 1003, + sub_tags_url: '/request/to/load/subtags/3', + }, + ], +}; +const rootTagsListUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?page=1'; +const subTagsResponse = { + next: null, + previous: null, + count: 1, + num_pages: 1, + current_page: 1, + start: 0, + results: [ + { + ...tagDefaults, + depth: 1, + value: 'the child tag', + child_count: 0, + _id: 1111, + sub_tags_url: null, + }, + ], +}; +const subTagsUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?full_depth_threshold=10000&parent_tag=two+level+tag+1'; + +describe('', () => { + beforeAll(async () => { initializeMockApp({ authenticatedUser: { userId: 3, @@ -32,36 +87,45 @@ describe('', async () => { roles: [], }, }); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + }); + beforeEach(async () => { store = initializeStore(); + axiosMock.reset(); }); it('shows the spinner before the query is complete', async () => { - useTagListData.mockReturnValue({ - isLoading: true, - isFetched: false, - }); - const { getByRole } = render(); - const spinner = getByRole('status'); + // Simulate an actual slow response from the API: + let resolveResponse; + const promise = new Promise(resolve => { resolveResponse = resolve; }); + axiosMock.onGet(rootTagsListUrl).reply(() => promise); + const result = render(); + const spinner = result.getByRole('status'); expect(spinner.textContent).toEqual('loading'); + resolveResponse([200, {}]); + await waitFor(() => { + expect(result.getByText('No results found')).toBeInTheDocument(); + }); }); it('should render page correctly', async () => { - useTagListData.mockReturnValue({ - isSuccess: true, - isFetched: true, - isError: false, - data: { - count: 3, - numPages: 1, - results: [ - { value: 'Tag 1' }, - { value: 'Tag 2' }, - { value: 'Tag 3' }, - ], - }, + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); + const result = render(); + await waitFor(() => { + expect(result.getByText('two level tag 1')).toBeInTheDocument(); }); - const { getAllByRole } = render(); - const rows = getAllByRole('row'); + const rows = result.getAllByRole('row'); expect(rows.length).toBe(3 + 1); // 3 items plus header }); + + it('should render page correctly with subtags', async () => { + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); + const result = render(); + const expandButton = result.getAllByLabelText('Expand row')[0]; + expandButton.click(); + await waitFor(() => { + expect(result.getByText('the child tag')).toBeInTheDocument(); + }); + }); }); diff --git a/src/taxonomy/tag-list/data/api.js b/src/taxonomy/tag-list/data/api.js index 570e29ce48..6561256e9a 100644 --- a/src/taxonomy/tag-list/data/api.js +++ b/src/taxonomy/tag-list/data/api.js @@ -25,3 +25,22 @@ export const useTagListData = (taxonomyId, options) => { }, }); }; + +/** + * Temporary hook to load *all* the subtags of a given tag in a taxonomy. + * Doesn't handle pagination or anything. This is meant to be replaced by + * something more sophisticated later, as we improve the "taxonomy details" page. + * @param {number} taxonomyId + * @param {string} parentTagValue + * @returns {import('@tanstack/react-query').UseQueryResult} + */ +export const useSubTags = (taxonomyId, parentTagValue) => useQuery({ + queryKey: ['subtagsList', taxonomyId, parentTagValue], + queryFn: async () => { + const url = new URL(`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/`, getApiBaseUrl()); + url.searchParams.set('full_depth_threshold', '10000'); // Load as deeply as we can + url.searchParams.set('parent_tag', parentTagValue); + const response = await getAuthenticatedHttpClient().get(url.href); + return camelCaseObject(response.data); + }, +}); diff --git a/src/taxonomy/tag-list/messages.js b/src/taxonomy/tag-list/messages.js index 5832fdb465..82db9caba7 100644 --- a/src/taxonomy/tag-list/messages.js +++ b/src/taxonomy/tag-list/messages.js @@ -9,6 +9,14 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.column.value.header', defaultMessage: 'Value', }, + tagListColumnChildCountHeader: { + id: 'course-authoring.tag-list.column.value.header', + defaultMessage: '# child tags', + }, + tagListError: { + id: 'course-authoring.tag-list.error', + defaultMessage: 'Error: unable to load child tags', + }, }); export default messages; diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx index a22fb0bd57..177bac326d 100644 --- a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx +++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx @@ -6,10 +6,12 @@ import { Container, Layout, } from '@edx/paragon'; +import { Helmet } from 'react-helmet'; import { Link, useParams } from 'react-router-dom'; import ConnectionErrorAlert from '../../generic/ConnectionErrorAlert'; import Loading from '../../generic/Loading'; +import getPageHeadTitle from '../../generic/utils'; import SubHeader from '../../generic/sub-header/SubHeader'; import taxonomyMessages from '../messages'; import TaxonomyDetailMenu from './TaxonomyDetailMenu'; @@ -20,9 +22,12 @@ import { useTaxonomyDetailDataResponse, useTaxonomyDetailDataStatus } from './da const TaxonomyDetailPage = () => { const intl = useIntl(); - const { taxonomyId } = useParams(); - const { isError, isFetched } = useTaxonomyDetailDataStatus(taxonomyId); + const { taxonomyId: taxonomyIdString } = useParams(); + const taxonomyId = Number(taxonomyIdString); + const taxonomy = useTaxonomyDetailDataResponse(taxonomyId); + const { isError, isFetched } = useTaxonomyDetailDataStatus(taxonomyId); + const [isExportModalOpen, setIsExportModalOpen] = useState(false); if (!isFetched) { @@ -74,6 +79,9 @@ const TaxonomyDetailPage = () => { return ( <> + + {getPageHeadTitle(intl.formatMessage(taxonomyMessages.headerTitle), taxonomy.name)} +