From 0e88e33f1e08e82b76103f71e56ab5d9c3a44c69 Mon Sep 17 00:00:00 2001 From: MaxFrank13 Date: Thu, 9 Oct 2025 17:27:20 +0000 Subject: [PATCH 1/3] feat!: remove NoticesWrapper --- src/components/NoticesWrapper/api.js | 25 ----- src/components/NoticesWrapper/api.test.js | 65 ------------- src/components/NoticesWrapper/hooks.js | 40 -------- src/components/NoticesWrapper/hooks.test.js | 99 -------------------- src/components/NoticesWrapper/index.jsx | 25 ----- src/components/NoticesWrapper/index.test.jsx | 36 ------- src/components/NoticesWrapper/messages.js | 11 --- 7 files changed, 301 deletions(-) delete mode 100644 src/components/NoticesWrapper/api.js delete mode 100644 src/components/NoticesWrapper/api.test.js delete mode 100644 src/components/NoticesWrapper/hooks.js delete mode 100644 src/components/NoticesWrapper/hooks.test.js delete mode 100644 src/components/NoticesWrapper/index.jsx delete mode 100644 src/components/NoticesWrapper/index.test.jsx delete mode 100644 src/components/NoticesWrapper/messages.js diff --git a/src/components/NoticesWrapper/api.js b/src/components/NoticesWrapper/api.js deleted file mode 100644 index 72284243d..000000000 --- a/src/components/NoticesWrapper/api.js +++ /dev/null @@ -1,25 +0,0 @@ -import { getConfig } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient, getAuthenticatedUser } from '@edx/frontend-platform/auth'; -import { logError, logInfo } from '@edx/frontend-platform/logging'; - -export const noticesUrl = `${getConfig().LMS_BASE_URL}/notices/api/v1/unacknowledged`; - -export const getNotices = ({ onLoad, notFoundMessage }) => { - const authenticatedUser = getAuthenticatedUser(); - - const handleError = async (e) => { - // Error probably means that notices is not installed, which is fine. - const { customAttributes: { httpErrorStatus } } = e; - if (httpErrorStatus === 404) { - logInfo(`${e}. ${notFoundMessage}`); - } else { - logError(e); - } - }; - if (authenticatedUser) { - return getAuthenticatedHttpClient().get(noticesUrl, {}).then(onLoad).catch(handleError); - } - return null; -}; - -export default { getNotices }; diff --git a/src/components/NoticesWrapper/api.test.js b/src/components/NoticesWrapper/api.test.js deleted file mode 100644 index 4470f6ece..000000000 --- a/src/components/NoticesWrapper/api.test.js +++ /dev/null @@ -1,65 +0,0 @@ -import { getAuthenticatedHttpClient, getAuthenticatedUser } from '@edx/frontend-platform/auth'; -import { logError, logInfo } from '@edx/frontend-platform/logging'; - -import * as api from './api'; - -jest.mock('@edx/frontend-platform', () => ({ - getConfig: jest.fn(() => ({ - LMS_BASE_URL: 'test-lms-url', - })), -})); - -jest.mock('@edx/frontend-platform/auth', () => ({ - getAuthenticatedHttpClient: jest.fn(), - getAuthenticatedUser: jest.fn(), -})); - -jest.mock('@edx/frontend-platform/logging', () => ({ - logError: jest.fn(), - logInfo: jest.fn(), -})); - -const testData = 'test-data'; -const successfulGet = () => Promise.resolve(testData); -const error404 = { customAttributes: { httpErrorStatus: 404 }, test: 'error' }; -const error404Get = () => Promise.reject(error404); -const error500 = { customAttributes: { httpErrorStatus: 500 }, test: 'error' }; -const error500Get = () => Promise.reject(error500); - -const get = jest.fn().mockImplementation(successfulGet); -getAuthenticatedHttpClient.mockReturnValue({ get }); -const authenticatedUser = { fake: 'user' }; -getAuthenticatedUser.mockReturnValue(authenticatedUser); - -const onLoad = jest.fn(); -describe('getNotices api method', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - describe('behavior', () => { - describe('not authenticated', () => { - it('does not fetch anything', () => { - getAuthenticatedUser.mockReturnValueOnce(null); - api.getNotices({ onLoad }); - expect(get).not.toHaveBeenCalled(); - }); - }); - describe('authenticated', () => { - it('fetches noticesUrl with onLoad behavior', async () => { - await api.getNotices({ onLoad }); - expect(get).toHaveBeenCalledWith(api.noticesUrl, {}); - expect(onLoad).toHaveBeenCalledWith(testData); - }); - it('calls logInfo if fetch fails with 404', async () => { - get.mockImplementation(error404Get); - await api.getNotices({ onLoad }); - expect(logInfo).toHaveBeenCalledWith(`${error404}. ${api.error404Message}`); - }); - it('calls logError if fetch fails with non-404 error', async () => { - get.mockImplementation(error500Get); - await api.getNotices({ onLoad }); - expect(logError).toHaveBeenCalledWith(error500); - }); - }); - }); -}); diff --git a/src/components/NoticesWrapper/hooks.js b/src/components/NoticesWrapper/hooks.js deleted file mode 100644 index f16f8fb77..000000000 --- a/src/components/NoticesWrapper/hooks.js +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react'; -import { getConfig } from '@edx/frontend-platform'; -import { useIntl } from '@edx/frontend-platform/i18n'; - -import { StrictDict } from 'utils'; -import { getNotices } from './api'; -import * as module from './hooks'; -import messages from './messages'; - -/** - * This component uses the platform-plugin-notices plugin to function. - * If the user has an unacknowledged notice, they will be rerouted off - * course home and onto a full-screen notice page. If the plugin is not - * installed, or there are no notices, we just passthrough this component. - */ -export const state = StrictDict({ - isRedirected: (val) => React.useState(val), // eslint-disable-line -}); - -export const useNoticesWrapperData = () => { - const [isRedirected, setIsRedirected] = module.state.isRedirected(); - const { formatMessage } = useIntl(); - - React.useEffect(() => { - if (getConfig().ENABLE_NOTICES) { - getNotices({ - onLoad: (data) => { - if (data?.data?.results?.length > 0) { - setIsRedirected(true); - window.location.replace(`${data.data.results[0]}?next=${window.location.href}`); - } - }, - notFoundMessage: formatMessage(messages.error404Message), - }); - } - }, [setIsRedirected, formatMessage]); - return { isRedirected }; -}; - -export default useNoticesWrapperData; diff --git a/src/components/NoticesWrapper/hooks.test.js b/src/components/NoticesWrapper/hooks.test.js deleted file mode 100644 index 6e308c83b..000000000 --- a/src/components/NoticesWrapper/hooks.test.js +++ /dev/null @@ -1,99 +0,0 @@ -import React from 'react'; - -import { MockUseState, formatMessage } from 'testUtils'; - -import { getConfig } from '@edx/frontend-platform'; -import { getNotices } from './api'; -import * as hooks from './hooks'; - -jest.mock('@edx/frontend-platform', () => ({ getConfig: jest.fn() })); -jest.mock('./api', () => ({ getNotices: jest.fn() })); - -jest.mock('react', () => ({ - ...jest.requireActual('react'), - useEffect: jest.fn((cb, prereqs) => ({ useEffect: { cb, prereqs } })), - useContext: jest.fn(context => context), -})); - -jest.mock('@edx/frontend-platform/i18n', () => { - const { formatMessage: fn } = jest.requireActual('testUtils'); - return { - ...jest.requireActual('@edx/frontend-platform/i18n'), - useIntl: () => ({ - formatMessage: fn, - }), - }; -}); - -getConfig.mockReturnValue({ ENABLE_NOTICES: true }); -const state = new MockUseState(hooks); - -let hook; -describe('NoticesWrapper hooks', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - describe('state hooks', () => { - state.testGetter(state.keys.isRedirected); - }); - describe('useNoticesWrapperData', () => { - beforeEach(() => { - state.mock(); - }); - describe('behavior', () => { - it('initializes state hooks', () => { - hooks.useNoticesWrapperData(); - expect(hooks.state.isRedirected).toHaveBeenCalledWith(); - }); - describe('effects', () => { - it('does not call notices if not enabled', () => { - getConfig.mockReturnValueOnce({ ENABLE_NOTICES: false }); - hooks.useNoticesWrapperData(); - const [cb, prereqs] = React.useEffect.mock.calls[0]; - expect(prereqs).toEqual([state.setState.isRedirected, formatMessage]); - cb(); - expect(getNotices).not.toHaveBeenCalled(); - }); - describe('getNotices call (if enabled) onLoad behavior', () => { - it('does not redirect if there are no results', () => { - hooks.useNoticesWrapperData(); - expect(React.useEffect).toHaveBeenCalled(); - const [cb, prereqs] = React.useEffect.mock.calls[0]; - expect(prereqs).toEqual([state.setState.isRedirected, formatMessage]); - cb(); - expect(getNotices).toHaveBeenCalled(); - const { onLoad } = getNotices.mock.calls[0][0]; - onLoad({}); - expect(state.setState.isRedirected).not.toHaveBeenCalled(); - onLoad({ data: {} }); - expect(state.setState.isRedirected).not.toHaveBeenCalled(); - onLoad({ data: { results: [] } }); - expect(state.setState.isRedirected).not.toHaveBeenCalled(); - }); - it('redirects and set isRedirected if results are returned', () => { - delete window.location; - window.location = { replace: jest.fn(), href: 'test-old-href' }; - hooks.useNoticesWrapperData(); - const [cb, prereqs] = React.useEffect.mock.calls[0]; - expect(prereqs).toEqual([state.setState.isRedirected, formatMessage]); - cb(); - expect(getNotices).toHaveBeenCalled(); - const { onLoad } = getNotices.mock.calls[0][0]; - const target = 'url-target'; - onLoad({ data: { results: [target] } }); - expect(state.setState.isRedirected).toHaveBeenCalledWith(true); - expect(window.location.replace).toHaveBeenCalledWith( - `${target}?next=${window.location.href}`, - ); - }); - }); - }); - }); - describe('output', () => { - it('forwards isRedirected from state call', () => { - hook = hooks.useNoticesWrapperData(); - expect(hook.isRedirected).toEqual(state.stateVals.isRedirected); - }); - }); - }); -}); diff --git a/src/components/NoticesWrapper/index.jsx b/src/components/NoticesWrapper/index.jsx deleted file mode 100644 index faf9b9f2e..000000000 --- a/src/components/NoticesWrapper/index.jsx +++ /dev/null @@ -1,25 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -import useNoticesWrapperData from './hooks'; - -/** - * This component uses the platform-plugin-notices plugin to function. - * If the user has an unacknowledged notice, they will be rerouted off - * course home and onto a full-screen notice page. If the plugin is not - * installed, or there are no notices, we just passthrough this component. - */ -const NoticesWrapper = ({ children }) => { - const { isRedirected } = useNoticesWrapperData(); - return ( -
- {isRedirected === true ? null : children} -
- ); -}; - -NoticesWrapper.propTypes = { - children: PropTypes.node.isRequired, -}; - -export default NoticesWrapper; diff --git a/src/components/NoticesWrapper/index.test.jsx b/src/components/NoticesWrapper/index.test.jsx deleted file mode 100644 index add2b2fd6..000000000 --- a/src/components/NoticesWrapper/index.test.jsx +++ /dev/null @@ -1,36 +0,0 @@ -import { render, screen } from '@testing-library/react'; - -import useNoticesWrapperData from './hooks'; -import NoticesWrapper from '.'; - -jest.mock('./hooks', () => jest.fn()); - -const hookProps = { isRedirected: false }; - -const children = [some, children]; -describe('NoticesWrapper component', () => { - beforeEach(() => { - useNoticesWrapperData.mockClear(); - }); - describe('behavior', () => { - it('initializes hooks', () => { - useNoticesWrapperData.mockReturnValue(hookProps); - render({children}); - expect(useNoticesWrapperData).toHaveBeenCalledWith(); - }); - }); - describe('output', () => { - it('does not show children if redirected', () => { - useNoticesWrapperData.mockReturnValueOnce({ isRedirected: true }); - render({children}); - expect(screen.queryByText('some')).not.toBeInTheDocument(); - expect(screen.queryByText('children')).not.toBeInTheDocument(); - }); - it('shows children if not redirected', () => { - useNoticesWrapperData.mockReturnValue(hookProps); - render({children}); - expect(screen.getByText('some')).toBeInTheDocument(); - expect(screen.getByText('children')).toBeInTheDocument(); - }); - }); -}); diff --git a/src/components/NoticesWrapper/messages.js b/src/components/NoticesWrapper/messages.js deleted file mode 100644 index 6dbbefaaf..000000000 --- a/src/components/NoticesWrapper/messages.js +++ /dev/null @@ -1,11 +0,0 @@ -import { defineMessages } from '@edx/frontend-platform/i18n'; - -const messages = defineMessages({ - error404Message: { - id: 'learner-dash.notices.error404Message', - defaultMessage: 'This probably happened because the notices plugin is not installed on platform.', - description: 'Error message when notices API returns 404', - }, -}); - -export default messages; From 8969510bda9930f79f929b3f68b9cea350a3fa40 Mon Sep 17 00:00:00 2001 From: MaxFrank13 Date: Thu, 9 Oct 2025 20:37:40 +0000 Subject: [PATCH 2/3] feat!: remove notices wrapper --- src/index.jsx | 11 ++++------- src/index.test.jsx | 1 - src/test/app.test.jsx | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/index.jsx b/src/index.jsx index 418d81587..49e5858ac 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -27,7 +27,6 @@ import { configuration } from './config'; import messages from './i18n'; import App from './App'; -import NoticesWrapper from './components/NoticesWrapper'; subscribe(APP_READY, () => { const root = createRoot(document.getElementById('root')); @@ -35,12 +34,10 @@ subscribe(APP_READY, () => { root.render( - - - } /> - } /> - - + + } /> + } /> + , ); diff --git a/src/index.test.jsx b/src/index.test.jsx index 53a0fdccb..ba53223f8 100644 --- a/src/index.test.jsx +++ b/src/index.test.jsx @@ -35,7 +35,6 @@ jest.mock('@edx/frontend-platform', () => ({ jest.mock('data/store', () => ({ redux: 'store' })); jest.mock('./App', () => 'App'); -jest.mock('components/NoticesWrapper', () => 'NoticesWrapper'); describe('app registry', () => { let getElement; diff --git a/src/test/app.test.jsx b/src/test/app.test.jsx index 1a23f6537..b36b18c68 100644 --- a/src/test/app.test.jsx +++ b/src/test/app.test.jsx @@ -39,7 +39,7 @@ jest.unmock('reselect'); jest.unmock('hooks'); jest.mock('plugin-slots/WidgetSidebarSlot', () => jest.fn(() => 'widget-sidebar')); -jest.mock('components/NoticesWrapper', () => 'notices-wrapper'); +jest.mock('components/', () => 'notices-wrapper'); jest.mock('@edx/frontend-platform', () => ({ ...jest.requireActual('@edx/frontend-platform'), From 3c00618cdf31cde36cf187a2a27f53bd2eb8a917 Mon Sep 17 00:00:00 2001 From: MaxFrank13 Date: Thu, 9 Oct 2025 20:47:42 +0000 Subject: [PATCH 3/3] fix: import --- .env | 1 - .env.development | 1 - .env.test | 1 - example.env.config.js | 1 - src/config/index.js | 1 - src/test/app.test.jsx | 1 - 6 files changed, 6 deletions(-) diff --git a/.env b/.env index 8d2358235..b9b30856c 100644 --- a/.env +++ b/.env @@ -37,7 +37,6 @@ HOTJAR_VERSION='6' HOTJAR_DEBUG='' ACCOUNT_SETTINGS_URL='' ACCOUNT_PROFILE_URL='' -ENABLE_NOTICES='' CAREER_LINK_URL='' ENABLE_EDX_PERSONAL_DASHBOARD=false ENABLE_PROGRAMS=false diff --git a/.env.development b/.env.development index 7c547ada6..73183ccc2 100644 --- a/.env.development +++ b/.env.development @@ -43,7 +43,6 @@ HOTJAR_VERSION='6' HOTJAR_DEBUG='' ACCOUNT_SETTINGS_URL='http://localhost:1997' ACCOUNT_PROFILE_URL='http://localhost:1995' -ENABLE_NOTICES='' CAREER_LINK_URL='' ENABLE_EDX_PERSONAL_DASHBOARD=false ENABLE_PROGRAMS=false diff --git a/.env.test b/.env.test index 16087686c..aa19975b6 100644 --- a/.env.test +++ b/.env.test @@ -42,7 +42,6 @@ HOTJAR_VERSION='6' HOTJAR_DEBUG='' ACCOUNT_SETTINGS_URL='http://account-settings-url.test' ACCOUNT_PROFILE_URL='http://account-profile-url.test' -ENABLE_NOTICES='' CAREER_LINK_URL='' ENABLE_EDX_PERSONAL_DASHBOARD=true ENABLE_PROGRAMS=false diff --git a/example.env.config.js b/example.env.config.js index 52656aa67..70436fd55 100644 --- a/example.env.config.js +++ b/example.env.config.js @@ -67,7 +67,6 @@ module.exports = { NEW_RELIC_LICENSE_KEY: '', ACCOUNT_SETTINGS_URL: 'http://localhost:1997', ACCOUNT_PROFILE_URL: 'http://localhost:1995', - ENABLE_NOTICES: '', CAREER_LINK_URL: '', EXPERIMENT_08_23_VAN_PAINTED_DOOR: true, }; diff --git a/src/config/index.js b/src/config/index.js index ce55cf093..ae923c27e 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -14,7 +14,6 @@ const configuration = { LEARNING_BASE_URL: process.env.LEARNING_BASE_URL, SESSION_COOKIE_DOMAIN: process.env.SESSION_COOKIE_DOMAIN || '', SUPPORT_URL: process.env.SUPPORT_URL || null, - ENABLE_NOTICES: process.env.ENABLE_NOTICES || null, CAREER_LINK_URL: process.env.CAREER_LINK_URL || null, LOGO_URL: process.env.LOGO_URL, ENABLE_EDX_PERSONAL_DASHBOARD: process.env.ENABLE_EDX_PERSONAL_DASHBOARD === 'true', diff --git a/src/test/app.test.jsx b/src/test/app.test.jsx index b36b18c68..cf39efd1d 100644 --- a/src/test/app.test.jsx +++ b/src/test/app.test.jsx @@ -39,7 +39,6 @@ jest.unmock('reselect'); jest.unmock('hooks'); jest.mock('plugin-slots/WidgetSidebarSlot', () => jest.fn(() => 'widget-sidebar')); -jest.mock('components/', () => 'notices-wrapper'); jest.mock('@edx/frontend-platform', () => ({ ...jest.requireActual('@edx/frontend-platform'),