Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UU-58] Implement tagging & taxonomy feature in outline #855

Merged
merged 10 commits into from
Mar 8, 2024
43 changes: 35 additions & 8 deletions src/content-tags-drawer/ContentTagsDrawer.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @ts-check
import React, { useMemo, useEffect } from 'react';
import PropTypes from 'prop-types';
import {
Container,
CloseButton,
Expand All @@ -20,9 +21,22 @@ import Loading from '../generic/Loading';
/** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */
/** @typedef {import("./data/types.mjs").Tag} ContentTagData */

const ContentTagsDrawer = () => {
/**
* Drawer with the functionality to show and manage tags in a certain content.
* It is used both in interfaces of this MFE and in edx-platform interfaces such as iframe.
* - If you want to use it as an iframe, the component obtains the `contentId` from the url parameters.
* Functions to close the drawer are handled internally.
* - If you want to use it as react component, you need to pass the content id and the close functions
* through the component parameters.
*/
const ContentTagsDrawer = ({ id, onClose }) => {
const intl = useIntl();
const { contentId } = /** @type {{contentId: string}} */(useParams());
const params = useParams();
let contentId = id;

if (contentId === undefined) {
contentId = params.contentId;
}

const org = extractOrgFromContentId(contentId);

Expand All @@ -39,17 +53,20 @@ const ContentTagsDrawer = () => {
} = useContentTaxonomyTagsData(contentId);
const { taxonomyListData, isTaxonomyListLoaded } = useTaxonomyListData();

const closeContentTagsDrawer = () => {
// "*" allows communication with any origin
window.parent.postMessage('closeManageTagsDrawer', '*');
};
let onCloseDrawer = onClose;
if (onCloseDrawer === undefined) {
onCloseDrawer = () => {
// "*" allows communication with any origin
window.parent.postMessage('closeManageTagsDrawer', '*');
};
}

useEffect(() => {
const handleEsc = (event) => {
/* Close drawer when ESC-key is pressed and selectable dropdown box not open */
const selectableBoxOpen = document.querySelector('[data-selectable-box="taxonomy-tags"]');
if (event.key === 'Escape' && !selectableBoxOpen) {
closeContentTagsDrawer();
onCloseDrawer();
}
};
document.addEventListener('keydown', handleEsc);
Expand Down Expand Up @@ -86,7 +103,7 @@ const ContentTagsDrawer = () => {

<div className="mt-1">
<Container size="xl">
<CloseButton onClick={() => closeContentTagsDrawer()} data-testid="drawer-close-button" />
<CloseButton onClick={() => onCloseDrawer()} data-testid="drawer-close-button" />
<span>{intl.formatMessage(messages.headerSubtitle)}</span>
{ isContentDataLoaded
? <h3>{ contentData.displayName }</h3>
Expand Down Expand Up @@ -116,4 +133,14 @@ const ContentTagsDrawer = () => {
);
};

ContentTagsDrawer.propTypes = {
id: PropTypes.string,
onClose: PropTypes.func,
};

ContentTagsDrawer.defaultProps = {
id: undefined,
onClose: undefined,
};

export default ContentTagsDrawer;
40 changes: 33 additions & 7 deletions src/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { act, render, fireEvent } from '@testing-library/react';
import {
act, render, fireEvent, screen,
} from '@testing-library/react';

import ContentTagsDrawer from './ContentTagsDrawer';
import {
Expand All @@ -9,10 +11,13 @@ import {
} from './data/apiHooks';
import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from '../taxonomy/data/apiHooks';

const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@7f47fe2dbcaf47c5a071671c741fe1ab';
const mockOnClose = jest.fn();

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useParams: () => ({
contentId: 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@7f47fe2dbcaf47c5a071671c741fe1ab',
contentId,
}),
}));

Expand All @@ -35,9 +40,9 @@ jest.mock('../taxonomy/data/apiHooks', () => ({
useIsTaxonomyListDataLoaded: jest.fn(),
}));

const RootWrapper = () => (
const RootWrapper = (params) => (
<IntlProvider locale="en" messages={{}}>
<ContentTagsDrawer />
<ContentTagsDrawer {...params} />
</IntlProvider>
);

Expand Down Expand Up @@ -77,6 +82,17 @@ describe('<ContentTagsDrawer />', () => {
});
});

it('shows content using params', async () => {
useContentData.mockReturnValue({
isSuccess: true,
data: {
displayName: 'Unit 1',
},
});
render(<RootWrapper id={contentId} />);
expect(screen.getByText('Unit 1')).toBeInTheDocument();
});

it('shows the taxonomies data including tag numbers after the query is complete', async () => {
useIsTaxonomyListDataLoaded.mockReturnValue(true);
useContentTaxonomyTagsData.mockReturnValue({
Expand Down Expand Up @@ -138,7 +154,7 @@ describe('<ContentTagsDrawer />', () => {
});
});

it('should call closeContentTagsDrawer when CloseButton is clicked', async () => {
it('should call closeManageTagsDrawer when CloseButton is clicked', async () => {
const postMessageSpy = jest.spyOn(window.parent, 'postMessage');

const { getByTestId } = render(<RootWrapper />);
Expand All @@ -152,7 +168,17 @@ describe('<ContentTagsDrawer />', () => {
postMessageSpy.mockRestore();
});

it('should call closeContentTagsDrawer when Escape key is pressed and no selectable box is active', () => {
it('should call onClose param when CloseButton is clicked', async () => {
render(<RootWrapper onClose={mockOnClose} />);

// Find the CloseButton element by its test ID and trigger a click event
const closeButton = screen.getByTestId('drawer-close-button');
fireEvent.click(closeButton);

expect(mockOnClose).toHaveBeenCalled();
});

it('should call closeManageTagsDrawer when Escape key is pressed and no selectable box is active', () => {
const postMessageSpy = jest.spyOn(window.parent, 'postMessage');

const { container } = render(<RootWrapper />);
Expand All @@ -166,7 +192,7 @@ describe('<ContentTagsDrawer />', () => {
postMessageSpy.mockRestore();
});

it('should not call closeContentTagsDrawer when Escape key is pressed and a selectable box is active', () => {
it('should not call closeManageTagsDrawer when Escape key is pressed and a selectable box is active', () => {
const postMessageSpy = jest.spyOn(window.parent, 'postMessage');

const { container } = render(<RootWrapper />);
Expand Down
4 changes: 3 additions & 1 deletion src/content-tags-drawer/data/apiHooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,10 @@ export const useContentTaxonomyTagsUpdater = (contentId, taxonomyId) => {
* >}
*/
mutationFn: ({ tags }) => updateContentTaxonomyTags(contentId, taxonomyId, tags),
onSettled: () => {
onSettled: /* istanbul ignore next */ () => {
queryClient.invalidateQueries({ queryKey: ['contentTaxonomyTags', contentId] });
/// Invalidate query with pattern on course outline
queryClient.invalidateQueries({ queryKey: ['unitTagsCount'] });
},
});
};
25 changes: 24 additions & 1 deletion src/course-outline/CourseOutline.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect } from 'react';
import { useState, useEffect, useMemo } from 'react';
import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Expand Down Expand Up @@ -41,6 +41,7 @@ import DeleteModal from './delete-modal/DeleteModal';
import PageAlerts from './page-alerts/PageAlerts';
import { useCourseOutline } from './hooks';
import messages from './messages';
import useUnitTagsCount from './data/apiHooks';

const CourseOutline = ({ courseId }) => {
const intl = useIntl();
Expand Down Expand Up @@ -157,6 +158,27 @@ const CourseOutline = ({ courseId }) => {
});
};

const unitsIdPattern = useMemo(() => {
let pattern = '';
sections.forEach((section) => {
section.childInfo.children.forEach((subsection) => {
subsection.childInfo.children.forEach((unit) => {
if (pattern !== '') {
pattern += `,${unit.id}`;
} else {
pattern += unit.id;
}
});
});
});
return pattern;
}, [sections]);

const {
data: unitsTagCounts,
isSuccess: isUnitsTagCountsLoaded,
} = useUnitTagsCount(unitsIdPattern);

/**
* Check if item can be moved by given step.
* Inner function returns false if the new index after moving by given step
Expand Down Expand Up @@ -400,6 +422,7 @@ const CourseOutline = ({ courseId }) => {
)}
onCopyToClipboardClick={handleCopyToClipboardClick}
discussionsSettings={discussionsSettings}
tagsCount={isUnitsTagCountsLoaded ? unitsTagCounts[unit.id] : 0}
/>
))}
</DraggableList>
Expand Down
5 changes: 5 additions & 0 deletions src/course-outline/CourseOutline.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ jest.mock('@edx/frontend-platform/i18n', () => ({
}),
}));

jest.mock('./data/apiHooks', () => () => ({
data: {},
isSuccess: true,
}));

const RootWrapper = () => (
<AppProvider store={store}>
<IntlProvider locale="en">
Expand Down
8 changes: 8 additions & 0 deletions src/course-outline/__mocks__/contentTagsCount.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb01': 10,
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb02': 11,
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb03': 12,
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb04': 13,
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb05': 14,
'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb06': 15,
};
1 change: 1 addition & 0 deletions src/course-outline/__mocks__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export { default as courseBestPracticesMock } from './courseBestPractices';
export { default as courseLaunchMock } from './courseLaunch';
export { default as courseSectionMock } from './courseSection';
export { default as courseSubsectionMock } from './courseSubsection';
export { default as contentTagsCountMock } from './contentTagsCount';
17 changes: 17 additions & 0 deletions src/course-outline/card-header/CardHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { ITEM_BADGE_STATUS } from '../constants';
import { scrollToElement } from '../utils';
import CardStatus from './CardStatus';
import messages from './messages';
import TagCount from '../../generic/tag-count';

const CardHeader = ({
title,
Expand All @@ -27,6 +28,7 @@ const CardHeader = ({
hasChanges,
onClickPublish,
onClickConfigure,
onClickManageTags,
onClickMenuButton,
onClickEdit,
isFormOpen,
Expand All @@ -48,6 +50,7 @@ const CardHeader = ({
discussionEnabled,
discussionsSettings,
parentInfo,
tagsCount,
}) => {
const intl = useIntl();
const [searchParams] = useSearchParams();
Expand Down Expand Up @@ -127,6 +130,7 @@ const CardHeader = ({
{(isVertical || isSequential) && (
<CardStatus status={status} showDiscussionsEnabledBadge={showDiscussionsEnabledBadge} />
)}
{ tagsCount !== undefined && tagsCount !== 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ tagsCount !== undefined && tagsCount !== 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }
{ tagsCount > 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }

<Dropdown data-testid={`${namePrefix}-card-header__menu`} onClick={onClickMenuButton}>
<Dropdown.Toggle
className="item-card-header__menu"
Expand Down Expand Up @@ -162,6 +166,15 @@ const CardHeader = ({
>
{intl.formatMessage(messages.menuConfigure)}
</Dropdown.Item>
{onClickManageTags && (
<Dropdown.Item
data-testid={`${namePrefix}-card-header__menu-manage-tags-button`}
onClick={onClickManageTags}
>
{intl.formatMessage(messages.menuManageTags)}
</Dropdown.Item>
)}
Comment on lines +169 to +176
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the dropdown item and the tag count to both have onClick attributes? Should the tag count be read only?

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, the tag count will be read only in other pages. E.g #852

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so you will update the tag count in the header to be read only and the dropdown item will have to onclick attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the course outline it is necessary that the Manage tags menu (dropdown item) and the tag count component can open the Manage tags drawer, for that reason they both have the onclick attribute (Ref)


{isVertical && enableCopyPasteUnits && (
<Dropdown.Item onClick={onClickCopy}>
{intl.formatMessage(messages.menuCopy)}
Expand Down Expand Up @@ -218,6 +231,8 @@ CardHeader.defaultProps = {
discussionEnabled: false,
discussionsSettings: {},
parentInfo: {},
onClickManageTags: null,
tagsCount: undefined,
};

CardHeader.propTypes = {
Expand All @@ -227,6 +242,7 @@ CardHeader.propTypes = {
hasChanges: PropTypes.bool.isRequired,
onClickPublish: PropTypes.func.isRequired,
onClickConfigure: PropTypes.func.isRequired,
onClickManageTags: PropTypes.func,
onClickMenuButton: PropTypes.func.isRequired,
onClickEdit: PropTypes.func.isRequired,
isFormOpen: PropTypes.bool.isRequired,
Expand Down Expand Up @@ -261,6 +277,7 @@ CardHeader.propTypes = {
isTimeLimited: PropTypes.bool,
graded: PropTypes.bool,
}),
tagsCount: PropTypes.number,
};

export default CardHeader;
30 changes: 29 additions & 1 deletion src/course-outline/card-header/CardHeader.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { MemoryRouter } from 'react-router-dom';
import {
act, render, fireEvent, waitFor,
act, render, fireEvent, waitFor, screen,
} from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';

Expand All @@ -18,6 +18,7 @@ const onClickDuplicateMock = jest.fn();
const onClickConfigureMock = jest.fn();
const onClickMoveUpMock = jest.fn();
const onClickMoveDownMock = jest.fn();
const onClickManageTagsMock = jest.fn();
const closeFormMock = jest.fn();

const cardHeaderProps = {
Expand All @@ -28,6 +29,7 @@ const cardHeaderProps = {
onClickMenuButton: onClickMenuButtonMock,
onClickPublish: onClickPublishMock,
onClickEdit: onClickEditMock,
onClickManageTags: onClickManageTagsMock,
isFormOpen: false,
onEditSubmit: jest.fn(),
closeForm: closeFormMock,
Expand Down Expand Up @@ -168,6 +170,16 @@ describe('<CardHeader />', () => {
expect(onClickPublishMock).toHaveBeenCalled();
});

it('calls onClickManageTags when the menu is clicked', async () => {
renderComponent();
const menuButton = await screen.findByTestId('subsection-card-header__menu-button');
fireEvent.click(menuButton);

const manageTagsMenuItem = await screen.findByText(messages.menuManageTags.defaultMessage);
await act(async () => fireEvent.click(manageTagsMenuItem));
expect(onClickManageTagsMock).toHaveBeenCalled();
});

it('calls onClickEdit when the button is clicked', async () => {
const { findByTestId } = renderComponent();

Expand Down Expand Up @@ -251,4 +263,20 @@ describe('<CardHeader />', () => {

expect(queryByText(messages.discussionEnabledBadgeText.defaultMessage)).toBeInTheDocument();
});

it('should render tag count if is not zero', () => {
renderComponent({
...cardHeaderProps,
tagsCount: 17,
});
expect(screen.getByText('17')).toBeInTheDocument();
});

it('should not render tag count if is zero', () => {
renderComponent({
...cardHeaderProps,
tagsCount: 0,
});
expect(screen.queryByText('0')).not.toBeInTheDocument();
});
});
Loading
Loading