fix: minor react errors in course authoring mfe [FC-0036] (openedx#789)

* fix: remove console warnings and add missing typing checks * fix: TagData <> TagListData swap names * fix: toast needs show property * fix: remove type guard from tagsCount * fix: apply suggestions from code review Co-authored-by: Jillian <[email protected]>
eduNEXT · Jan 24, 2024 · 3842b04 · 3842b04
1 parent c2ad1b8
commit 3842b04
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 32 deletions.
diff --git a/src/content-tags-drawer/data/api.js b/src/content-tags-drawer/data/api.js
@@ -36,7 +36,7 @@ export const getLibraryContentDataApiUrl = (contentId) => new URL(`/api/librarie
  * Get all tags that belong to taxonomy.
  * @param {number} taxonomyId The id of the taxonomy to fetch tags for
  * @param {{page?: number, searchTerm?: string, parentTag?: string}} options
- * @returns {Promise<import("../../taxonomy/tag-list/data/types.mjs").TagData>}
+ * @returns {Promise<import("../../taxonomy/tag-list/data/types.mjs").TagListData>}
  */
 export async function getTaxonomyTagsData(taxonomyId, options = {}) {
   const url = getTaxonomyTagsApiUrl(taxonomyId, options);

diff --git a/src/content-tags-drawer/data/apiHooks.jsx b/src/content-tags-drawer/data/apiHooks.jsx
@@ -27,7 +27,7 @@ import {
  *  tagPages: {
  *    isLoading: boolean,
  *    isError: boolean,
- *    data: TagListData[],
+ *    data: TagData[],
  *  }[],
  * }}
  */
@@ -53,14 +53,14 @@ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1,
   const hasMorePages = numPages < totalPages;
 
   const tagPages = useMemo(() => {
-    /** @type { { isLoading: boolean, isError: boolean, data: TagListData[] }[] } */
+    /** @type { { isLoading: boolean, isError: boolean, data: TagData[] }[] } */
     const newTags = [];
 
     // Pre-load desendants if possible
     const preLoadedData = new Map();
 
     dataPages.forEach(result => {
-      /** @type {TagListData[]} */
+      /** @type {TagData[]} */
       const simplifiedTagsList = [];
 
       result.data?.results?.forEach((tag) => {
@@ -79,7 +79,7 @@ export const useTaxonomyTagsData = (taxonomyId, parentTag = null, numPages = 1,
     // Store the pre-loaded descendants into the query cache:
     preLoadedData.forEach((tags, parentValue) => {
       const queryKey = ['taxonomyTags', taxonomyId, parentValue, 1, searchTerm];
-      /** @type {TagData} */
+      /** @type {TagListData} */
       const cachedData = {
         next: '',
         previous: '',

diff --git a/src/taxonomy/TaxonomyListPage.jsx b/src/taxonomy/TaxonomyListPage.jsx
@@ -1,3 +1,4 @@
+// @ts-check
 import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import {
@@ -39,13 +40,14 @@ const TaxonomyListHeaderButtons = () => {
       <OverlayTrigger
         placement="top"
         overlay={(
-          <Tooltip>
+          <Tooltip id="download-template-tooltip">
             {intl.formatMessage(messages.downloadTemplateButtonHint)}
           </Tooltip>
         )}
       >
-        <Dropdown>
+        <Dropdown id="download-template-dropdown">
           <Dropdown.Toggle
+            id="download-template-dropdown-toggle"
             variant="outline-primary"
             data-testid="taxonomy-download-template"
           >
@@ -185,7 +187,7 @@ const TaxonomyListPage = () => {
       </div>
       <div className="bg-light-400 mt-1">
         <Container size="xl">
-          {isLoaded && (
+          {isLoaded && taxonomyListData && (
             <DataTable
               disableElevation
               data={taxonomyListData.results}
@@ -208,6 +210,7 @@ const TaxonomyListPage = () => {
                   accessor: 'systemDefined',
                 },
                 {
+                  Header: '',
                   accessor: 'tagsCount',
                 },
               ]}
@@ -235,11 +238,15 @@ const TaxonomyListPage = () => {
 
 OrganizationFilterSelector.propTypes = {
   isOrganizationListLoaded: PropTypes.bool.isRequired,
-  organizationListData: PropTypes.arrayOf(PropTypes.string).isRequired,
+  organizationListData: PropTypes.arrayOf(PropTypes.string),
   selectedOrgFilter: PropTypes.string.isRequired,
   setSelectedOrgFilter: PropTypes.func.isRequired,
 };
 
+OrganizationFilterSelector.defaultProps = {
+  organizationListData: null,
+};
+
 TaxonomyListPage.propTypes = {};
 
 export default TaxonomyListPage;
diff --git a/src/taxonomy/delete-dialog/index.jsx b/src/taxonomy/delete-dialog/index.jsx
@@ -1,4 +1,5 @@
-import React, { useState } from 'react';
+// @ts-check
+import React, { useCallback, useState } from 'react';
 import {
   ActionRow,
   Button,
@@ -23,13 +24,13 @@ const DeleteDialog = ({
   const [deleteButtonDisabled, setDeleteButtonDisabled] = useState(true);
   const deleteLabel = intl.formatMessage(messages.deleteDialogConfirmDeleteLabel);
 
-  const handleInputChange = React.useCallback((event) => {
+  const handleInputChange = useCallback((event) => {
     if (event.target.value === deleteLabel) {
       setDeleteButtonDisabled(false);
     } else {
       setDeleteButtonDisabled(true);
     }
-  });
+  }, []);
 
   const onClickDelete = React.useCallback(() => {
     onClose();
@@ -55,10 +56,7 @@ const DeleteDialog = ({
         </ModalDialog.Header>
         <ModalDialog.Body>
           <div className="mb-4">
-            {/* Delete `(?)` after implement get tags count of a taxonomy */}
-            {intl.formatMessage(messages.deleteDialogBody, {
-              tagsCount: tagsCount !== undefined ? tagsCount : '(?)',
-            })}
+            {intl.formatMessage(messages.deleteDialogBody, { tagsCount })}
           </div>
           <Form.Group>
             <Form.Label>

diff --git a/src/taxonomy/tag-list/TagListTable.jsx b/src/taxonomy/tag-list/TagListTable.jsx
@@ -1,9 +1,9 @@
-// ts-check
+// @ts-check
+import React, { useState } from 'react';
 import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
 import { DataTable } from '@edx/paragon';
-import _ from 'lodash';
+import { isEqual } from 'lodash';
 import Proptypes from 'prop-types';
-import { useState } from 'react';
 
 import { LoadingSpinner } from '../../generic/Loading';
 import messages from './messages';
@@ -51,7 +51,14 @@ const TagValue = ({ row }) => (
     <span className="text-secondary-500">{` (${row.original.childCount})`}</span>
   </>
 );
-TagValue.propTypes = DataTable.TableCell.propTypes;
+TagValue.propTypes = {
+  row: Proptypes.shape({
+    original: Proptypes.shape({
+      value: Proptypes.string.isRequired,
+      childCount: Proptypes.number.isRequired,
+    }).isRequired,
+  }).isRequired,
+};
 
 const TagListTable = ({ taxonomyId }) => {
   const intl = useIntl();
@@ -62,7 +69,7 @@ const TagListTable = ({ taxonomyId }) => {
   const tagList = useTagListDataResponse(taxonomyId, options);
 
   const fetchData = (args) => {
-    if (!_.isEqual(args, options)) {
+    if (!isEqual(args, options)) {
       setOptions({ ...args });
     }
   };

diff --git a/src/taxonomy/tag-list/data/api.js b/src/taxonomy/tag-list/data/api.js
@@ -18,7 +18,7 @@ const getTagListApiUrl = (taxonomyId, page) => new URL(
  * @param {number} taxonomyId
  * @param {import('./types.mjs').QueryOptions} options
  * @returns {import('@tanstack/react-query').UseQueryResult<import('./types.mjs').TagListData>}
- */ // eslint-disable-next-line import/prefer-default-export
+ */
 export const useTagListData = (taxonomyId, options) => {
   const { pageIndex } = options;
   return useQuery({
@@ -36,7 +36,7 @@ export const useTagListData = (taxonomyId, options) => {
  * 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>}
+ * @returns {import('@tanstack/react-query').UseQueryResult<import('./types.mjs').TagListData>}
  */
 export const useSubTags = (taxonomyId, parentTagValue) => useQuery({
   queryKey: ['subtagsList', taxonomyId, parentTagValue],

diff --git a/src/taxonomy/tag-list/data/types.mjs b/src/taxonomy/tag-list/data/types.mjs
@@ -9,9 +9,8 @@
  * @property {number} pageIndex
  */
 
-// FIXME: this should be renamed to TagData
 /**
- * @typedef {Object} TagListData
+ * @typedef {Object} TagData
  * @property {number} childCount
  * @property {number} depth
  * @property {string} externalId
@@ -23,14 +22,13 @@
  * @property {string?} _id Database ID. Don't rely on this, as it is not present for free-text tags.
  */
 
-// FIXME: this should be renamed to TagListData
 /**
- * @typedef {Object} TagData
+ * @typedef {Object} TagListData
  * @property {number} count
  * @property {number} currentPage
  * @property {string} next
  * @property {number} numPages
  * @property {string} previous
- * @property {TagListData[]} results
+ * @property {TagData[]} results
  * @property {number} start
  */
diff --git a/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx b/src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx
@@ -121,12 +121,19 @@ const TaxonomyMenu = ({
     </>
   );
 
+  const toggleProps = iconMenu ? {
+    as: IconButton,
+    src: MoreVert,
+    iconAs: Icon,
+  } : {
+    as: Button,
+  };
+
   return (
-    <Dropdown onToggle={(_isOpen, ev) => ev.preventDefault()}>
+    <Dropdown id={`taxonomy-menu-${taxonomy.id}`} onToggle={(_isOpen, ev) => ev.preventDefault()}>
       <Dropdown.Toggle
-        as={iconMenu ? IconButton : Button}
-        src={MoreVert}
-        iconAs={Icon}
+        id={`taxonomy-menu-toggle-${taxonomy.id}`}
+        {...toggleProps}
         variant="primary"
         alt={intl.formatMessage(messages.actionsButtonAlt, { name: taxonomy.name })}
         data-testid="taxonomy-menu-button"
@@ -139,6 +146,7 @@ const TaxonomyMenu = ({
           <Dropdown.Item
             key={key}
             data-testid={`taxonomy-menu-${key}`}
+            as="button" // Prevents <a> cannot appear as a descendant of <a> warning
             onClick={
               (e) => {
                 e.preventDefault();