From 9a6e12bd3b058b54e959d4a1f2fd1d97ca696b68 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 19 Mar 2024 21:01:10 -0700 Subject: [PATCH] Clean up Taxonomy API files/hooks/queries [FC-0036] (#850) * chore: rename apiHooks.jsx to apihooks.js * refactor: consolidate taxonomy API code * fix: was not invalidating tags after import * fix: UI was freezing while computing plan for large import files --- src/content-tags-drawer/ContentTagsDrawer.jsx | 19 +- .../ContentTagsDrawer.test.jsx | 46 ++-- src/taxonomy/TaxonomyListPage.jsx | 20 +- src/taxonomy/TaxonomyListPage.test.jsx | 111 ++++++---- src/taxonomy/data/api.js | 134 ++++++++---- src/taxonomy/data/api.test.js | 43 ++-- src/taxonomy/data/apiHooks.js | 206 ++++++++++++++++++ src/taxonomy/data/apiHooks.jsx | 106 --------- src/taxonomy/data/apiHooks.test.jsx | 164 ++++++++------ src/taxonomy/data/types.mjs | 5 +- src/taxonomy/export-modal/index.jsx | 2 +- src/taxonomy/import-tags/ImportTagsWizard.jsx | 74 +++---- .../import-tags/ImportTagsWizard.test.jsx | 111 +++++----- src/taxonomy/import-tags/__mocks__/index.js | 1 - .../__mocks__/taxonomyImportMock.js | 5 - src/taxonomy/import-tags/data/api.js | 114 ---------- src/taxonomy/import-tags/data/api.test.jsx | 88 -------- src/taxonomy/import-tags/index.js | 2 +- src/taxonomy/import-tags/{data => }/utils.js | 41 ++-- src/taxonomy/manage-orgs/ManageOrgsModal.jsx | 4 +- src/taxonomy/tag-list/TagListTable.jsx | 6 +- src/taxonomy/tag-list/data/api.js | 52 ----- src/taxonomy/tag-list/data/api.test.js | 27 --- src/taxonomy/tag-list/data/apiHooks.js | 41 ++++ src/taxonomy/tag-list/data/apiHooks.jsx | 41 ---- src/taxonomy/tag-list/data/apiHooks.test.jsx | 45 ---- .../taxonomy-detail/TaxonomyDetailPage.jsx | 9 +- .../TaxonomyDetailPage.test.jsx | 12 +- 28 files changed, 703 insertions(+), 826 deletions(-) create mode 100644 src/taxonomy/data/apiHooks.js delete mode 100644 src/taxonomy/data/apiHooks.jsx delete mode 100644 src/taxonomy/import-tags/__mocks__/taxonomyImportMock.js delete mode 100644 src/taxonomy/import-tags/data/api.js delete mode 100644 src/taxonomy/import-tags/data/api.test.jsx rename src/taxonomy/import-tags/{data => }/utils.js (76%) delete mode 100644 src/taxonomy/tag-list/data/api.js delete mode 100644 src/taxonomy/tag-list/data/api.test.js create mode 100644 src/taxonomy/tag-list/data/apiHooks.js delete mode 100644 src/taxonomy/tag-list/data/apiHooks.jsx delete mode 100644 src/taxonomy/tag-list/data/apiHooks.test.jsx diff --git a/src/content-tags-drawer/ContentTagsDrawer.jsx b/src/content-tags-drawer/ContentTagsDrawer.jsx index d0f987b12..72e41c6fd 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.jsx @@ -20,7 +20,7 @@ import { useContentTaxonomyTagsData, useContentData, } from './data/apiHooks'; -import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from '../taxonomy/data/apiHooks'; +import { useTaxonomyList } from '../taxonomy/data/apiHooks'; import Loading from '../generic/Loading'; /** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ @@ -37,14 +37,9 @@ import Loading from '../generic/Loading'; */ const ContentTagsDrawer = ({ id, onClose }) => { const intl = useIntl(); - // TODO: We can delete this when the iframe is no longer used on edx-platform + // TODO: We can delete 'params' when the iframe is no longer used on edx-platform const params = useParams(); - let contentId = id; - - if (contentId === undefined) { - // TODO: We can delete this when the iframe is no longer used on edx-platform - contentId = params.contentId; - } + const contentId = id ?? params.contentId; const org = extractOrgFromContentId(contentId); @@ -74,18 +69,12 @@ const ContentTagsDrawer = ({ id, onClose }) => { setStagedContentTags(prevStagedContentTags => ({ ...prevStagedContentTags, [taxonomyId]: tagsList })); }, [setStagedContentTags]); - const useTaxonomyListData = () => { - const taxonomyListData = useTaxonomyListDataResponse(org); - const isTaxonomyListLoaded = useIsTaxonomyListDataLoaded(org); - return { taxonomyListData, isTaxonomyListLoaded }; - }; - const { data: contentData, isSuccess: isContentDataLoaded } = useContentData(contentId); const { data: contentTaxonomyTagsData, isSuccess: isContentTaxonomyTagsLoaded, } = useContentTaxonomyTagsData(contentId); - const { taxonomyListData, isTaxonomyListLoaded } = useTaxonomyListData(); + const { data: taxonomyListData, isSuccess: isTaxonomyListLoaded } = useTaxonomyList(org); let onCloseDrawer = onClose; if (onCloseDrawer === undefined) { diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 37fd2343f..8ff9d52f2 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -1,7 +1,12 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { - act, render, fireEvent, screen, + act, + fireEvent, + render, + waitFor, + screen, } from '@testing-library/react'; import ContentTagsDrawer from './ContentTagsDrawer'; @@ -10,7 +15,7 @@ import { useContentData, useTaxonomyTagsData, } from './data/apiHooks'; -import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from '../taxonomy/data/apiHooks'; +import { getTaxonomyListData } from '../taxonomy/data/api'; import messages from './messages'; const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@7f47fe2dbcaf47c5a071671c741fe1ab'; @@ -23,6 +28,7 @@ jest.mock('react-router-dom', () => ({ }), })); +// FIXME: replace these mocks with API mocks jest.mock('./data/apiHooks', () => ({ useContentTaxonomyTagsData: jest.fn(() => ({ isSuccess: false, @@ -46,20 +52,30 @@ jest.mock('./data/apiHooks', () => ({ })), })); -jest.mock('../taxonomy/data/apiHooks', () => ({ - useTaxonomyListDataResponse: jest.fn(), - useIsTaxonomyListDataLoaded: jest.fn(), +jest.mock('../taxonomy/data/api', () => ({ + // By default, the mock taxonomy list will never load (promise never resolves): + getTaxonomyListData: jest.fn(), })); +const queryClient = new QueryClient(); + const RootWrapper = (params) => ( - + + + ); describe('', () => { + beforeEach(async () => { + await queryClient.resetQueries(); + // By default, we mock the API call with a promise that never resolves. + // You can override this in specific test. + getTaxonomyListData.mockReturnValue(new Promise(() => {})); + }); + const setupMockDataForStagedTagsTesting = () => { - useIsTaxonomyListDataLoaded.mockReturnValue(true); useContentTaxonomyTagsData.mockReturnValue({ isSuccess: true, data: { @@ -84,7 +100,7 @@ describe('', () => { ], }, }); - useTaxonomyListDataResponse.mockReturnValue({ + getTaxonomyListData.mockResolvedValue({ results: [{ id: 123, name: 'Taxonomy 1', @@ -148,7 +164,6 @@ describe('', () => { }); it('shows spinner before the taxonomy tags query is complete', async () => { - useIsTaxonomyListDataLoaded.mockReturnValue(false); await act(async () => { const { getAllByRole } = render(); const spinner = getAllByRole('status')[1]; @@ -181,7 +196,6 @@ describe('', () => { }); it('shows the taxonomies data including tag numbers after the query is complete', async () => { - useIsTaxonomyListDataLoaded.mockReturnValue(true); useContentTaxonomyTagsData.mockReturnValue({ isSuccess: true, data: { @@ -218,7 +232,7 @@ describe('', () => { ], }, }); - useTaxonomyListDataResponse.mockReturnValue({ + getTaxonomyListData.mockResolvedValue({ results: [{ id: 123, name: 'Taxonomy 1', @@ -233,6 +247,7 @@ describe('', () => { }); await act(async () => { const { container, getByText } = render(); + await waitFor(() => { expect(getByText('Taxonomy 1')).toBeInTheDocument(); }); expect(getByText('Taxonomy 1')).toBeInTheDocument(); expect(getByText('Taxonomy 2')).toBeInTheDocument(); const tagCountBadges = container.getElementsByClassName('badge'); @@ -241,10 +256,11 @@ describe('', () => { }); }); - it('should test adding a content tag to the staged tags for a taxonomy', () => { + it('should test adding a content tag to the staged tags for a taxonomy', async () => { setupMockDataForStagedTagsTesting(); const { container, getByText, getAllByText } = render(); + await waitFor(() => { expect(getByText('Taxonomy 1')).toBeInTheDocument(); }); // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; @@ -267,10 +283,11 @@ describe('', () => { expect(getAllByText('Tag 3').length).toBe(2); }); - it('should test removing a staged content from a taxonomy', () => { + it('should test removing a staged content from a taxonomy', async () => { setupMockDataForStagedTagsTesting(); const { container, getByText, getAllByText } = render(); + await waitFor(() => { expect(getByText('Taxonomy 1')).toBeInTheDocument(); }); // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; @@ -297,7 +314,7 @@ describe('', () => { expect(getAllByText('Tag 3').length).toBe(1); }); - it('should test clearing staged tags for a taxonomy', () => { + it('should test clearing staged tags for a taxonomy', async () => { setupMockDataForStagedTagsTesting(); const { @@ -306,6 +323,7 @@ describe('', () => { getAllByText, queryByText, } = render(); + await waitFor(() => { expect(getByText('Taxonomy 1')).toBeInTheDocument(); }); // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; diff --git a/src/taxonomy/TaxonomyListPage.jsx b/src/taxonomy/TaxonomyListPage.jsx index d6139c7e2..4ad78fadb 100644 --- a/src/taxonomy/TaxonomyListPage.jsx +++ b/src/taxonomy/TaxonomyListPage.jsx @@ -24,17 +24,15 @@ import { Helmet } from 'react-helmet'; import { useOrganizationListData } from '../generic/data/apiHooks'; import SubHeader from '../generic/sub-header/SubHeader'; import getPageHeadTitle from '../generic/utils'; -import { getTaxonomyTemplateApiUrl } from './data/api'; -import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from './data/apiHooks'; +import { ALL_TAXONOMIES, apiUrls, UNASSIGNED } from './data/api'; +import { useImportNewTaxonomy, useTaxonomyList } from './data/apiHooks'; import { importTaxonomy } from './import-tags'; import messages from './messages'; import TaxonomyCard from './taxonomy-card'; -const ALL_TAXONOMIES = 'All taxonomies'; -const UNASSIGNED = 'Unassigned'; - const TaxonomyListHeaderButtons = ({ canAddTaxonomy }) => { const intl = useIntl(); + const importMutation = useImportNewTaxonomy(); return ( <> { {intl.formatMessage(messages.downloadTemplateButtonCSVLabel)} {intl.formatMessage(messages.downloadTemplateButtonJSONLabel)} @@ -71,7 +69,7 @@ const TaxonomyListHeaderButtons = ({ canAddTaxonomy }) => {