From 96505f2efa84caec988b51fa1476959e7e8a701b Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Sat, 19 Oct 2024 00:38:11 +1100 Subject: [PATCH 1/3] fix: display programs only if the url is configured --- .../LearnerDashboardMenu.jsx | 5 +++-- .../__snapshots__/index.test.jsx.snap | 5 ----- .../LearnerDashboardHeader/hooks.js | 4 +++- .../LearnerDashboardHeader/hooks.test.js | 7 ++++++- .../LearnerDashboardHeader/index.test.jsx | 12 +++++++++++- src/data/services/lms/api.js | 3 +++ src/data/services/lms/urls.js | 2 ++ src/hooks/api.js | 19 +++++++++++++++++++ 8 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/containers/LearnerDashboardHeader/LearnerDashboardMenu.jsx b/src/containers/LearnerDashboardHeader/LearnerDashboardMenu.jsx index 092bcfe7c..054701949 100644 --- a/src/containers/LearnerDashboardHeader/LearnerDashboardMenu.jsx +++ b/src/containers/LearnerDashboardHeader/LearnerDashboardMenu.jsx @@ -9,6 +9,7 @@ const getLearnerHeaderMenu = ( courseSearchUrl, authenticatedUser, exploreCoursesClick, + programsEnabled = false, ) => ({ mainMenu: [ { @@ -17,11 +18,11 @@ const getLearnerHeaderMenu = ( content: formatMessage(messages.course), isActive: true, }, - { + ...(programsEnabled ? [{ type: 'item', href: `${urls.programsUrl()}`, content: formatMessage(messages.program), - }, + }] : []), { type: 'item', href: `${urls.baseAppUrl(courseSearchUrl)}`, diff --git a/src/containers/LearnerDashboardHeader/__snapshots__/index.test.jsx.snap b/src/containers/LearnerDashboardHeader/__snapshots__/index.test.jsx.snap index 80d090019..34bcdc1f9 100644 --- a/src/containers/LearnerDashboardHeader/__snapshots__/index.test.jsx.snap +++ b/src/containers/LearnerDashboardHeader/__snapshots__/index.test.jsx.snap @@ -12,11 +12,6 @@ exports[`LearnerDashboardHeader render 1`] = ` "isActive": true, "type": "item", }, - { - "content": "Programs", - "href": "http://localhost:18000/dashboard/programs", - "type": "item", - }, { "content": "Discover New", "href": "http://localhost:18000/course-search-url", diff --git a/src/containers/LearnerDashboardHeader/hooks.js b/src/containers/LearnerDashboardHeader/hooks.js index 115322c1d..e3eecf8aa 100644 --- a/src/containers/LearnerDashboardHeader/hooks.js +++ b/src/containers/LearnerDashboardHeader/hooks.js @@ -5,6 +5,7 @@ import track from 'tracking'; import { StrictDict } from 'utils'; import { linkNames } from 'tracking/constants'; +import { apiHooks } from 'hooks'; import getLearnerHeaderMenu from './LearnerDashboardMenu'; import * as module from './hooks'; @@ -30,8 +31,9 @@ export const findCoursesNavDropdownClicked = (href) => track.findCourses.findCou export const useLearnerDashboardHeaderMenu = ({ courseSearchUrl, authenticatedUser, exploreCoursesClick, }) => { + const { enabled: programsEnabled } = apiHooks.useProgramsConfig(); const { formatMessage } = useIntl(); - return getLearnerHeaderMenu(formatMessage, courseSearchUrl, authenticatedUser, exploreCoursesClick); + return getLearnerHeaderMenu(formatMessage, courseSearchUrl, authenticatedUser, exploreCoursesClick, programsEnabled); }; export const useLearnerDashboardHeaderData = () => { diff --git a/src/containers/LearnerDashboardHeader/hooks.test.js b/src/containers/LearnerDashboardHeader/hooks.test.js index c9bac080a..e724c4916 100644 --- a/src/containers/LearnerDashboardHeader/hooks.test.js +++ b/src/containers/LearnerDashboardHeader/hooks.test.js @@ -21,6 +21,11 @@ jest.mock('tracking', () => ({ findCoursesClicked: jest.fn(), }, })); +jest.mock('hooks', () => ({ + apiHooks: { + useProgramsConfig: jest.fn(() => ({})), + }, +})); const url = 'http://example.com'; @@ -56,7 +61,7 @@ describe('LearnerDashboardHeader hooks', () => { username: 'test', }; const learnerHomeHeaderMenu = useLearnerDashboardHeaderMenu({ courseSearchUrl, authenticatedUser }); - expect(learnerHomeHeaderMenu.mainMenu.length).toBe(3); + expect(learnerHomeHeaderMenu.mainMenu.length).toBe(2); }); }); diff --git a/src/containers/LearnerDashboardHeader/index.test.jsx b/src/containers/LearnerDashboardHeader/index.test.jsx index 4e552925e..4c455eeaf 100644 --- a/src/containers/LearnerDashboardHeader/index.test.jsx +++ b/src/containers/LearnerDashboardHeader/index.test.jsx @@ -3,6 +3,7 @@ import { shallow } from '@edx/react-unit-test-utils'; import Header from '@edx/frontend-component-header'; import urls from 'data/services/lms/urls'; +import { apiHooks } from 'hooks'; import LearnerDashboardHeader from '.'; import { findCoursesNavClicked } from './hooks'; @@ -12,6 +13,9 @@ jest.mock('hooks', () => ({ courseSearchUrl: '/course-search-url', })), }, + apiHooks: { + useProgramsConfig: jest.fn(() => ({})), + }, })); jest.mock('./hooks', () => ({ ...jest.requireActual('./hooks'), @@ -29,7 +33,7 @@ describe('LearnerDashboardHeader', () => { expect(wrapper.instance.findByType('ConfirmEmailBanner')).toHaveLength(1); expect(wrapper.instance.findByType('MasqueradeBar')).toHaveLength(1); expect(wrapper.instance.findByType(Header)).toHaveLength(1); - wrapper.instance.findByType(Header)[0].props.mainMenuItems[2].onClick(); + wrapper.instance.findByType(Header)[0].props.mainMenuItems[1].onClick(); expect(findCoursesNavClicked).toHaveBeenCalledWith(urls.baseAppUrl('/course-search-url')); expect(wrapper.instance.findByType(Header)[0].props.secondaryMenuItems.length).toBe(0); }); @@ -38,5 +42,11 @@ describe('LearnerDashboardHeader', () => { mergeConfig({ SUPPORT_URL: 'http://localhost:18000/support' }); const wrapper = shallow(); expect(wrapper.instance.findByType(Header)[0].props.secondaryMenuItems.length).toBe(1); + expect(wrapper.instance.findByType(Header)[0].props.mainMenuItems.length).toBe(2); + }); + test('should display Programs link if the service is configured in the backend', () => { + apiHooks.useProgramsConfig.mockReturnValue({ enabled: true }); + const wrapper = shallow(); + expect(wrapper.instance.findByType(Header)[0].props.mainMenuItems.length).toBe(3); }); }); diff --git a/src/data/services/lms/api.js b/src/data/services/lms/api.js index 6f9acd560..915b52a9b 100644 --- a/src/data/services/lms/api.js +++ b/src/data/services/lms/api.js @@ -20,6 +20,8 @@ export const initializeList = ({ user } = {}) => get( stringifyUrl(urls.getInitApiUrl(), { [apiKeys.user]: user }), ); +export const getProgramsConfig = () => get(urls.programsConfigUrl()); + export const updateEntitlementEnrollment = ({ uuid, courseId }) => post( urls.entitlementEnrollment(uuid), { [apiKeys.courseRunId]: courseId }, @@ -73,6 +75,7 @@ export const createCreditRequest = ({ providerId, courseId, username }) => post( export default { initializeList, + getProgramsConfig, unenrollFromCourse, updateEmailSettings, updateEntitlementEnrollment, diff --git a/src/data/services/lms/urls.js b/src/data/services/lms/urls.js index 30c2bbf1b..789d614c3 100644 --- a/src/data/services/lms/urls.js +++ b/src/data/services/lms/urls.js @@ -22,6 +22,7 @@ export const baseAppUrl = (url) => updateUrl(getBaseUrl(), url); export const learningMfeUrl = (url) => updateUrl(getConfig().LEARNING_BASE_URL, url); // static view url +const programsConfigUrl = () => baseAppUrl('/config/programs'); const programsUrl = () => baseAppUrl('/dashboard/programs'); export const creditPurchaseUrl = (courseId) => `${getEcommerceUrl()}/credit/checkout/${courseId}/`; @@ -37,6 +38,7 @@ export default StrictDict({ event, getInitApiUrl, learningMfeUrl, + programsConfigUrl, programsUrl, updateEmailSettings, }); diff --git a/src/hooks/api.js b/src/hooks/api.js index d64a11850..70a448090 100644 --- a/src/hooks/api.js +++ b/src/hooks/api.js @@ -31,6 +31,25 @@ export const useInitializeApp = () => { }); }; +export const useProgramsConfig = () => { + const [config, setConfig] = React.useState({}); + + React.useEffect(() => { + const fetchProgramsConfig = async () => { + try { + const { data } = await api.getProgramsConfig(); + setConfig(data); + } catch (error) { + console.error('Error accessing programs configuration', error); + } + }; + + fetchProgramsConfig(); + }, []); + + return config; +}; + export const useNewEntitlementEnrollment = (cardId) => { const { uuid } = reduxHooks.useCardEntitlementData(cardId); const onSuccess = module.useInitializeApp(); From 4d37fa04d8abff764b81c10ed93dc2443771c9ae Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Fri, 25 Oct 2024 09:29:01 +1100 Subject: [PATCH 2/3] test: add testing for useProgramsn config api hook --- src/hooks/api.test.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/hooks/api.test.js b/src/hooks/api.test.js index cd1a5b6b7..33c781ed5 100644 --- a/src/hooks/api.test.js +++ b/src/hooks/api.test.js @@ -13,6 +13,7 @@ const reduxKeys = keyStore(reduxHooks); jest.mock('data/services/lms/utils', () => ({ post: jest.fn((...args) => ({ post: args })), })); + jest.mock('data/services/lms/api', () => ({ initializeList: jest.fn(), updateEntitlementEnrollment: jest.fn(), @@ -20,7 +21,9 @@ jest.mock('data/services/lms/api', () => ({ deleteEntitlementEnrollment: jest.fn(), updateEmailSettings: jest.fn(), createCreditRequest: jest.fn(), + getProgramsConfig: jest.fn(), })); + jest.mock('data/redux/hooks', () => ({ useCardCourseRunData: jest.fn(), useCardCreditData: jest.fn(), @@ -110,6 +113,31 @@ describe('api hooks', () => { }); }); + describe('useProgramsConfig', () => { + const mockState = {}; + const setState = jest.fn((newState) => { Object.assign(mockState, newState); }); + React.useState.mockReturnValue([mockState, setState]); + it('should return the programs configuration when the API call is successful', async () => { + api.getProgramsConfig.mockResolvedValue({ data: { enabled: true } }); + const config = apiHooks.useProgramsConfig(); + const [cb] = React.useEffect.mock.calls[0]; + await cb(); + expect(setState).toHaveBeenCalled(); + expect(config).toEqual({ enabled: true }); + }); + + it.only('should return an empty object if the api call fails', async () => { + api.getProgramsConfig.mockRejectedValue(new Error('error test')); + const consoleSpy = jest.spyOn(global.console, 'error').mockImplementation(() => { }); + const config = apiHooks.useProgramsConfig(); + const [cb] = React.useEffect.mock.calls[0]; + await cb(); + + expect(config).toEqual({}); + expect(consoleSpy).toHaveBeenCalled(); + }); + }); + describe('entitlement enrollment hooks', () => { beforeEach(() => { jest.spyOn(apiHooks, moduleKeys.useInitializeApp).mockReturnValue(initializeApp); From a9cca145574be15cb8d02e046b7918868b752440 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Tue, 29 Oct 2024 12:16:51 +1100 Subject: [PATCH 3/3] fix: logError and test implementation --- src/hooks/api.js | 3 ++- src/hooks/api.test.js | 20 ++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/hooks/api.js b/src/hooks/api.js index 70a448090..24a844f84 100644 --- a/src/hooks/api.js +++ b/src/hooks/api.js @@ -1,5 +1,6 @@ import React from 'react'; +import { logError } from '@edx/frontend-platform/logging'; import { AppContext } from '@edx/frontend-platform/react'; import { RequestKeys } from 'data/constants/requests'; @@ -40,7 +41,7 @@ export const useProgramsConfig = () => { const { data } = await api.getProgramsConfig(); setConfig(data); } catch (error) { - console.error('Error accessing programs configuration', error); + logError(`Error accessing programs configuration ${error}`); } }; diff --git a/src/hooks/api.test.js b/src/hooks/api.test.js index 33c781ed5..ab18a6dfd 100644 --- a/src/hooks/api.test.js +++ b/src/hooks/api.test.js @@ -1,4 +1,5 @@ import React from 'react'; +import { logError } from '@edx/frontend-platform/logging'; import { AppContext } from '@edx/frontend-platform/react'; import { keyStore } from 'utils'; import { RequestKeys } from 'data/constants/requests'; @@ -10,6 +11,10 @@ import * as apiHooks from './api'; const reduxKeys = keyStore(reduxHooks); +jest.mock('@edx/frontend-platform/logging', () => ({ + logError: jest.fn(), +})); + jest.mock('data/services/lms/utils', () => ({ post: jest.fn((...args) => ({ post: args })), })); @@ -114,9 +119,13 @@ describe('api hooks', () => { }); describe('useProgramsConfig', () => { - const mockState = {}; + let mockState; const setState = jest.fn((newState) => { Object.assign(mockState, newState); }); - React.useState.mockReturnValue([mockState, setState]); + beforeEach(() => { + mockState = {}; + React.useState.mockReturnValue([mockState, setState]); + }); + it('should return the programs configuration when the API call is successful', async () => { api.getProgramsConfig.mockResolvedValue({ data: { enabled: true } }); const config = apiHooks.useProgramsConfig(); @@ -126,15 +135,14 @@ describe('api hooks', () => { expect(config).toEqual({ enabled: true }); }); - it.only('should return an empty object if the api call fails', async () => { + it('should return an empty object if the api call fails', async () => { + mockState = {}; api.getProgramsConfig.mockRejectedValue(new Error('error test')); - const consoleSpy = jest.spyOn(global.console, 'error').mockImplementation(() => { }); const config = apiHooks.useProgramsConfig(); const [cb] = React.useEffect.mock.calls[0]; await cb(); - expect(config).toEqual({}); - expect(consoleSpy).toHaveBeenCalled(); + expect(logError).toHaveBeenCalled(); }); });