From 87d508cfcdcac581373eafb216d3e699da01259f Mon Sep 17 00:00:00 2001
From: Christofer <christofer@opencraft.com>
Date: Mon, 11 Dec 2023 22:11:53 +0000
Subject: [PATCH] refactor: Fix typing nits and refactor delete mutation

---
 src/content-tags-drawer/data/apiHooks.jsx     |  6 +--
 src/content-tags-drawer/data/types.mjs        |  5 ---
 src/taxonomy/TaxonomyLayout.test.jsx          |  2 +-
 src/taxonomy/TaxonomyListPage.jsx             | 29 ++++++---------
 src/taxonomy/TaxonomyListPage.test.jsx        | 37 ++++++++-----------
 src/taxonomy/data/apiHooks.jsx                | 17 +++------
 src/taxonomy/data/apiHooks.test.jsx           |  2 +-
 src/taxonomy/data/types.mjs                   | 16 --------
 .../taxonomy-detail/TaxonomyDetailPage.jsx    | 27 +++++++-------
 .../TaxonomyDetailPage.test.jsx               | 34 +++++------------
 10 files changed, 60 insertions(+), 115 deletions(-)

diff --git a/src/content-tags-drawer/data/apiHooks.jsx b/src/content-tags-drawer/data/apiHooks.jsx
index 099c88129d..d30c3db007 100644
--- a/src/content-tags-drawer/data/apiHooks.jsx
+++ b/src/content-tags-drawer/data/apiHooks.jsx
@@ -7,7 +7,7 @@ import { getTaxonomyTagsData, getContentTaxonomyTagsData, getContentData } from
  * @param {string} taxonomyId The id of the taxonomy to fetch tags for
  * @param {string} fullPathProvided Optional param that contains the full URL to fetch data
  *                 If provided, we use it instead of generating the URL. This is usually for fetching subTags
- * @returns {import("./types.mjs").UseQueryResult}
+ * @returns {import('@tanstack/react-query').UseQueryResult<import("./types.mjs").TaxonomyTagsData>}
  */
 const useTaxonomyTagsData = (taxonomyId, fullPathProvided) => (
   useQuery({
@@ -45,7 +45,7 @@ export const useIsTaxonomyTagsDataLoaded = (taxonomyId, fullPathProvided) => (
 /**
  * Builds the query to get the taxonomy tags applied to the content object
  * @param {string} contentId The id of the content object to fetch the applied tags for
- * @returns {import("./types.mjs").UseQueryResult}
+ * @returns {import('@tanstack/react-query').UseQueryResult<import("./types.mjs").ContentTaxonomyTagsData>}
  */
 const useContentTaxonomyTagsData = (contentId) => (
   useQuery({
@@ -79,7 +79,7 @@ export const useIsContentTaxonomyTagsDataLoaded = (contentId) => (
 /**
  * Builds the query to get meta data about the content object
  * @param {string} contentId The id of the content object (unit/component)
- * @returns {import("./types.mjs").UseQueryResult}
+ * @returns {import('@tanstack/react-query').UseQueryResult<import("./types.mjs").ContentData>}
  */
 const useContentData = (contentId) => (
   useQuery({
diff --git a/src/content-tags-drawer/data/types.mjs b/src/content-tags-drawer/data/types.mjs
index 00b3fefd4c..c16cb45ea6 100644
--- a/src/content-tags-drawer/data/types.mjs
+++ b/src/content-tags-drawer/data/types.mjs
@@ -100,8 +100,3 @@
  * @property {TaxonomyTagData[]} results
  */
 
-/**
- * @typedef {Object} UseQueryResult
- * @property {Object} data
- * @property {string} status
- */
diff --git a/src/taxonomy/TaxonomyLayout.test.jsx b/src/taxonomy/TaxonomyLayout.test.jsx
index e11e85b02a..d833809997 100644
--- a/src/taxonomy/TaxonomyLayout.test.jsx
+++ b/src/taxonomy/TaxonomyLayout.test.jsx
@@ -55,7 +55,7 @@ describe('<TaxonomyLayout />', async () => {
     expect(getByTestId('mock-footer')).toBeInTheDocument();
   });
 
-  it('shoul show toast', async () => {
+  it('should show toast', async () => {
     const { getByTestId, getByText } = render(<RootWrapper />);
     act(() => {
       expect(getByTestId('taxonomy-toast')).toBeInTheDocument();
diff --git a/src/taxonomy/TaxonomyListPage.jsx b/src/taxonomy/TaxonomyListPage.jsx
index a9f729ac2d..6f23b90d8d 100644
--- a/src/taxonomy/TaxonomyListPage.jsx
+++ b/src/taxonomy/TaxonomyListPage.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect, useContext } from 'react';
+import React, { useContext } from 'react';
 import {
   CardView,
   Container,
@@ -17,14 +17,19 @@ import { TaxonomyContext } from './common/context';
 
 const TaxonomyListPage = () => {
   const intl = useIntl();
-  const [deletedTaxonomyName, setDeletedTaxonomyName] = useState('');
-  const deleteMutation = useDeleteTaxonomy();
+  const deleteTaxonomy = useDeleteTaxonomy();
   const { setToastMessage } = useContext(TaxonomyContext);
 
-  const onDeleteTaxonomy = (id, name) => {
-    setDeletedTaxonomyName(name);
-    deleteMutation.mutate({ pk: id });
-  };
+  const onDeleteTaxonomy = React.useCallback((id, name) => {
+    deleteTaxonomy({ pk: id }, {
+      onSuccess: async () => {
+        setToastMessage(intl.formatMessage(messages.taxonomyDeleteToast, { name }));
+      },
+      onError: async () => {
+        // TODO: display the error to the user
+      },
+    });
+  }, [setToastMessage]);
 
   const useTaxonomyListData = () => {
     const taxonomyListData = useTaxonomyListDataResponse();
@@ -33,16 +38,6 @@ const TaxonomyListPage = () => {
   };
   const { taxonomyListData, isLoaded } = useTaxonomyListData();
 
-  // Verifies the status after run the delete mutation.
-  // Shows the toast on success or error.
-  useEffect(() => {
-    if (deleteMutation.status === 'success') {
-      setToastMessage(intl.formatMessage(messages.taxonomyDeleteToast, { name: deletedTaxonomyName }));
-    } else if (deleteMutation.status === 'error') {
-      setToastMessage(deleteMutation.error.message);
-    }
-  }, [deleteMutation.status]);
-
   const getHeaderButtons = () => (
     // Download template and import buttons.
     // TODO Add functionality to this buttons.
diff --git a/src/taxonomy/TaxonomyListPage.test.jsx b/src/taxonomy/TaxonomyListPage.test.jsx
index 1d61d3f8b2..17c95034e2 100644
--- a/src/taxonomy/TaxonomyListPage.test.jsx
+++ b/src/taxonomy/TaxonomyListPage.test.jsx
@@ -2,16 +2,17 @@ import React, { useMemo } from 'react';
 import { IntlProvider, injectIntl } from '@edx/frontend-platform/i18n';
 import { initializeMockApp } from '@edx/frontend-platform';
 import { AppProvider } from '@edx/frontend-platform/react';
-import { act, render } from '@testing-library/react';
+import { act, render, fireEvent } from '@testing-library/react';
 
 import initializeStore from '../store';
 
 import TaxonomyListPage from './TaxonomyListPage';
-import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded, useDeleteTaxonomy } from './data/apiHooks';
+import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from './data/apiHooks';
 import { TaxonomyContext } from './common/context';
 
 let store;
 const mockSetToastMessage = jest.fn();
+const mockDeleteTaxonomy = jest.fn();
 const taxonomies = [{
   id: 1,
   name: 'Taxonomy',
@@ -21,8 +22,12 @@ const taxonomies = [{
 jest.mock('./data/apiHooks', () => ({
   useTaxonomyListDataResponse: jest.fn(),
   useIsTaxonomyListDataLoaded: jest.fn(),
-  useDeleteTaxonomy: jest.fn(),
+  useDeleteTaxonomy: () => mockDeleteTaxonomy,
 }));
+jest.mock('./taxonomy-card/TaxonomyCardMenu', () => jest.fn(({ onClickMenuItem }) => (
+  // eslint-disable-next-line jsx-a11y/control-has-associated-label
+  <button type="button" data-testid="test-delete-button" onClick={() => onClickMenuItem('delete')} />
+)));
 
 const RootWrapper = () => {
   const context = useMemo(() => ({
@@ -41,8 +46,6 @@ const RootWrapper = () => {
   );
 };
 
-useDeleteTaxonomy.mockReturnValue({ status: 'idle' });
-
 describe('<TaxonomyListPage />', async () => {
   beforeEach(async () => {
     initializeMockApp({
@@ -54,7 +57,6 @@ describe('<TaxonomyListPage />', async () => {
       },
     });
     store = initializeStore();
-    useDeleteTaxonomy.mockClear();
   });
 
   it('should render page and page title correctly', () => {
@@ -87,22 +89,15 @@ describe('<TaxonomyListPage />', async () => {
     useTaxonomyListDataResponse.mockReturnValue({
       results: taxonomies,
     });
-    useDeleteTaxonomy.mockReturnValue({ status: 'success' });
-    render(<RootWrapper />);
-    await act(async () => {
-      expect(mockSetToastMessage).toBeCalledWith('"" deleted');
+    mockDeleteTaxonomy.mockImplementationOnce(async (params, callbacks) => {
+      callbacks.onSuccess();
     });
-  });
+    const { getByTestId, getByLabelText } = render(<RootWrapper />);
+    fireEvent.click(getByTestId('test-delete-button'));
+    fireEvent.change(getByLabelText('Type DELETE to confirm'), { target: { value: 'DELETE' } });
+    fireEvent.click(getByTestId('delete-button'));
 
-  it('should show the error toast after delete', async () => {
-    useIsTaxonomyListDataLoaded.mockReturnValue(true);
-    useTaxonomyListDataResponse.mockReturnValue({
-      results: taxonomies,
-    });
-    useDeleteTaxonomy.mockReturnValue({ status: 'error', error: { message: 'Error' } });
-    render(<RootWrapper />);
-    await act(async () => {
-      expect(mockSetToastMessage).toBeCalledWith('Error');
-    });
+    expect(mockDeleteTaxonomy).toBeCalledTimes(1);
+    expect(mockSetToastMessage).toBeCalledWith(`"${taxonomies[0].name}" deleted`);
   });
 });
diff --git a/src/taxonomy/data/apiHooks.jsx b/src/taxonomy/data/apiHooks.jsx
index fd62e8564c..21e522b0e9 100644
--- a/src/taxonomy/data/apiHooks.jsx
+++ b/src/taxonomy/data/apiHooks.jsx
@@ -17,7 +17,7 @@ import { getTaxonomyListData, deleteTaxonomy } from './api';
 /**
  * Builds the query to get the taxonomy list
  * @param {string} org Optional organization query param
- * @returns {import("./types.mjs").UseQueryResult}
+ * @returns {import('@tanstack/react-query').UseQueryResult<import("./types.mjs").TaxonomyListData>}
  */
 const useTaxonomyListData = (org) => (
   useQuery({
@@ -28,25 +28,18 @@ const useTaxonomyListData = (org) => (
 
 /**
  * Builds the mutation to delete a taxonomy.
- * @returns {Object} - An object with the mutation configuration.
+ * @returns An object with the mutation configuration.
  */
 export const useDeleteTaxonomy = () => {
   const queryClient = useQueryClient();
-  return useMutation({
-    /**
-     * @type {import("@tanstack/react-query").MutateFunction<
-    *   any,
-    *   any,
-    *   {
-    *     pk: number
-    *   }
-    * >}
-    */
+  const { mutate } = useMutation({
+    /** @type {import("@tanstack/react-query").MutateFunction<any, any, {pk: number}>} */
     mutationFn: async ({ pk }) => deleteTaxonomy(pk),
     onSettled: () => {
       queryClient.invalidateQueries({ queryKey: ['taxonomyList'] });
     },
   });
+  return mutate;
 };
 
 /**
diff --git a/src/taxonomy/data/apiHooks.test.jsx b/src/taxonomy/data/apiHooks.test.jsx
index 539c787d76..d865aa4f13 100644
--- a/src/taxonomy/data/apiHooks.test.jsx
+++ b/src/taxonomy/data/apiHooks.test.jsx
@@ -58,7 +58,7 @@ describe('useDeleteTaxonomy', () => {
     useMutation.mockReturnValueOnce({ mutate: jest.fn() });
 
     const mutation = useDeleteTaxonomy();
-    mutation.mutate();
+    mutation();
 
     expect(useMutation).toBeCalled();
 
diff --git a/src/taxonomy/data/types.mjs b/src/taxonomy/data/types.mjs
index fba3a5e387..e2c1026fd8 100644
--- a/src/taxonomy/data/types.mjs
+++ b/src/taxonomy/data/types.mjs
@@ -23,19 +23,3 @@
  * @property {TaxonomyData[]} results
  */
 
-/**
- * @typedef {Object} QueryTaxonomyListData
- * @property {TaxonomyListData} data
- */
-
-/**
- * @typedef {Object} UseQueryResult
- * @property {Object} data
- * @property {string} status
- * @property {function} refetch
- */
-
-/**
- * @typedef {Object} DeleteTaxonomyParams
- * @property {number} pk - The primary key of the taxonomy.
- */
diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx
index 65ffc1b1ab..45ebc7ef44 100644
--- a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx
+++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx
@@ -1,5 +1,4 @@
-// ts-check
-import React, { useContext, useEffect, useState } from 'react';
+import React, { useContext, useState } from 'react';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import {
   Breadcrumb,
@@ -35,14 +34,19 @@ const TaxonomyDetailPage = () => {
 
   const [isExportModalOpen, setIsExportModalOpen] = useState(false);
   const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false);
-  const deleteMutation = useDeleteTaxonomy();
+  const deleteTaxonomy = useDeleteTaxonomy();
 
-  useEffect(() => {
-    if (deleteMutation.status === 'success') {
-      setToastMessage(intl.formatMessage(taxonomyMessages.taxonomyDeleteToast, { name: taxonomy.name }));
-      navigate('/taxonomies');
-    }
-  }, [deleteMutation.status]);
+  const onClickDeleteTaxonomy = React.useCallback(() => {
+    deleteTaxonomy({ pk: taxonomy.id }, {
+      onSuccess: async () => {
+        setToastMessage(intl.formatMessage(taxonomyMessages.taxonomyDeleteToast, { name: taxonomy.name }));
+        navigate('/taxonomies');
+      },
+      onError: async () => {
+        // TODO: display the error to the user
+      },
+    });
+  }, [setToastMessage, taxonomy]);
 
   const menuItems = ['export', 'delete'];
   const systemDefinedMenuItems = ['export'];
@@ -63,16 +67,11 @@ const TaxonomyDetailPage = () => {
     );
   }
 
-  const onClickDeleteTaxonomy = () => {
-    deleteMutation.mutate({ pk: taxonomy.id });
-  };
-
   const renderModals = () => isExportModalOpen && (
     <ExportModal
       isOpen={isExportModalOpen}
       onClose={() => setIsExportModalOpen(false)}
       taxonomyId={taxonomy.id}
-      taxonomyName={taxonomy.name}
     />
   );
 
diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx
index 262ec5a4be..bd95a7500e 100644
--- a/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx
+++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailPage.test.jsx
@@ -2,12 +2,11 @@ import React, { useMemo } from 'react';
 import { initializeMockApp } from '@edx/frontend-platform';
 import { IntlProvider } from '@edx/frontend-platform/i18n';
 import { AppProvider } from '@edx/frontend-platform/react';
-import { fireEvent, render, act } from '@testing-library/react';
+import { fireEvent, render } from '@testing-library/react';
 
 import { useTaxonomyDetailData } from './data/api';
 import initializeStore from '../../store';
 import TaxonomyDetailPage from './TaxonomyDetailPage';
-import { useDeleteTaxonomy } from '../data/apiHooks';
 import { TaxonomyContext } from '../common/context';
 
 let store;
@@ -26,9 +25,8 @@ jest.mock('react-router-dom', () => ({
   useNavigate: () => mockNavigate,
 }));
 jest.mock('../data/apiHooks', () => ({
-  useDeleteTaxonomy: jest.fn(),
+  useDeleteTaxonomy: () => mockMutate,
 }));
-useDeleteTaxonomy.mockReturnValue({ status: 'idle', mutate: mockMutate });
 
 jest.mock('./TaxonomyDetailSideCard', () => jest.fn(() => <>Mock TaxonomyDetailSideCard</>));
 jest.mock('../tag-list/TagListTable', () => jest.fn(() => <>Mock TagListTable</>));
@@ -174,6 +172,9 @@ describe('<TaxonomyDetailPage />', async () => {
         description: 'This is a description',
       },
     });
+    mockMutate.mockImplementationOnce(async (params, callbacks) => {
+      callbacks.onSuccess();
+    });
 
     const {
       getByRole, getByText, getByLabelText, getByTestId,
@@ -197,27 +198,10 @@ describe('<TaxonomyDetailPage />', async () => {
 
     // Modal closed
     expect(() => getByText(`Delete "${taxonomyName}"`)).toThrow();
-    expect(mockMutate).toBeCalledWith({ pk: 1 });
-  });
-
-  it('should redirect after a success delete', async () => {
-    const taxonomyName = 'Test taxonomy';
+    expect(mockMutate).toBeCalledTimes(1);
 
-    useTaxonomyDetailData.mockReturnValue({
-      isSuccess: true,
-      isFetched: true,
-      isError: false,
-      data: {
-        id: 1,
-        name: taxonomyName,
-        description: 'This is a description',
-      },
-    });
-    useDeleteTaxonomy.mockReturnValue({ status: 'success' });
-    render(<RootWrapper />);
-    await act(async () => {
-      expect(mockSetToastMessage).toBeCalled();
-      expect(mockNavigate).toBeCalledWith('/taxonomies');
-    });
+    // Should redirect after a success delete
+    expect(mockSetToastMessage).toBeCalledTimes(1);
+    expect(mockNavigate).toBeCalledWith('/taxonomies');
   });
 });