Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions src/generic/Loading.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
);

Expand Down
12 changes: 9 additions & 3 deletions src/generic/utils.js
Original file line number Diff line number Diff line change
@@ -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;
6 changes: 6 additions & 0 deletions src/taxonomy/TaxonomyListPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -35,6 +38,9 @@ const TaxonomyListPage = () => {

return (
<>
<Helmet>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

<title>{getPageHeadTitle('', intl.formatMessage(messages.headerTitle))}</title>
</Helmet>
<div className="pt-4.5 pr-4.5 pl-4.5 pb-2 bg-light-100 box-shadow-down-2">
<Container size="xl">
<SubHeader
Expand Down
1 change: 1 addition & 0 deletions src/taxonomy/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const getApiBaseUrl = () => 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisChV FYI - I included a quick fix here for the pagination issue on the taxonomy list page.

if (org !== undefined) {
url.searchParams.append('org', org);
}
Expand Down
55 changes: 50 additions & 5 deletions src/taxonomy/tag-list/TagListTable.jsx
Original file line number Diff line number Diff line change
@@ -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 <LoadingSpinner />;
}
if (subTagsData.isError) {
return <FormattedMessage {...messages.tagListError} />;
}

return (
<ul style={{ listStyleType: 'none' }}>
{subTagsData.data.results.map(tagData => (
<li key={tagData.id} style={{ paddingLeft: `${(tagData.depth - 1) * 30}px` }}>
{tagData.value} <span className="text-light-900">{tagData.childCount > 0 ? `(${tagData.childCount})` : null}</span>
</li>
))}
</ul>
Comment on lines +24 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this date is nested, instead of using padding perhaps it can do the following:

const TagList = ({subTagData}) => (
    <ul style={{ listStyleType: 'none' }}>
      {subTagsData.data.results.map(tagData => (
        <li key={tagData.id}>
          {tagData.value} <span className="text-light-900">{tagData.childCount > 0 ? `(${tagData.childCount})` : null}</span> 
          {tagData.children && <TagList subTagData={tagData} />}
        </li>
      ))}
    </ul>
)

I don't know if the data is structured that way or can be accessed that way. It will avoid the need to use padding to denote hierarchy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that's what we'll do eventually. We're basically going to be re-building this whole table in openedx/modular-learning#130 so I didn't bother doing anything too fancy for now. Right now we're just trying to get the basic read-only functionality out the door. The data is currently just in a single array, though the tags are in "tree order" in that array, which is why we can display them this way and why I went with the simplest option.

);
};

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 ? <DataTable.ExpandRow row={row} /> : null);
OptionalExpandLink.propTypes = DataTable.ExpandRow.propTypes;

const TagListTable = ({ taxonomyId }) => {
const intl = useIntl();
Expand All @@ -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 }) => <SubTagsExpanded taxonomyId={taxonomyId} parentTagValue={row.values.value} />}
columns={[
{
Header: intl.formatMessage(messages.tagListColumnValueHeader),
accessor: 'value',
},
{
id: 'expander',
Header: DataTable.ExpandAll,
Cell: OptionalExpandLink,
},
{
Header: intl.formatMessage(messages.tagListColumnChildCountHeader),
accessor: 'childCount',
},
]}
>
<DataTable.TableControlBar />
Expand All @@ -50,7 +95,7 @@ const TagListTable = ({ taxonomyId }) => {
};

TagListTable.propTypes = {
taxonomyId: Proptypes.string.isRequired,
taxonomyId: Proptypes.number.isRequired,
};

export default TagListTable;
124 changes: 94 additions & 30 deletions src/taxonomy/tag-list/TagListTable.test.jsx
Original file line number Diff line number Diff line change
@@ -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 = () => (
<AppProvider store={store}>
<IntlProvider locale="en" messages={{}}>
<TagListTable taxonomyId="1" />
<QueryClientProvider client={queryClient}>
<TagListTable taxonomyId={1} />
</QueryClientProvider>
</IntlProvider>
</AppProvider>
);

describe('<TagListPage />', 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('<TagListPage />', () => {
beforeAll(async () => {
initializeMockApp({
authenticatedUser: {
userId: 3,
Expand All @@ -32,36 +87,45 @@ describe('<TagListPage />', 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(<RootWrapper />);
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(<RootWrapper />);
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(<RootWrapper />);
await waitFor(() => {
expect(result.getByText('two level tag 1')).toBeInTheDocument();
});
const { getAllByRole } = render(<RootWrapper />);
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(<RootWrapper />);
const expandButton = result.getAllByLabelText('Expand row')[0];
expandButton.click();
await waitFor(() => {
expect(result.getByText('the child tag')).toBeInTheDocument();
});
});
});
19 changes: 19 additions & 0 deletions src/taxonomy/tag-list/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<import('./types.mjs').TagData>}
*/
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);
},
});
8 changes: 8 additions & 0 deletions src/taxonomy/tag-list/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading