diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 574e1617fb..470b38f9ce 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -99,11 +99,10 @@ import useContentTagsCollapsibleHelper from './ContentTagsCollapsibleHelper'; * @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 ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData }) => { const intl = useIntl(); - const { id, name } = taxonomyAndTagsData; + const { id, name, canTagObject } = taxonomyAndTagsData; const { tagChangeHandler, tagsTree, contentTagsCount, checkedTags, @@ -141,12 +140,12 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData, editable }) =>
- +
- {editable && ( + {canTagObject && (
@@ -73,10 +72,10 @@ ContentTagsTree.propTypes = { PropTypes.shape({ explicit: PropTypes.bool.isRequired, children: PropTypes.shape({}).isRequired, + canDeleteObjecttag: PropTypes.bool.isRequired, }).isRequired, ).isRequired, removeTagHandler: PropTypes.func.isRequired, - editable: PropTypes.bool.isRequired, }; export default ContentTagsTree; diff --git a/src/content-tags-drawer/ContentTagsTree.test.jsx b/src/content-tags-drawer/ContentTagsTree.test.jsx index ac41f3c1f1..23941393ab 100644 --- a/src/content-tags-drawer/ContentTagsTree.test.jsx +++ b/src/content-tags-drawer/ContentTagsTree.test.jsx @@ -1,13 +1,13 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { act, render } from '@testing-library/react'; -import PropTypes from 'prop-types'; import ContentTagsTree from './ContentTagsTree'; const data = { 'Science and Research': { explicit: false, + canDeleteObjecttag: false, children: { 'Genetics Subcategory': { explicit: false, @@ -15,8 +15,10 @@ const data = { 'DNA Sequencing': { explicit: true, children: {}, + canDeleteObjecttag: true, }, }, + canDeleteObjecttag: false, }, 'Molecular, Cellular, and Microbiology': { explicit: false, @@ -24,34 +26,27 @@ const data = { Virology: { explicit: true, children: {}, + canDeleteObjecttag: true, }, }, + canDeleteObjecttag: false, }, }, }, }; -const ContentTagsTreeComponent = ({ tagsTree, removeTagHandler, editable }) => ( +const ContentTagsTreeComponent = ({ tagsTree, removeTagHandler }) => ( - + ); -ContentTagsTreeComponent.propTypes = { - tagsTree: PropTypes.objectOf( - PropTypes.shape({ - explicit: PropTypes.bool.isRequired, - children: PropTypes.shape({}).isRequired, - }).isRequired, - ).isRequired, - removeTagHandler: PropTypes.func.isRequired, - editable: PropTypes.bool.isRequired, -}; +ContentTagsTreeComponent.propTypes = ContentTagsTree.propTypes; describe('', () => { it('should render taxonomy tags data along content tags number badge', async () => { await act(async () => { - const { getByText } = render( {}} editable />); + const { getByText } = render( {}} />); expect(getByText('Science and Research')).toBeInTheDocument(); expect(getByText('Genetics Subcategory')).toBeInTheDocument(); expect(getByText('Molecular, Cellular, and Microbiology')).toBeInTheDocument(); diff --git a/src/content-tags-drawer/TagBubble.jsx b/src/content-tags-drawer/TagBubble.jsx index 2287e48ab3..aa5c5d60ab 100644 --- a/src/content-tags-drawer/TagBubble.jsx +++ b/src/content-tags-drawer/TagBubble.jsx @@ -8,15 +8,15 @@ import PropTypes from 'prop-types'; import TagOutlineIcon from './TagOutlineIcon'; const TagBubble = ({ - value, implicit, level, lineage, removeTagHandler, editable, + value, implicit, level, lineage, removeTagHandler, canRemove, }) => { const className = `tag-bubble mb-2 border-light-300 ${implicit ? 'implicit' : ''}`; const handleClick = React.useCallback(() => { - if (!implicit && editable) { + if (!implicit && canRemove) { removeTagHandler(lineage.join(','), false); } - }, [implicit, lineage, editable, removeTagHandler]); + }, [implicit, lineage, canRemove, removeTagHandler]); return (
@@ -24,7 +24,7 @@ const TagBubble = ({ className={className} variant="light" iconBefore={!implicit ? Tag : TagOutlineIcon} - iconAfter={!implicit && editable ? Close : null} + iconAfter={!implicit && canRemove ? Close : null} onIconAfterClick={handleClick} > {value} @@ -36,6 +36,7 @@ const TagBubble = ({ TagBubble.defaultProps = { implicit: true, level: 0, + canRemove: false, }; TagBubble.propTypes = { @@ -44,7 +45,7 @@ TagBubble.propTypes = { level: PropTypes.number, lineage: PropTypes.arrayOf(PropTypes.string).isRequired, removeTagHandler: PropTypes.func.isRequired, - editable: PropTypes.bool.isRequired, + canRemove: PropTypes.bool, }; export default TagBubble; diff --git a/src/content-tags-drawer/TagBubble.test.jsx b/src/content-tags-drawer/TagBubble.test.jsx index e03fe18726..df73586426 100644 --- a/src/content-tags-drawer/TagBubble.test.jsx +++ b/src/content-tags-drawer/TagBubble.test.jsx @@ -1,7 +1,6 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { render, fireEvent } from '@testing-library/react'; -import PropTypes from 'prop-types'; import TagBubble from './TagBubble'; @@ -12,7 +11,7 @@ const data = { }; const TagBubbleComponent = ({ - value, implicit, level, lineage, removeTagHandler, editable, + value, implicit, level, lineage, removeTagHandler, canRemove, }) => ( ); @@ -29,23 +28,16 @@ const TagBubbleComponent = ({ TagBubbleComponent.defaultProps = { implicit: true, level: 0, + canRemove: false, }; -TagBubbleComponent.propTypes = { - value: PropTypes.string.isRequired, - implicit: PropTypes.bool, - level: PropTypes.number, - lineage: PropTypes.arrayOf(PropTypes.string).isRequired, - removeTagHandler: PropTypes.func.isRequired, - editable: PropTypes.bool.isRequired, -}; +TagBubbleComponent.propTypes = TagBubble.propTypes; describe('', () => { it('should render implicit tag', () => { const { container, getByText } = render( , @@ -58,12 +50,13 @@ describe('', () => { it('should render explicit tag', () => { const tagBubbleData = { implicit: false, + canRemove: true, ...data, }; const { container, getByText } = render( ', () => { it('should call removeTagHandler when "x" clicked on explicit tag', async () => { const tagBubbleData = { implicit: false, + canRemove: true, ...data, }; const { container } = render( ', () => { fireEvent.click(xButton); expect(data.removeTagHandler).toHaveBeenCalled(); }); + + it('should not show "x" when canRemove is not allowed', async () => { + const tagBubbleData = { + implicit: false, + canRemove: false, + ...data, + }; + const { container } = render( + , + ); + + expect(container.getElementsByClassName('pgn__chip__icon-after')[0]).toBeUndefined(); + }); }); diff --git a/src/content-tags-drawer/__mocks__/contentTaxonomyTagsMock.js b/src/content-tags-drawer/__mocks__/contentTaxonomyTagsMock.js index 2e8aa0bea7..fd8214f5cd 100644 --- a/src/content-tags-drawer/__mocks__/contentTaxonomyTagsMock.js +++ b/src/content-tags-drawer/__mocks__/contentTaxonomyTagsMock.js @@ -4,7 +4,7 @@ module.exports = { { name: 'FlatTaxonomy', taxonomyId: 3, - editable: true, + canTagObject: true, tags: [ { value: 'flat taxonomy tag 3856', @@ -17,7 +17,7 @@ module.exports = { { name: 'HierarchicalTaxonomy', taxonomyId: 4, - editable: true, + canTagObject: true, tags: [ { value: 'hierarchical taxonomy tag 1.7.59', diff --git a/src/content-tags-drawer/__mocks__/updateContentTaxonomyTagsMock.js b/src/content-tags-drawer/__mocks__/updateContentTaxonomyTagsMock.js index cc319f1197..0fb6ffc6f1 100644 --- a/src/content-tags-drawer/__mocks__/updateContentTaxonomyTagsMock.js +++ b/src/content-tags-drawer/__mocks__/updateContentTaxonomyTagsMock.js @@ -4,7 +4,7 @@ module.exports = { { name: 'FlatTaxonomy', taxonomyId: 3, - editable: true, + canTagObject: true, tags: [ { value: 'flat taxonomy tag 100', diff --git a/src/content-tags-drawer/data/api.js b/src/content-tags-drawer/data/api.js index b5b28514f9..86cbb192c2 100644 --- a/src/content-tags-drawer/data/api.js +++ b/src/content-tags-drawer/data/api.js @@ -76,7 +76,7 @@ export async function getContentData(contentId) { */ export async function updateContentTaxonomyTags(contentId, taxonomyId, tags) { let url = getContentTaxonomyTagsApiUrl(contentId); - url = `${url}?taxonomy=${taxonomyId}`; + url = `${url}&taxonomy=${taxonomyId}`; const { data } = await getAuthenticatedHttpClient().put(url, { tags }); return camelCaseObject(data[contentId]); } diff --git a/src/content-tags-drawer/data/api.test.js b/src/content-tags-drawer/data/api.test.js index d183f59ea2..229e6daee1 100644 --- a/src/content-tags-drawer/data/api.test.js +++ b/src/content-tags-drawer/data/api.test.js @@ -110,10 +110,10 @@ describe('content tags drawer api calls', () => { const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@aaf8b8eb86b54281aeeab12499d2cb0b'; const taxonomyId = 3; const tags = ['flat taxonomy tag 100', 'flat taxonomy tag 3856']; - axiosMock.onPut(`${getContentTaxonomyTagsApiUrl(contentId)}?taxonomy=${taxonomyId}`).reply(200, updateContentTaxonomyTagsMock); + axiosMock.onPut(`${getContentTaxonomyTagsApiUrl(contentId)}&taxonomy=${taxonomyId}`).reply(200, updateContentTaxonomyTagsMock); const result = await updateContentTaxonomyTags(contentId, taxonomyId, tags); - expect(axiosMock.history.put[0].url).toEqual(`${getContentTaxonomyTagsApiUrl(contentId)}?taxonomy=${taxonomyId}`); + expect(axiosMock.history.put[0].url).toEqual(`${getContentTaxonomyTagsApiUrl(contentId)}&taxonomy=${taxonomyId}`); expect(result).toEqual(updateContentTaxonomyTagsMock[contentId]); }); }); diff --git a/src/content-tags-drawer/data/types.mjs b/src/content-tags-drawer/data/types.mjs index 2145f41b95..3fad73ccf1 100644 --- a/src/content-tags-drawer/data/types.mjs +++ b/src/content-tags-drawer/data/types.mjs @@ -4,13 +4,15 @@ * @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 + * @property {boolean} canChangeObjecttag + * @property {boolean} canDeleteObjecttag */ /** * @typedef {Object} ContentTaxonomyTagData A list of the tags from one taxonomy that are applied to a content object. * @property {string} name * @property {number} taxonomyId - * @property {boolean} editable + * @property {boolean} canTagObject * @property {Tag[]} tags */ diff --git a/src/taxonomy/TaxonomyListPage.jsx b/src/taxonomy/TaxonomyListPage.jsx index 4e77b12641..0ebdaeff99 100644 --- a/src/taxonomy/TaxonomyListPage.jsx +++ b/src/taxonomy/TaxonomyListPage.jsx @@ -32,7 +32,7 @@ import TaxonomyCard from './taxonomy-card'; const ALL_TAXONOMIES = 'All taxonomies'; const UNASSIGNED = 'Unassigned'; -const TaxonomyListHeaderButtons = () => { +const TaxonomyListHeaderButtons = ({ canAddTaxonomy }) => { const intl = useIntl(); return ( <> @@ -71,6 +71,7 @@ const TaxonomyListHeaderButtons = () => { iconBefore={Add} onClick={() => importTaxonomy(intl)} data-testid="taxonomy-import-button" + disabled={!canAddTaxonomy} > {intl.formatMessage(messages.importButtonLabel)} @@ -157,6 +158,7 @@ const TaxonomyListPage = () => { return { taxonomyListData, isLoaded }; }; const { taxonomyListData, isLoaded } = useTaxonomyListData(); + const canAddTaxonomy = isLoaded ? taxonomyListData.canAddTaxonomy : false; const getOrgSelect = () => ( // Initialize organization select component @@ -178,7 +180,7 @@ const TaxonomyListPage = () => { } + headerActions={} hideBorder /> @@ -233,6 +235,10 @@ const TaxonomyListPage = () => { ); }; +TaxonomyListHeaderButtons.propTypes = { + canAddTaxonomy: PropTypes.bool.isRequired, +}; + OrganizationFilterSelector.propTypes = { isOrganizationListLoaded: PropTypes.bool.isRequired, organizationListData: PropTypes.arrayOf(PropTypes.string).isRequired, diff --git a/src/taxonomy/TaxonomyListPage.test.jsx b/src/taxonomy/TaxonomyListPage.test.jsx index 6dc3fe8b14..08437c46ea 100644 --- a/src/taxonomy/TaxonomyListPage.test.jsx +++ b/src/taxonomy/TaxonomyListPage.test.jsx @@ -21,6 +21,10 @@ const taxonomies = [{ id: 1, name: 'Taxonomy', description: 'This is a description', + showSystemBadge: false, + canChangeTaxonomy: true, + canDeleteTaxonomy: true, + tagsCount: 0, }]; const organizationsListUrl = 'http://localhost:18010/organizations'; const organizations = ['Org 1', 'Org 2']; @@ -90,6 +94,7 @@ describe('', () => { useIsTaxonomyListDataLoaded.mockReturnValue(true); useTaxonomyListDataResponse.mockReturnValue({ results: taxonomies, + canAddTaxonomy: false, }); await act(async () => { const { getByTestId } = render(); @@ -100,11 +105,8 @@ describe('', () => { it.each(['CSV', 'JSON'])('downloads the taxonomy template %s', async (fileFormat) => { useIsTaxonomyListDataLoaded.mockReturnValue(true); useTaxonomyListDataResponse.mockReturnValue({ - results: [{ - id: 1, - name: 'Taxonomy', - description: 'This is a description', - }], + results: taxonomies, + canAddTaxonomy: false, }); const { findByRole } = render(); const templateMenu = await findByRole('button', { name: 'Download template' }); @@ -115,19 +117,28 @@ describe('', () => { expect(templateButton.href).toBe(getTaxonomyTemplateApiUrl(fileFormat.toLowerCase())); }); + it('disables the import taxonomy button if not permitted', async () => { + useIsTaxonomyListDataLoaded.mockReturnValue(true); + useTaxonomyListDataResponse.mockReturnValue({ + results: [], + canAddTaxonomy: false, + }); + + const { getByRole } = render(); + const importButton = getByRole('button', { name: 'Import' }); + expect(importButton).toBeDisabled(); + }); + it('calls the import taxonomy action when the import button is clicked', async () => { useIsTaxonomyListDataLoaded.mockReturnValue(true); useTaxonomyListDataResponse.mockReturnValue({ - results: [{ - id: 1, - name: 'Taxonomy', - description: 'This is a description', - }], + results: [], + canAddTaxonomy: true, }); const { getByRole } = render(); const importButton = getByRole('button', { name: 'Import' }); - expect(importButton).toBeInTheDocument(); + expect(importButton).not.toBeDisabled(); fireEvent.click(importButton); expect(importTaxonomy).toHaveBeenCalled(); }); @@ -139,7 +150,12 @@ describe('', () => { id: 1, name: 'Taxonomy', description: 'This is a description', + showSystemBadge: false, + canChangeTaxonomy: false, + canDeleteTaxonomy: false, + tagsCount: 0, }], + canAddTaxonomy: false, }); const { @@ -171,6 +187,7 @@ describe('', () => { useIsTaxonomyListDataLoaded.mockReturnValue(true); useTaxonomyListDataResponse.mockReturnValue({ results: taxonomies, + canAddTaxonomy: false, }); const { getByRole } = render(); diff --git a/src/taxonomy/__mocks__/taxonomyListMock.js b/src/taxonomy/__mocks__/taxonomyListMock.js index 5eab0af023..0e7b24e4f7 100644 --- a/src/taxonomy/__mocks__/taxonomyListMock.js +++ b/src/taxonomy/__mocks__/taxonomyListMock.js @@ -5,6 +5,7 @@ module.exports = { numPages: 1, currentPage: 1, start: 0, + canAddTaxonomy: true, results: [ { id: -2, @@ -15,6 +16,8 @@ module.exports = { allowFreeText: false, systemDefined: true, visibleToAuthors: false, + canChangeTaxonomy: false, + canDeleteTaxonomy: false, }, { id: -1, @@ -25,6 +28,8 @@ module.exports = { allowFreeText: false, systemDefined: true, visibleToAuthors: true, + canChangeTaxonomy: false, + canDeleteTaxonomy: false, }, { id: 1, @@ -35,6 +40,8 @@ module.exports = { allowFreeText: false, systemDefined: false, visibleToAuthors: true, + canChangeTaxonomy: true, + canDeleteTaxonomy: true, }, { id: 2, @@ -45,6 +52,8 @@ module.exports = { allowFreeText: false, systemDefined: false, visibleToAuthors: true, + canChangeTaxonomy: true, + canDeleteTaxonomy: true, }, ], }; diff --git a/src/taxonomy/data/api.js b/src/taxonomy/data/api.js index 3c28b40bc4..c4a3a7d653 100644 --- a/src/taxonomy/data/api.js +++ b/src/taxonomy/data/api.js @@ -7,7 +7,6 @@ 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 if (org !== undefined) { if (org === 'Unassigned') { url.searchParams.append('unassigned', 'true'); diff --git a/src/taxonomy/data/types.mjs b/src/taxonomy/data/types.mjs index 60437e7c33..6609a342a2 100644 --- a/src/taxonomy/data/types.mjs +++ b/src/taxonomy/data/types.mjs @@ -13,6 +13,9 @@ * @property {number} tagsCount * @property {string[]} orgs * @property {boolean} allOrgs + * @property {boolean} canChangeTaxonomy + * @property {boolean} canDeleteTaxonomy + * @property {boolean} canTagObject */ /** @@ -24,5 +27,6 @@ * @property {number} currentPage * @property {number} start * @property {function} refetch + * @property {boolean} canAddTaxonomy * @property {TaxonomyData[]} results */ diff --git a/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx b/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx index 6cbbf4c630..a63b451d06 100644 --- a/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx +++ b/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx @@ -3,8 +3,7 @@ import { IntlProvider } from '@edx/frontend-platform/i18n'; import { initializeMockApp } from '@edx/frontend-platform'; import { AppProvider } from '@edx/frontend-platform/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { render } from '@testing-library/react'; -import PropTypes from 'prop-types'; +import { fireEvent, render } from '@testing-library/react'; import initializeStore from '../../store'; import TaxonomyCard from '.'; @@ -16,6 +15,10 @@ const data = { id: taxonomyId, name: 'Taxonomy 1', description: 'This is a description', + systemDefined: false, + canChangeTaxonomy: true, + canDeleteTaxonomy: true, + tagsCount: 0, }; const queryClient = new QueryClient(); @@ -32,16 +35,7 @@ const TaxonomyCardComponent = ({ original }) => ( ); -TaxonomyCardComponent.propTypes = { - original: PropTypes.shape({ - id: PropTypes.number, - name: PropTypes.string, - description: PropTypes.string, - systemDefined: PropTypes.bool, - orgsCount: PropTypes.number, - onDeleteTaxonomy: PropTypes.func, - }).isRequired, -}; +TaxonomyCardComponent.propTypes = TaxonomyCard.propTypes; describe('', async () => { beforeEach(async () => { @@ -62,16 +56,41 @@ describe('', async () => { expect(getByText(data.description)).toBeInTheDocument(); }); + it('should show the ⋮ menu', () => { + const { getByTestId, queryByTestId } = render(); + + // Menu closed/doesn't exist yet + expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument(); + + // Click on the menu button to open + fireEvent.click(getByTestId('taxonomy-menu-button')); + + // Menu opened + expect(getByTestId('taxonomy-menu')).toBeVisible(); + expect(getByTestId('taxonomy-menu-import')).toBeVisible(); + expect(getByTestId('taxonomy-menu-export')).toBeVisible(); + expect(getByTestId('taxonomy-menu-delete')).toBeVisible(); + + // Click on button again to close the menu + fireEvent.click(getByTestId('taxonomy-menu-button')); + + // Menu closed + // Jest bug: toBeVisible() isn't checking opacity correctly + // expect(getByTestId('taxonomy-menu')).not.toBeVisible(); + expect(getByTestId('taxonomy-menu').style.opacity).toEqual('0'); + + // Menu button still visible + expect(getByTestId('taxonomy-menu-button')).toBeVisible(); + }); + it('not show the system-defined badge with normal taxonomies', () => { const { queryByText } = render(); expect(queryByText('System-level')).not.toBeInTheDocument(); }); it('shows the system-defined badge with system taxonomies', () => { - const cardData = { - systemDefined: true, - ...data, - }; + const cardData = { ...data }; + cardData.systemDefined = true; const { getByText } = render(); expect(getByText('System-level')).toBeInTheDocument(); diff --git a/src/taxonomy/taxonomy-card/index.jsx b/src/taxonomy/taxonomy-card/index.jsx index 1449730ae5..87d6da9f17 100644 --- a/src/taxonomy/taxonomy-card/index.jsx +++ b/src/taxonomy/taxonomy-card/index.jsx @@ -149,6 +149,8 @@ TaxonomyCard.propTypes = { systemDefined: PropTypes.bool, orgsCount: PropTypes.number, tagsCount: PropTypes.number, + canChangeTaxonomy: PropTypes.bool, + canDeleteTaxonomy: PropTypes.bool, }).isRequired, }; diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx index 49916995a5..bbcb68059a 100644 --- a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx +++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx @@ -1,11 +1,10 @@ -import React from 'react'; +import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { AppProvider } from '@edx/frontend-platform/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { render } from '@testing-library/react'; -import MockAdapter from 'axios-mock-adapter'; +import { fireEvent, render } from '@testing-library/react'; import { getTaxonomyApiUrl } from '../data/api'; import initializeStore from '../../store'; @@ -87,11 +86,26 @@ describe('', () => { name: 'Test taxonomy', description: 'This is a description', system_defined: false, + can_change_taxonomy: true, + can_delete_taxonomy: true, + tagsCount: 0, }); - const { findByRole } = render(); + const { getByTestId, queryByTestId, findByRole } = render(); expect(await findByRole('heading')).toHaveTextContent('Test taxonomy'); + + // Menu closed/doesn't exist yet + expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument(); + + // Click on the menu button to open + fireEvent.click(getByTestId('taxonomy-menu-button')); + + // Menu opened + expect(getByTestId('taxonomy-menu')).toBeVisible(); + expect(getByTestId('taxonomy-menu-import')).toBeVisible(); + expect(getByTestId('taxonomy-menu-export')).toBeVisible(); + expect(getByTestId('taxonomy-menu-delete')).toBeVisible(); }); it('should show system defined badge', async () => { @@ -100,9 +114,12 @@ describe('', () => { name: 'Test taxonomy', description: 'This is a description', system_defined: true, + can_change_taxonomy: false, + can_delete_taxonomy: false, }); const { findByRole, getByText } = render(); + expect(await findByRole('heading')).toHaveTextContent('Test taxonomy'); expect(getByText('System-level')).toBeInTheDocument(); }); @@ -113,6 +130,8 @@ describe('', () => { name: 'Test taxonomy', description: 'This is a description', system_defined: false, + can_change_taxonomy: false, + can_delete_taxonomy: false, }); const { findByRole, queryByText } = render(); diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.jsx index 57fe8614fd..e4eec9cb68 100644 --- a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.jsx +++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.jsx @@ -2,7 +2,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Card, } from '@edx/paragon'; -import Proptypes from 'prop-types'; +import PropTypes from 'prop-types'; import messages from './messages'; @@ -23,9 +23,9 @@ const TaxonomyDetailSideCard = ({ taxonomy }) => { }; TaxonomyDetailSideCard.propTypes = { - taxonomy: Proptypes.shape({ - name: Proptypes.string.isRequired, - description: Proptypes.string.isRequired, + taxonomy: PropTypes.shape({ + name: PropTypes.string.isRequired, + description: PropTypes.string.isRequired, }).isRequired, }; diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx index fb053eca72..7b2dd1fd3a 100644 --- a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx +++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx @@ -3,7 +3,6 @@ import { IntlProvider } from '@edx/frontend-platform/i18n'; import { initializeMockApp } from '@edx/frontend-platform'; import { AppProvider } from '@edx/frontend-platform/react'; import { render } from '@testing-library/react'; -import PropTypes from 'prop-types'; import initializeStore from '../../store'; @@ -25,12 +24,7 @@ const TaxonomyCardComponent = ({ taxonomy }) => ( ); -TaxonomyCardComponent.propTypes = { - taxonomy: PropTypes.shape({ - name: PropTypes.string, - description: PropTypes.string, - }).isRequired, -}; +TaxonomyCardComponent.propTypes = TaxonomyDetailSideCard.propTypes; describe('', async () => { beforeEach(async () => { diff --git a/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx b/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx index c1d15da5a3..55385426f8 100644 --- a/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx +++ b/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx @@ -9,7 +9,7 @@ import { IconButton, } from '@edx/paragon'; import { MoreVert } from '@edx/paragon/icons'; -import { omitBy } from 'lodash'; +import { pickBy } from 'lodash'; import PropTypes from 'prop-types'; import { useNavigate } from 'react-router-dom'; @@ -53,7 +53,7 @@ const TaxonomyMenu = ({ * @typedef {Object} MenuItem * @property {string} title - The title of the menu item * @property {() => void} action - The action to perform when the menu item is clicked - * @property {boolean} [hide] - Whether or not to hide the menu item + * @property {boolean} [show] - Whether or not to show the menu item * * @constant * @type {Record} @@ -62,29 +62,27 @@ const TaxonomyMenu = ({ import: { title: intl.formatMessage(messages.importMenu), action: importModalOpen, - // Hide import menu item if taxonomy is system defined or allows free text - hide: taxonomy.systemDefined || taxonomy.allowFreeText, + show: taxonomy.canChangeTaxonomy && !taxonomy.systemDefined, }, export: { title: intl.formatMessage(messages.exportMenu), action: exportModalOpen, + show: true, // if we can view the taxonomy, we can export it }, delete: { title: intl.formatMessage(messages.deleteMenu), action: deleteDialogOpen, - // Hide delete menu item if taxonomy is system defined - hide: taxonomy.systemDefined, + show: taxonomy.canDeleteTaxonomy && !taxonomy.systemDefined, }, manageOrgs: { title: intl.formatMessage(messages.manageOrgsMenu), action: manageOrgsModalOpen, - // Hide import menu item if taxonomy is system defined - hide: taxonomy.systemDefined, + show: taxonomy.canChangeTaxonomy, }, }; // Remove hidden menu items - menuItems = omitBy(menuItems, (value) => value.hide); + menuItems = pickBy(menuItems, (value) => value.show); const renderModals = () => ( <> @@ -159,9 +157,10 @@ TaxonomyMenu.propTypes = { taxonomy: PropTypes.shape({ id: PropTypes.number.isRequired, name: PropTypes.string.isRequired, - systemDefined: PropTypes.bool.isRequired, - allowFreeText: PropTypes.bool.isRequired, tagsCount: PropTypes.number.isRequired, + systemDefined: PropTypes.bool.isRequired, + canChangeTaxonomy: PropTypes.bool.isRequired, + canDeleteTaxonomy: PropTypes.bool.isRequired, }).isRequired, iconMenu: PropTypes.bool, }; diff --git a/src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx b/src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx index f6c0ba767d..666c2543fb 100644 --- a/src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx +++ b/src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx @@ -4,7 +4,6 @@ import { initializeMockApp } from '@edx/frontend-platform'; import { AppProvider } from '@edx/frontend-platform/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { render, fireEvent, waitFor } from '@testing-library/react'; -import PropTypes from 'prop-types'; import { TaxonomyContext } from '../common/context'; import initializeStore from '../../store'; @@ -27,9 +26,10 @@ const queryClient = new QueryClient(); const mockSetToastMessage = jest.fn(); const TaxonomyMenuComponent = ({ - systemDefined, - allowFreeText, iconMenu, + systemDefined, + canChangeTaxonomy, + canDeleteTaxonomy, }) => { const context = useMemo(() => ({ toastMessage: null, @@ -45,9 +45,10 @@ const TaxonomyMenuComponent = ({ taxonomy={{ id: taxonomyId, name: taxonomyName, - systemDefined, - allowFreeText, tagsCount: 0, + systemDefined, + canChangeTaxonomy, + canDeleteTaxonomy, }} iconMenu={iconMenu} /> @@ -58,15 +59,15 @@ const TaxonomyMenuComponent = ({ ); }; -TaxonomyMenuComponent.propTypes = { - iconMenu: PropTypes.bool.isRequired, - systemDefined: PropTypes.bool, - allowFreeText: PropTypes.bool, -}; +TaxonomyMenuComponent.propTypes = TaxonomyMenu.propTypes; TaxonomyMenuComponent.defaultProps = { + id: taxonomyId, + name: taxonomyName, + tagsCount: 0, systemDefined: false, - allowFreeText: false, + canChangeTaxonomy: true, + canDeleteTaxonomy: true, }; describe.each([true, false])('', async (iconMenu) => { @@ -86,20 +87,22 @@ describe.each([true, false])('', async (iconMenu) => jest.clearAllMocks(); }); - test('should open and close menu on button click', () => { - const { getByTestId, queryByTestId } = render(); + test('should open and close menu on button click', async () => { + const { findByTestId, getByTestId, queryByTestId } = render( + , + ); // Menu closed/doesn't exist yet expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument(); // Click on the menu button to open - fireEvent.click(getByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); // Menu opened expect(getByTestId('taxonomy-menu')).toBeVisible(); // Click on button again to close the menu - fireEvent.click(getByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); // Menu closed // Jest bug: toBeVisible() isn't checking opacity correctly @@ -110,48 +113,76 @@ describe.each([true, false])('', async (iconMenu) => expect(getByTestId('taxonomy-menu-button')).toBeVisible(); }); - test('doesnt show systemDefined taxonomies disabled menus', () => { - const { getByTestId, queryByTestId } = render(); - - // Menu closed/doesn't exist yet - expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument(); + test('Shows menu actions that user is permitted', async () => { + const { findByTestId, getByTestId, queryByTestId } = render( + , + ); // Click on the menu button to open - fireEvent.click(getByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); // Menu opened expect(getByTestId('taxonomy-menu')).toBeVisible(); - // Check that the import menu is not show + // Ensure expected menu items are found + expect(queryByTestId('taxonomy-menu-export')).toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-import')).toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-manageOrgs')).toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-delete')).toBeInTheDocument(); + }); + + test('Hides menu actions that user is not permitted', async () => { + const { findByTestId, queryByTestId } = render( + , + ); + + // Click on the menu button to open + fireEvent.click(await findByTestId('taxonomy-menu-button')); + + // Ensure expected menu items are found + expect(queryByTestId('taxonomy-menu-export')).toBeInTheDocument(); expect(queryByTestId('taxonomy-menu-import')).not.toBeInTheDocument(); expect(queryByTestId('taxonomy-menu-manageOrgs')).not.toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-delete')).not.toBeInTheDocument(); }); - test('doesnt show freeText taxonomies disabled menus', () => { - const { getByTestId, queryByTestId } = render(); - - // Menu closed/doesn't exist yet - expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument(); + test('Hides import/delete actions for system-defined taxonomies', async () => { + const systemDefined = true; + const { queryByTestId } = render( + , + ); // Click on the menu button to open - fireEvent.click(getByTestId('taxonomy-menu-button')); + fireEvent.click(queryByTestId('taxonomy-menu-button')); - // Menu opened - expect(getByTestId('taxonomy-menu')).toBeVisible(); - - // Check that the import menu is not show + // Ensure expected menu items are found + expect(queryByTestId('taxonomy-menu-export')).toBeInTheDocument(); expect(queryByTestId('taxonomy-menu-import')).not.toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-manageOrgs')).toBeInTheDocument(); + expect(queryByTestId('taxonomy-menu-delete')).not.toBeInTheDocument(); }); - test('should open export modal on export menu click', () => { - const { getByTestId, getByText, queryByText } = render(); + test('should open export modal on export menu click', async () => { + const { findByTestId, getByText, queryByText } = render( + , + ); // Modal closed expect(queryByText('Select format to export')).not.toBeInTheDocument(); // Click on export menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-export')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-export')); // Modal opened expect(getByText('Select format to export')).toBeInTheDocument(); @@ -163,22 +194,24 @@ describe.each([true, false])('', async (iconMenu) => expect(queryByText('Select format to export')).not.toBeInTheDocument(); }); - test('should call import tags when menu click', () => { - const { getByTestId, getByText } = render(); + test('should call import tags when menu click', async () => { + const { findByTestId, getByText } = render(); // Click on import menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-import')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-import')); expect(getByText('Update "Taxonomy 1"')).toBeInTheDocument(); }); - test('should export a taxonomy', () => { - const { getByTestId, getByText, queryByText } = render(); + test('should export a taxonomy', async () => { + const { + findByTestId, getByTestId, getByText, queryByText, + } = render(); // Click on export menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-export')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-export')); // Select JSON format and click on export fireEvent.click(getByText('JSON file')); @@ -189,15 +222,17 @@ describe.each([true, false])('', async (iconMenu) => expect(getTaxonomyExportFile).toHaveBeenCalledWith(taxonomyId, 'json'); }); - test('should open delete dialog on delete menu click', () => { - const { getByTestId, getByText, queryByText } = render(); + test('should open delete dialog on delete menu click', async () => { + const { findByTestId, getByText, queryByText } = render( + , + ); // Modal closed expect(queryByText(`Delete "${taxonomyName}"`)).not.toBeInTheDocument(); // Click on delete menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-delete')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-delete')); // Modal opened expect(getByText(`Delete "${taxonomyName}"`)).toBeInTheDocument(); @@ -210,11 +245,13 @@ describe.each([true, false])('', async (iconMenu) => }); test('should delete a taxonomy', async () => { - const { getByTestId, getByLabelText, queryByText } = render(); + const { + getByTestId, getByLabelText, findByTestId, queryByText, + } = render(); // Click on delete menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-delete')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-delete')); const deleteButton = getByTestId('delete-button'); @@ -246,7 +283,7 @@ describe.each([true, false])('', async (iconMenu) => it('should open manage orgs dialog menu click', async () => { const { - findByText, getByTestId, getByText, queryByText, + findByTestId, findByText, getByText, queryByText, } = render(); // We need to provide a taxonomy or the modal will not open @@ -261,8 +298,8 @@ describe.each([true, false])('', async (iconMenu) => expect(queryByText('Assign to organizations')).not.toBeInTheDocument(); // Click on delete menu - fireEvent.click(getByTestId('taxonomy-menu-button')); - fireEvent.click(getByTestId('taxonomy-menu-manageOrgs')); + fireEvent.click(await findByTestId('taxonomy-menu-button')); + fireEvent.click(await findByTestId('taxonomy-menu-manageOrgs')); // Modal opened expect(await findByText('Assign to organizations')).toBeInTheDocument();