fix: renames the can<Action> fields to can<Action><Modelname>

to alleviate confusion about what permissions they reflect. Also fixes some properties and wraps menu clicks in act() to remove errors from the tests.
openedx · Jan 12, 2024 · d5456b1 · d5456b1
1 parent 93b0ad0
commit d5456b1
Show file tree

Hide file tree

Showing 17 changed files with 220 additions and 98 deletions.
diff --git a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx
@@ -22,6 +22,7 @@ jest.mock('./data/apiHooks', () => ({
     tagPages: [{
       isLoading: true,
       isError: false,
+      canAddTag: false,
       data: [],
     }],
   })),
@@ -32,21 +33,24 @@ const data = {
   taxonomyAndTagsData: {
     id: 123,
     name: 'Taxonomy 1',
+    canTagObject: true,
     contentTags: [
       {
         value: 'Tag 1',
         lineage: ['Tag 1'],
+        canDeleteObjecttag: true,
       },
       {
         value: 'Tag 1.1',
         lineage: ['Tag 1', 'Tag 1.1'],
+        canDeleteObjecttag: true,
       },
       {
         value: 'Tag 2',
         lineage: ['Tag 2'],
+        canDeleteObjecttag: true,
       },
     ],
-    canTagObject: true,
   },
 };
 
@@ -61,11 +65,12 @@ ContentTagsCollapsibleComponent.propTypes = {
   taxonomyAndTagsData: PropTypes.shape({
     id: PropTypes.number,
     name: PropTypes.string,
+    canTagObject: PropTypes.bool.isRequired,
     contentTags: PropTypes.arrayOf(PropTypes.shape({
       value: PropTypes.string,
       lineage: PropTypes.arrayOf(PropTypes.string),
+      canDeleteObjecttag: PropTypes.bool.isRequired,
     })),
-    canTagObjects: PropTypes.bool.isRequired,
   }).isRequired,
 };
 
@@ -92,6 +97,7 @@ describe('<ContentTagsCollapsible />', () => {
   function setupTaxonomyMock() {
     useTaxonomyTagsData.mockReturnValue({
       hasMorePages: false,
+      canAddTag: false,
       tagPages: [{
         isLoading: false,
         isError: false,
@@ -103,6 +109,8 @@ describe('<ContentTagsCollapsible />', () => {
           parentValue: null,
           id: 12345,
           subTagsUrl: null,
+          canChangeTag: false,
+          canDeleteTag: false,
         }, {
           value: 'Tag 2',
           externalId: null,
@@ -111,6 +119,8 @@ describe('<ContentTagsCollapsible />', () => {
           parentValue: null,
           id: 12346,
           subTagsUrl: null,
+          canChangeTag: false,
+          canDeleteTag: false,
         }, {
           value: 'Tag 3',
           externalId: null,
@@ -119,6 +129,8 @@ describe('<ContentTagsCollapsible />', () => {
           parentValue: null,
           id: 12347,
           subTagsUrl: null,
+          canChangeTag: false,
+          canDeleteTag: false,
         }],
       }],
     });

diff --git a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx
@@ -128,7 +128,8 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => {
           currentLevel[key] = {
             explicit: isExplicit,
             children: {},
-            canDelete: item.canDelete,
+            canChangeObjecttag: item.canChangeObjecttag,
+            canDeleteObjecttag: item.canDeleteObjecttag,
           };
 
           // Populating the SelectableBox with "selected" (explicit) tags
@@ -163,7 +164,12 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => {
       const isExplicit = selectedTag === tag;
 
       if (!traversal[tag]) {
-        traversal[tag] = { explicit: isExplicit, children: {} };
+        traversal[tag] = {
+          explicit: isExplicit,
+          children: {},
+          canChangeObjecttag: false,
+          canDeleteObjecttag: false,
+        };
       } else {
         traversal[tag].explicit = isExplicit;
       }

diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx
@@ -91,10 +91,12 @@ describe('<ContentTagsDrawer />', () => {
               {
                 value: 'Tag 1',
                 lineage: ['Tag 1'],
+                canDeleteObjecttag: true,
               },
               {
                 value: 'Tag 2',
                 lineage: ['Tag 2'],
+                canDeleteObjecttag: true,
               },
             ],
           },
@@ -106,6 +108,7 @@ describe('<ContentTagsDrawer />', () => {
               {
                 value: 'Tag 3',
                 lineage: ['Tag 3'],
+                canDeleteObjecttag: true,
               },
             ],
           },
@@ -117,10 +120,12 @@ describe('<ContentTagsDrawer />', () => {
         id: 123,
         name: 'Taxonomy 1',
         description: 'This is a description 1',
+        canTagObject: false,
       }, {
         id: 124,
         name: 'Taxonomy 2',
         description: 'This is a description 2',
+        canTagObject: false,
       }],
     });
     await act(async () => {

diff --git a/src/content-tags-drawer/ContentTagsTree.jsx b/src/content-tags-drawer/ContentTagsTree.jsx
@@ -55,7 +55,7 @@ const ContentTagsTree = ({ tagsTree, removeTagHandler }) => {
             level={level}
             lineage={updatedLineage}
             removeTagHandler={removeTagHandler}
-            canDelete={tag[key].canDelete}
+            canRemove={tag[key].canDeleteObjecttag}
           />
           { renderTagsTree(tag[key].children, level + 1, updatedLineage) }
         </div>
@@ -72,7 +72,7 @@ ContentTagsTree.propTypes = {
     PropTypes.shape({
       explicit: PropTypes.bool.isRequired,
       children: PropTypes.shape({}).isRequired,
-      canDelete: PropTypes.bool.isRequired,
+      canDeleteObjecttag: PropTypes.bool.isRequired,
     }).isRequired,
   ).isRequired,
   removeTagHandler: PropTypes.func.isRequired,

diff --git a/src/content-tags-drawer/ContentTagsTree.test.jsx b/src/content-tags-drawer/ContentTagsTree.test.jsx
@@ -8,28 +8,29 @@ import ContentTagsTree from './ContentTagsTree';
 const data = {
   'Science and Research': {
     explicit: false,
+    canDeleteObjecttag: false,
     children: {
       'Genetics Subcategory': {
         explicit: false,
         children: {
           'DNA Sequencing': {
             explicit: true,
             children: {},
-            canDelete: true,
+            canDeleteObjecttag: true,
           },
         },
-        canDelete: false,
+        canDeleteObjecttag: false,
       },
       'Molecular, Cellular, and Microbiology': {
         explicit: false,
         children: {
           Virology: {
             explicit: true,
             children: {},
-            canDelete: true,
+            canDeleteObjecttag: true,
           },
         },
-        canDelete: false,
+        canDeleteObjecttag: false,
       },
     },
   },
@@ -46,7 +47,7 @@ ContentTagsTreeComponent.propTypes = {
     PropTypes.shape({
       explicit: PropTypes.bool.isRequired,
       children: PropTypes.shape({}).isRequired,
-      canDelete: PropTypes.bool.isRequired,
+      canDeleteObjecttag: PropTypes.bool.isRequired,
     }).isRequired,
   ).isRequired,
   removeTagHandler: PropTypes.func.isRequired,

diff --git a/src/content-tags-drawer/TagBubble.jsx b/src/content-tags-drawer/TagBubble.jsx
@@ -8,23 +8,23 @@ import PropTypes from 'prop-types';
 import TagOutlineIcon from './TagOutlineIcon';
 
 const TagBubble = ({
-  value, implicit, level, lineage, removeTagHandler, canDelete,
+  value, implicit, level, lineage, removeTagHandler, canRemove,
 }) => {
   const className = `tag-bubble mb-2 border-light-300 ${implicit ? 'implicit' : ''}`;
 
   const handleClick = React.useCallback(() => {
-    if (!implicit && canDelete) {
+    if (!implicit && canRemove) {
       removeTagHandler(lineage.join(','), false);
     }
-  }, [implicit, lineage, canDelete, removeTagHandler]);
+  }, [implicit, lineage, canRemove, removeTagHandler]);
 
   return (
     <div style={{ paddingLeft: `${level * 1}rem` }}>
       <Chip
         className={className}
         variant="light"
         iconBefore={!implicit ? Tag : TagOutlineIcon}
-        iconAfter={!implicit && canDelete ? 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,
-  canDelete: 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
@@ -12,7 +12,7 @@ const data = {
 };
 
 const TagBubbleComponent = ({
-  value, implicit, level, lineage, removeTagHandler, canDelete,
+  value, implicit, level, lineage, removeTagHandler, canRemove,
 }) => (
   <IntlProvider locale="en" messages={{}}>
     <TagBubble
@@ -21,14 +21,15 @@ const TagBubbleComponent = ({
       level={level}
       lineage={lineage}
       removeTagHandler={removeTagHandler}
-      canDelete={canDelete}
+      canRemove={canRemove}
     />
   </IntlProvider>
 );
 
 TagBubbleComponent.defaultProps = {
   implicit: true,
   level: 0,
+  canRemove: false,
 };
 
 TagBubbleComponent.propTypes = {
@@ -37,15 +38,14 @@ TagBubbleComponent.propTypes = {
   level: PropTypes.number,
   lineage: PropTypes.arrayOf(PropTypes.string).isRequired,
   removeTagHandler: PropTypes.func.isRequired,
-  canDelete: PropTypes.bool.isRequired,
+  canRemove: PropTypes.bool,
 };
 
 describe('<TagBubble />', () => {
   it('should render implicit tag', () => {
     const { container, getByText } = render(
       <TagBubbleComponent
         value={data.value}
-        canDelete
         lineage={data.lineage}
         removeTagHandler={data.removeTagHandler}
       />,
@@ -58,12 +58,13 @@ describe('<TagBubble />', () => {
   it('should render explicit tag', () => {
     const tagBubbleData = {
       implicit: false,
+      canRemove: true,
       ...data,
     };
     const { container, getByText } = render(
       <TagBubbleComponent
         value={tagBubbleData.value}
-        canDelete
+        canRemove={tagBubbleData.canRemove}
         lineage={data.lineage}
         implicit={tagBubbleData.implicit}
         removeTagHandler={tagBubbleData.removeTagHandler}
@@ -77,12 +78,13 @@ describe('<TagBubble />', () => {
   it('should call removeTagHandler when "x" clicked on explicit tag', async () => {
     const tagBubbleData = {
       implicit: false,
+      canRemove: true,
       ...data,
     };
     const { container } = render(
       <TagBubbleComponent
         value={tagBubbleData.value}
-        canDelete
+        canRemove={tagBubbleData.canRemove}
         lineage={data.lineage}
         implicit={tagBubbleData.implicit}
         removeTagHandler={tagBubbleData.removeTagHandler}
@@ -93,4 +95,22 @@ describe('<TagBubble />', () => {
     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(
+      <TagBubbleComponent
+        value={tagBubbleData.value}
+        lineage={data.lineage}
+        implicit={tagBubbleData.implicit}
+        removeTagHandler={tagBubbleData.removeTagHandler}
+      />,
+    );
+
+    expect(container.getElementsByClassName('pgn__chip__icon-after')[0]).toBeUndefined();
+  });
 });
diff --git a/src/content-tags-drawer/data/types.mjs b/src/content-tags-drawer/data/types.mjs
@@ -4,6 +4,8 @@
  * @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
  */
 
 /**

diff --git 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 = ({ canAdd }) => {
+const TaxonomyListHeaderButtons = ({ canAddTaxonomy }) => {
   const intl = useIntl();
   return (
     <>
@@ -71,7 +71,7 @@ const TaxonomyListHeaderButtons = ({ canAdd }) => {
         iconBefore={Add}
         onClick={() => importTaxonomy(intl)}
         data-testid="taxonomy-import-button"
-        disabled={!canAdd}
+        disabled={!canAddTaxonomy}
       >
         {intl.formatMessage(messages.importButtonLabel)}
       </Button>
@@ -158,7 +158,7 @@ const TaxonomyListPage = () => {
     return { taxonomyListData, isLoaded };
   };
   const { taxonomyListData, isLoaded } = useTaxonomyListData();
-  const canAdd = isLoaded ? taxonomyListData.canAdd : false;
+  const canAddTaxonomy = isLoaded ? taxonomyListData.canAddTaxonomy : false;
 
   const getOrgSelect = () => (
     // Initialize organization select component
@@ -180,7 +180,7 @@ const TaxonomyListPage = () => {
           <SubHeader
             title={intl.formatMessage(messages.headerTitle)}
             titleActions={getOrgSelect()}
-            headerActions={<TaxonomyListHeaderButtons canAdd={canAdd} />}
+            headerActions={<TaxonomyListHeaderButtons canAddTaxonomy={canAddTaxonomy} />}
             hideBorder
           />
         </Container>
@@ -236,7 +236,7 @@ const TaxonomyListPage = () => {
 };
 
 TaxonomyListHeaderButtons.propTypes = {
-  canAdd: PropTypes.bool.isRequired,
+  canAddTaxonomy: PropTypes.bool.isRequired,
 };
 
 OrganizationFilterSelector.propTypes = {