From 7ede1197b199d41e178205cc3024f8121ea54177 Mon Sep 17 00:00:00 2001 From: Alexander J Sheehan <67655836+alex-sheehan-edx@users.noreply.github.com> Date: Wed, 21 Jun 2023 14:54:13 -0400 Subject: [PATCH] feat: removing public catalog's dependency on enterprise catalog default results (#320) --- .env.test | 1 + .../CatalogNoResultsDeck.jsx | 62 +++++++++---------- .../CatalogNoResultsDeck.test.jsx | 57 ++++++----------- .../CatalogSearchResults.jsx | 39 +++++++++--- .../CatalogSearchResults.test.jsx | 31 +++------- .../DownloadCsvButton.test.jsx | 4 +- .../services/EnterpriseCatalogAPIService.js | 18 ------ .../EnterpriseCatalogAPIService.test.js | 35 ++++------- 8 files changed, 97 insertions(+), 150 deletions(-) diff --git a/.env.test b/.env.test index 3222c6b2..a268a50f 100644 --- a/.env.test +++ b/.env.test @@ -17,3 +17,4 @@ REFRESH_ACCESS_TOKEN_ENDPOINT='http://localhost:18000/login_refresh' SEGMENT_KEY='' SITE_NAME='edX' USER_INFO_COOKIE_NAME='edx-user-info' +CATALOG_SERVICE_BASE_URL='foobar.com' diff --git a/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.jsx b/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.jsx index 02bd9181..8bb59fd6 100644 --- a/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.jsx +++ b/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.jsx @@ -1,20 +1,19 @@ -import { Alert, CardView, DataTable } from '@edx/paragon'; -import React, { useEffect, useState } from 'react'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { logError } from '@edx/frontend-platform/logging'; +import React, { useMemo } from 'react'; import PropTypes from 'prop-types'; +import { connectStateResults } from 'react-instantsearch-dom'; + +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { Alert, CardView, DataTable } from '@edx/paragon'; import { CONTENT_TYPE_COURSE, CONTENT_TYPE_PROGRAM, EXEC_ED_TITLE, - LEARNING_TYPE_REFINEMENT, NO_RESULTS_DECK_ITEM_COUNT, NO_RESULTS_PAGE_SIZE, NO_RESULTS_PAGE_ITEM_COUNT, } from '../../constants'; import messages from './CatalogNoResultsDeck.messages'; -import EnterpriseCatalogApiService from '../../data/services/EnterpriseCatalogAPIService'; import { getSelectedCatalogFromURL } from '../../utils/common'; const BASE_APP_URL = process.env.BASE_URL; @@ -25,9 +24,12 @@ const CatalogNoResultsDeck = ({ columns, renderCardComponent, contentType, + searchResults, }) => { - const [defaultData, setDefaultData] = useState([]); - const [apiError, setApiError] = useState(false); + const tableData = useMemo( + () => searchResults?.hits || [], + [searchResults?.hits], + ); const selectedCatalog = getSelectedCatalogFromURL(); let redirect; @@ -39,25 +41,6 @@ const CatalogNoResultsDeck = ({ redirect = BASE_APP_URL; } - useEffect(() => { - const defaultCoursesRefinements = { - enterprise_catalog_query_titles: selectedCatalog, - [LEARNING_TYPE_REFINEMENT]: contentType, - }; - - EnterpriseCatalogApiService.fetchDefaultCoursesInCatalogWithFacets( - defaultCoursesRefinements, - ) - .then((response) => { - setDefaultData(response.default_content || []); - setApiError(false); - }) - .catch((err) => { - setApiError(true); - logError(err); - }); - }, [selectedCatalog, contentType]); - let defaultDeckTitle; let alertText; if (contentType === CONTENT_TYPE_COURSE) { @@ -97,11 +80,10 @@ const CatalogNoResultsDeck = ({ )} - {!apiError && ( -

- {defaultDeckTitle} -

- )} + +

+ {defaultDeckTitle} +

{}, columns: [], contentType: '', + searchResults: {}, }; CatalogNoResultsDeck.propTypes = { + searchResults: PropTypes.shape({ + _state: PropTypes.shape({ + disjunctiveFacetsRefinements: PropTypes.shape({}), + }), + disjunctiveFacetsRefinements: PropTypes.arrayOf(PropTypes.shape({})), + nbHits: PropTypes.number, + hits: PropTypes.arrayOf(PropTypes.shape({})), + nbPages: PropTypes.number, + hitsPerPage: PropTypes.number, + page: PropTypes.number, + }), contentType: PropTypes.string, intl: intlShape.isRequired, setCardView: PropTypes.func, @@ -145,4 +139,4 @@ CatalogNoResultsDeck.propTypes = { columns: PropTypes.arrayOf(PropTypes.shape({})), }; -export default injectIntl(CatalogNoResultsDeck); +export default connectStateResults(injectIntl(CatalogNoResultsDeck)); diff --git a/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.test.jsx b/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.test.jsx index cc1d656d..11a91381 100644 --- a/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.test.jsx +++ b/src/components/catalogNoResultsDeck/CatalogNoResultsDeck.test.jsx @@ -1,11 +1,10 @@ -import { IntlProvider } from '@edx/frontend-platform/i18n'; import React from 'react'; -import { render, screen, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; -import { logError } from '@edx/frontend-platform/logging'; +import { render, screen, waitFor } from '@testing-library/react'; + +import { IntlProvider } from '@edx/frontend-platform/i18n'; import CatalogNoResultsDeck from './CatalogNoResultsDeck'; -import EnterpriseCatalogApiService from '../../data/services/EnterpriseCatalogAPIService'; import { getSelectedCatalogFromURL } from '../../utils/common'; const TEST_COURSE_NAME = 'test course'; @@ -16,9 +15,19 @@ const TEST_COURSE_NAME_2 = 'test course 2'; const TEST_PARTNER_2 = 'edx 2'; const TEST_CATALOGS_2 = ['baz', 'ayylmao']; -const csvData = { - default_content: [ - { +// fetching catalog from query params mock +jest.mock('../../utils/common', () => ({ + ...jest.requireActual('../../utils/common'), + getSelectedCatalogFromURL: jest.fn(), +})); + +const defaultProps = { + setCardView: jest.fn(), + columns: [], + renderCardComponent: jest.fn(), + contentType: 'course', + searchResults: { + hits: [{ title: TEST_COURSE_NAME, partners: [{ name: TEST_PARTNER, logo_image_url: '' }], enterprise_catalog_query_titles: TEST_CATALOGS, @@ -35,25 +44,8 @@ const csvData = { first_enrollable_paid_seat_price: 99, original_image_url: '', availability: ['Available Now'], - }, - ], -}; -// Enterprise catalog API mock -const mockCatalogApiService = jest.spyOn( - EnterpriseCatalogApiService, - 'fetchDefaultCoursesInCatalogWithFacets', -); -// fetching catalog from query params mock -jest.mock('../../utils/common', () => ({ - ...jest.requireActual('../../utils/common'), - getSelectedCatalogFromURL: jest.fn(), -})); - -const defaultProps = { - setCardView: jest.fn(), - columns: [], - renderCardComponent: jest.fn(), - contentType: 'course', + }], + }, }; const execEdProps = { @@ -65,7 +57,6 @@ const execEdProps = { describe('catalog no results deck works as expected', () => { test('it displays no results alert text', async () => { - mockCatalogApiService.mockResolvedValue(csvData); render( @@ -90,18 +81,6 @@ describe('catalog no results deck works as expected', () => { ); }); }); - test('API error responses will hide content deck', async () => { - mockCatalogApiService.mockRejectedValue(new Error('Async error')); - render( - - - , - ); - expect( - await screen.findByTestId('noResultsDeckTitleTestId'), - ).not.toBeInTheDocument(); - expect(logError).toBeCalled(); - }); test('shows executive education alert text', async () => { render( diff --git a/src/components/catalogSearchResults/CatalogSearchResults.jsx b/src/components/catalogSearchResults/CatalogSearchResults.jsx index 3215b59d..4fbc9c63 100644 --- a/src/components/catalogSearchResults/CatalogSearchResults.jsx +++ b/src/components/catalogSearchResults/CatalogSearchResults.jsx @@ -20,7 +20,12 @@ import React, { useMemo, useState, } from 'react'; -import { connectStateResults } from 'react-instantsearch-dom'; +import { + Configure, + connectStateResults, + Index, + InstantSearch, +} from 'react-instantsearch-dom'; import Skeleton from 'react-loading-skeleton'; import { CONTENT_TYPE_COURSE, @@ -30,8 +35,10 @@ import { EXEC_ED_TITLE, HIDE_PRICE_REFINEMENT, LEARNING_TYPE_REFINEMENT, + NO_RESULTS_PAGE_SIZE, PROGRAM_TITLE, PROGRAM_TITLE_DESC, + QUERY_TITLE_REFINEMENT, TWOU_EXEC_ED_TITLE_DESC, } from '../../constants'; import { @@ -40,7 +47,7 @@ import { mapAlgoliaObjectToProgram, } from '../../utils/algoliaUtils'; import CatalogInfoModal from '../catalogInfoModal/CatalogInfoModal'; -import { useSelectedCourse } from '../catalogs/data/hooks'; +import { useSelectedCourse, useAlgoliaIndex } from '../catalogs/data/hooks'; import CourseCard from '../courseCard/CourseCard'; import ProgramCard from '../programCard/ProgramCard'; import CatalogBadges from './associatedComponents/catalogBadges/CatalogBadges'; @@ -49,7 +56,7 @@ import DownloadCsvButton from './associatedComponents/downloadCsvButton/Download import messages from './CatalogSearchResults.messages'; import CatalogNoResultsDeck from '../catalogNoResultsDeck/CatalogNoResultsDeck'; -import { formatDate, makePlural } from '../../utils/common'; +import { formatDate, makePlural, getSelectedCatalogFromURL } from '../../utils/common'; export const ERROR_MESSAGE = 'An error occured while retrieving data'; @@ -82,6 +89,7 @@ export const BaseCatalogSearchResults = ({ setNoContent, preview, }) => { + const { algoliaIndexName, searchClient } = useAlgoliaIndex(); const [isProgramType, setIsProgramType] = useState(); const [isCourseType, setIsCourseType] = useState(); const [isExecEdType, setIsExecEdType] = useState(); @@ -455,12 +463,25 @@ export const BaseCatalogSearchResults = ({ )}
{searchResults?.nbHits === 0 && ( - + + + + + + )} {searchResults?.nbHits !== 0 && (
{PAGINATE_ME}
; -const csvData = [{ csv_data: 'foobar' }]; -jest - .spyOn(EnterpriseCatalogApiService, 'fetchDefaultCoursesInCatalogWithFacets') - .mockResolvedValue(csvData); +// all we are testing is routes, we don't need InstantSearch to work here +jest.mock('react-instantsearch-dom', () => ({ + ...jest.requireActual('react-instantsearch-dom'), + InstantSearch: () =>
Popular Courses
, + Index: () =>
Popular Courses
, +})); const DEFAULT_SEARCH_CONTEXT_VALUE = { refinements: {} }; @@ -523,23 +524,8 @@ describe('Main Catalogs view works as expected', () => { ), ).toBeInTheDocument(); }); - test('no program search results displays popular programs text', async () => { - const emptySearchResults = { ...searchResults, nbHits: 0 }; - renderWithRouter( - - - - - , - ); - expect(screen.getByTestId('noResultsAlertTestId')).toBeInTheDocument(); - await act(() => screen.findByText('Popular Programs')); - expect(screen.getByText('Popular Programs')).toBeInTheDocument(); - }); - test('no course search results displays popular programs text', async () => { + + test('no course search results displays popular course text', async () => { const emptySearchResults = { ...searchResults, nbHits: 0 }; renderWithRouter( @@ -551,7 +537,6 @@ describe('Main Catalogs view works as expected', () => { , ); - expect(screen.getByTestId('noResultsAlertTestId')).toBeInTheDocument(); await act(() => screen.findByText('Popular Courses')); expect(screen.getByText('Popular Courses')).toBeInTheDocument(); }); diff --git a/src/components/catalogSearchResults/associatedComponents/downloadCsvButton/DownloadCsvButton.test.jsx b/src/components/catalogSearchResults/associatedComponents/downloadCsvButton/DownloadCsvButton.test.jsx index b3932489..ec72de8f 100644 --- a/src/components/catalogSearchResults/associatedComponents/downloadCsvButton/DownloadCsvButton.test.jsx +++ b/src/components/catalogSearchResults/associatedComponents/downloadCsvButton/DownloadCsvButton.test.jsx @@ -62,9 +62,7 @@ describe('Download button', () => { const input = screen.getByText('Download results'); userEvent.click(input); }); - // The query, query param should not have an `&` in it. - // TODO: figure out why the process env for catalog base service can't be set in the test - const expectedWindowLocation = 'undefined/api/v1/enterprise-catalogs/catalog_workbook?availability=Available' + const expectedWindowLocation = `${process.env.CATALOG_SERVICE_BASE_URL}/api/v1/enterprise-catalogs/catalog_workbook?availability=Available` + '+Now&availability=Upcoming&query=math%20%26%20science'; expect(window.location.href).toEqual(expectedWindowLocation); }); diff --git a/src/data/services/EnterpriseCatalogAPIService.js b/src/data/services/EnterpriseCatalogAPIService.js index 6ffa5898..e053060b 100644 --- a/src/data/services/EnterpriseCatalogAPIService.js +++ b/src/data/services/EnterpriseCatalogAPIService.js @@ -1,11 +1,8 @@ -import { getHttpClient } from '@edx/frontend-platform/auth'; import { createQueryParams } from '../../utils/catalogUtils'; class EnterpriseCatalogApiService { static enterpriseCatalogServiceApiUrl = `${process.env.CATALOG_SERVICE_BASE_URL}/api/v1/enterprise-catalogs`; - static apiClient = getHttpClient; - static generateCsvDownloadLink(options, query) { const facetQuery = query ? `&query=${encodeURIComponent(query)}` : ''; const queryParams = createQueryParams(options); @@ -14,21 +11,6 @@ class EnterpriseCatalogApiService { }/catalog_workbook?${queryParams}${facetQuery}`; return enterpriseListUrl; } - - static fetchDefaultCoursesInCatalog(options) { - const queryParams = new URLSearchParams(options); - const enterpriseListUrl = `${ - EnterpriseCatalogApiService.enterpriseCatalogServiceApiUrl - }/default_course_set?${queryParams.toString()}`; - return EnterpriseCatalogApiService.apiClient().get(enterpriseListUrl); - } - - static fetchDefaultCoursesInCatalogWithFacets(facets) { - return this.fetchDefaultCoursesInCatalog(facets).then((response) => { - const { data } = response; - return data; - }); - } } export default EnterpriseCatalogApiService; diff --git a/src/data/services/EnterpriseCatalogAPIService.test.js b/src/data/services/EnterpriseCatalogAPIService.test.js index 76f21259..31f8e974 100644 --- a/src/data/services/EnterpriseCatalogAPIService.test.js +++ b/src/data/services/EnterpriseCatalogAPIService.test.js @@ -1,30 +1,17 @@ import '@testing-library/jest-dom/extend-expect'; -import { getHttpClient } from '@edx/frontend-platform/auth'; import EnterpriseCatalogApiService from './EnterpriseCatalogAPIService'; -jest.mock('@edx/frontend-platform/auth'); - -const mockApiClient = { - get: jest.fn().mockResolvedValue({ value: 'foobar' }), -}; -getHttpClient.mockReturnValue(mockApiClient); - -const mockDefaultCourses = jest.spyOn( - EnterpriseCatalogApiService, - 'fetchDefaultCoursesInCatalog', -); - -describe('fetchDefaultCoursesInCatalogWithFacets', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - it('requests the catalog API with fetchDefaultCoursesInCatalog', () => { - const facets = { content_type: 'course' }; - EnterpriseCatalogApiService.fetchDefaultCoursesInCatalogWithFacets(facets); - expect(mockDefaultCourses).toBeCalledWith(facets); - // An issue with process.env is making it impossible to assert the value the method is called with - // So for now simply assert the method's been called - expect(mockApiClient.get).toBeCalled(); +describe('generateCsvDownloadLink', () => { + test('correctly formats csv download link', () => { + const options = { enterprise_catalog_query_titles: 'test' }; + const query = 'somequery'; + const generatedDownloadLink = EnterpriseCatalogApiService.generateCsvDownloadLink( + options, + query, + ); + expect(generatedDownloadLink).toEqual( + `${process.env.CATALOG_SERVICE_BASE_URL}/api/v1/enterprise-catalogs/catalog_workbook?enterprise_catalog_query_titles=test&query=${query}`, + ); }); });