From 0606bca7ae52e15b892c578b4645dd2fb31e1dc7 Mon Sep 17 00:00:00 2001 From: "artur.filippovskii" Date: Mon, 10 Nov 2025 14:27:15 +0200 Subject: [PATCH 1/4] feat: add notification tray status to session storage --- src/courseware/course/Course.test.jsx | 4 +++ .../course/sidebar/SidebarContextProvider.jsx | 20 +++++++++----- .../course/sidebar/common/SidebarBase.jsx | 10 ++++++- .../notifications/NotificationTrigger.jsx | 27 +++++++++++++++++-- 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/courseware/course/Course.test.jsx b/src/courseware/course/Course.test.jsx index 744bebe548..f70fa08bb8 100644 --- a/src/courseware/course/Course.test.jsx +++ b/src/courseware/course/Course.test.jsx @@ -26,6 +26,10 @@ jest.mock( }, ); +jest.mock('@src/data/sessionStorage', () => ({ + getSessionStorage: jest.fn().mockReturnValue(null), +})); + const recordFirstSectionCelebration = jest.fn(); // eslint-disable-next-line no-import-assign celebrationUtils.recordFirstSectionCelebration = recordFirstSectionCelebration; diff --git a/src/courseware/course/sidebar/SidebarContextProvider.jsx b/src/courseware/course/sidebar/SidebarContextProvider.jsx index 9b3b824d53..70a8f382de 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.jsx @@ -6,6 +6,7 @@ import { import { useModel } from '@src/generic/model-store'; import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; +import { getSessionStorage } from '@src/data/sessionStorage'; import * as discussionsSidebar from './sidebars/discussions'; import * as notificationsSidebar from './sidebars/notifications'; @@ -25,12 +26,20 @@ const SidebarProvider = ({ const query = new URLSearchParams(window.location.search); const isInitiallySidebarOpen = shouldDisplaySidebarOpen || query.get('sidebar') === 'true'; - let initialSidebar = shouldDisplayFullScreen ? getLocalStorage(`sidebar.${courseId}`) : null; - if (!shouldDisplayFullScreen && isInitiallySidebarOpen) { - initialSidebar = isUnitHasDiscussionTopics - ? SIDEBARS[discussionsSidebar.ID].ID - : verifiedMode && SIDEBARS[notificationsSidebar.ID].ID; + const isNotificationTrayOpen = getSessionStorage(`notificationTrayStatus.${courseId}`) === 'open'; + + let initialSidebar; + if (isNotificationTrayOpen) { + initialSidebar = SIDEBARS[notificationsSidebar.ID].ID; + } else { + initialSidebar = shouldDisplayFullScreen ? getLocalStorage(`sidebar.${courseId}`) : null; + if (!shouldDisplayFullScreen && isInitiallySidebarOpen) { + initialSidebar = isUnitHasDiscussionTopics + ? SIDEBARS[discussionsSidebar.ID].ID + : verifiedMode && SIDEBARS[notificationsSidebar.ID].ID; + } } + const [currentSidebar, setCurrentSidebar] = useState(initialSidebar); const [notificationStatus, setNotificationStatus] = useState(getLocalStorage(`notificationStatus.${courseId}`)); const [upgradeNotificationCurrentState, setUpgradeNotificationCurrentState] = useState(getLocalStorage(`upgradeNotificationCurrentState.${courseId}`)); @@ -53,7 +62,6 @@ const SidebarProvider = ({ }, [courseId]); const toggleSidebar = useCallback((sidebarId) => { - // Switch to new sidebar or hide the current sidebar const newSidebar = sidebarId === currentSidebar ? null : sidebarId; setCurrentSidebar(newSidebar); setLocalStorage(`sidebar.${courseId}`, newSidebar); diff --git a/src/courseware/course/sidebar/common/SidebarBase.jsx b/src/courseware/course/sidebar/common/SidebarBase.jsx index 87775ea1c3..346b5ff530 100644 --- a/src/courseware/course/sidebar/common/SidebarBase.jsx +++ b/src/courseware/course/sidebar/common/SidebarBase.jsx @@ -4,7 +4,9 @@ import { ArrowBackIos, Close } from '@openedx/paragon/icons'; import classNames from 'classnames'; import PropTypes from 'prop-types'; import { useCallback, useContext } from 'react'; + import { useEventListener } from '@src/generic/hooks'; +import { setSessionStorage } from '@src/data/sessionStorage'; import messages from '../../messages'; import SidebarContext from '../SidebarContext'; @@ -19,6 +21,7 @@ const SidebarBase = ({ }) => { const intl = useIntl(); const { + courseId, toggleSidebar, shouldDisplayFullScreen, currentSidebar, @@ -34,6 +37,11 @@ const SidebarBase = ({ useEventListener('message', receiveMessage); + const handleCloseNotificationTray = () => { + toggleSidebar(null); + setSessionStorage(`notificationTrayStatus.${courseId}`, 'closed'); + }; + return (
toggleSidebar(null)} + onClick={handleCloseNotificationTray} variant="primary" alt={intl.formatMessage(messages.closeNotificationTrigger)} /> diff --git a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx index 4b2037be5b..f1c57a2d40 100644 --- a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx +++ b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import { WIDGETS } from '@src/constants'; import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; +import { getSessionStorage, setSessionStorage } from '@src/data/sessionStorage'; import messages from '../../../messages'; import SidebarTriggerBase from '../../common/TriggerBase'; import SidebarContext from '../../SidebarContext'; @@ -19,6 +20,8 @@ const NotificationTrigger = ({ courseId, notificationStatus, setNotificationStatus, + currentSidebar, + toggleSidebar, upgradeNotificationCurrentState, } = useContext(SidebarContext); @@ -45,10 +48,30 @@ const NotificationTrigger = ({ useEffect(() => { UpdateUpgradeNotificationLastSeen(); - }); + const notificationTrayStatus = getSessionStorage(`notificationTrayStatus.${courseId}`); + const isNotificationTrayOpen = notificationTrayStatus === 'open'; + + if (isNotificationTrayOpen && !currentSidebar) { + if (toggleSidebar) { + toggleSidebar(ID); + } + } + }, [courseId, currentSidebar, ID]); + + const handleClick = () => { + const isNotificationTrayOpen = getSessionStorage(`notificationTrayStatus.${courseId}`) === 'open'; + + if (isNotificationTrayOpen) { + setSessionStorage(`notificationTrayStatus.${courseId}`, 'closed'); + } else { + setSessionStorage(`notificationTrayStatus.${courseId}`, 'open'); + } + + onClick(); + }; return ( - + ); From aa55953cd7f060ab44f36eae157df25b6b739bcd Mon Sep 17 00:00:00 2001 From: "artur.filippovskii" Date: Thu, 20 Nov 2025 14:56:05 +0200 Subject: [PATCH 2/4] fix: fix test and upgrade coverage --- src/courseware/course/Course.test.jsx | 22 +- .../sidebar/SidebarContextProvider.test.jsx | 199 ++++++++++++++++++ .../NotificationTrigger.test.jsx | 154 +++++++++----- 3 files changed, 322 insertions(+), 53 deletions(-) create mode 100644 src/courseware/course/sidebar/SidebarContextProvider.test.jsx diff --git a/src/courseware/course/Course.test.jsx b/src/courseware/course/Course.test.jsx index f70fa08bb8..e092c5ecd6 100644 --- a/src/courseware/course/Course.test.jsx +++ b/src/courseware/course/Course.test.jsx @@ -11,6 +11,7 @@ import * as celebrationUtils from './celebration/utils'; import { handleNextSectionCelebration } from './celebration'; import Course from './Course'; import setupDiscussionSidebar from './test-utils'; +import SidebarProvider from './sidebar/SidebarContextProvider'; jest.mock('@edx/frontend-platform/analytics'); jest.mock('@edx/frontend-lib-special-exams/dist/data/thunks.js', () => ({ @@ -155,6 +156,13 @@ describe('Course', () => { it('handles click to open/close discussions sidebar', async () => { await setupDiscussionSidebar(); + render( + + + , + { wrapWithRouter: true }, + ); + waitFor(() => { expect(screen.getByTestId('sidebar-DISCUSSIONS')).toBeInTheDocument(); expect(screen.getByTestId('sidebar-DISCUSSIONS')).not.toHaveClass('d-none'); @@ -184,7 +192,12 @@ describe('Course', () => { await setupDiscussionSidebar(); - const { rerender } = render(, { store: testStore }); + const { rerender } = render( + + + , + { store: testStore }, + ); loadUnit(); waitFor(() => { @@ -197,6 +210,13 @@ describe('Course', () => { it('handles click to open/close notification tray', async () => { await setupDiscussionSidebar(); + render( + + + , + { wrapWithRouter: true }, + ); + waitFor(() => { const notificationShowButton = screen.findByRole('button', { name: /Show notification tray/i }); expect(screen.queryByRole('region', { name: /notification tray/i })).not.toBeInTheDocument(); diff --git a/src/courseware/course/sidebar/SidebarContextProvider.test.jsx b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx new file mode 100644 index 0000000000..33411dfab7 --- /dev/null +++ b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx @@ -0,0 +1,199 @@ +import { useContext } from 'react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useWindowSize } from '@openedx/paragon'; + +import { useModel } from '@src/generic/model-store'; +import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; +import { getSessionStorage } from '@src/data/sessionStorage'; + +import SidebarProvider from './SidebarContextProvider'; +import SidebarContext from './SidebarContext'; +import * as discussionsSidebar from './sidebars/discussions'; +import * as notificationsSidebar from './sidebars/notifications'; + +jest.mock('@openedx/paragon', () => ({ + ...jest.requireActual('@openedx/paragon'), + useWindowSize: jest.fn(), + breakpoints: { + extraLarge: { minWidth: 1200 }, + }, +})); + +jest.mock('@src/generic/model-store', () => ({ + useModel: jest.fn(), +})); + +jest.mock('@src/data/localStorage', () => ({ + getLocalStorage: jest.fn(), + setLocalStorage: jest.fn(), +})); + +jest.mock('@src/data/sessionStorage', () => ({ + getSessionStorage: jest.fn(), +})); + +jest.mock('./sidebars/discussions', () => ({ ID: 'discussions' })); +jest.mock('./sidebars/notifications', () => ({ ID: 'notifications' })); +jest.mock('./sidebars', () => ({ + SIDEBARS: { + discussions: { ID: 'discussions' }, + notifications: { ID: 'notifications' }, + }, +})); + +const TestConsumer = () => { + const { + currentSidebar, + toggleSidebar, + onNotificationSeen, + notificationStatus, + } = useContext(SidebarContext); + + return ( +
+
{currentSidebar || 'none'}
+
{notificationStatus || 'none'}
+ + +
+ ); +}; + +describe('SidebarContextProvider', () => { + const defaultProps = { + courseId: 'course-v1:test', + unitId: 'unit-1', + }; + + beforeEach(() => { + jest.clearAllMocks(); + useWindowSize.mockReturnValue({ width: 1400 }); + useModel.mockReturnValue({}); + getLocalStorage.mockReturnValue(null); + getSessionStorage.mockReturnValue(null); + }); + + it('renders without crashing and provides default context', () => { + render( + + + , + ); + expect(screen.getByTestId('current-sidebar')).toHaveTextContent('none'); + }); + + it('initializes with notifications sidebar if notification tray is open in session storage', () => { + getSessionStorage.mockReturnValue('open'); + + render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent(notificationsSidebar.ID); + }); + + it('loads initial sidebar from local storage on small screens (mobile behavior)', () => { + useWindowSize.mockReturnValue({ width: 800 }); + getLocalStorage.mockImplementation((key) => { + if (key === `sidebar.${defaultProps.courseId}`) { return discussionsSidebar.ID; } + return null; + }); + + render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent(discussionsSidebar.ID); + }); + + it('does not load from local storage on large screens (desktop behavior)', () => { + useWindowSize.mockReturnValue({ width: 1400 }); + getLocalStorage.mockReturnValue(discussionsSidebar.ID); + + render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent('none'); + }); + + it('toggles sidebar open and updates local storage', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent('none'); + + await user.click(screen.getByText('Toggle Discussions')); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent(discussionsSidebar.ID); + expect(setLocalStorage).toHaveBeenCalledWith(`sidebar.${defaultProps.courseId}`, discussionsSidebar.ID); + }); + + it('toggles sidebar closed (null) if clicking the same sidebar', async () => { + useWindowSize.mockReturnValue({ width: 800 }); + getLocalStorage.mockReturnValue(discussionsSidebar.ID); + const user = userEvent.setup(); + + render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent(discussionsSidebar.ID); + + await user.click(screen.getByText('Toggle Discussions')); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent('none'); + expect(setLocalStorage).toHaveBeenCalledWith(`sidebar.${defaultProps.courseId}`, null); + }); + + it('updates notification status when seen', async () => { + const user = userEvent.setup(); + render( + + + , + ); + + await user.click(screen.getByText('See Notifications')); + + expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${defaultProps.courseId}`, 'inactive'); + expect(screen.getByTestId('notification-status')).toHaveTextContent('inactive'); + }); + + it('updates current sidebar when unitId changes (Effect trigger)', () => { + useWindowSize.mockReturnValue({ width: 800 }); + getLocalStorage.mockReturnValue(notificationsSidebar.ID); + + const { rerender } = render( + + + , + ); + + expect(screen.getByTestId('current-sidebar')).toHaveTextContent(notificationsSidebar.ID); + + useModel.mockImplementation((model) => { + if (model === 'discussionTopics') { return { id: 'topic-1', enabledInContext: true }; } + return {}; + }); + + rerender( + + + , + ); + }); +}); diff --git a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx index c6c264bec3..9e58811376 100644 --- a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx +++ b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx @@ -1,16 +1,29 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Factory } from 'rosie'; +import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; +import { getSessionStorage, setSessionStorage } from '@src/data/sessionStorage'; import { fireEvent, initializeTestStore, render, screen, } from '../../../../../setupTest'; import SidebarContext from '../../SidebarContext'; -import NotificationTrigger from './NotificationTrigger'; +// FIX 1: Імпортуємо ID +import NotificationTrigger, { ID } from './NotificationTrigger'; + +// FIX 2: Використовуємо модульні моки замість window.localStorage + +jest.mock('@src/data/localStorage', () => ({ + getLocalStorage: jest.fn(), + setLocalStorage: jest.fn(), +})); + +jest.mock('@src/data/sessionStorage', () => ({ + getSessionStorage: jest.fn(), + setSessionStorage: jest.fn(), +})); describe('Notification Trigger', () => { let mockData; - let getItemSpy; - let setItemSpy; const courseMetadata = Factory.build('courseMetadata'); beforeEach(async () => { @@ -19,22 +32,21 @@ describe('Notification Trigger', () => { excludeFetchCourse: true, excludeFetchSequence: true, }); + + jest.clearAllMocks(); + mockData = { courseId: courseMetadata.id, - toggleNotificationTray: () => {}, - isNotificationTrayVisible: () => {}, + toggleSidebar: jest.fn(), + currentSidebar: null, notificationStatus: 'inactive', - setNotificationStatus: () => {}, + setNotificationStatus: jest.fn(), upgradeNotificationCurrentState: 'FPDdaysLeft', }; - // Jest does not support calls to localStorage, spying on localStorage's prototype directly instead - getItemSpy = jest.spyOn(Object.getPrototypeOf(window.localStorage), 'getItem'); - setItemSpy = jest.spyOn(Object.getPrototypeOf(window.localStorage), 'setItem'); - }); - afterAll(() => { - getItemSpy.mockRestore(); - setItemSpy.mockRestore(); + // Default mocks + getLocalStorage.mockReturnValue(null); + getSessionStorage.mockReturnValue('closed'); }); const SidebarWrapper = ({ contextValue, onClick }) => ( @@ -48,8 +60,7 @@ describe('Notification Trigger', () => { onClick: PropTypes.func.isRequired, }; - function renderWithProvider(data, onClick = () => { - }) { + function renderWithProvider(data, onClick = () => {}) { const { container } = render(); return container; } @@ -58,14 +69,21 @@ describe('Notification Trigger', () => { const toggleNotificationTray = jest.fn(); const testData = { ...mockData, - toggleNotificationTray, + toggleSidebar: toggleNotificationTray, // Fix naming for consistnecy if needed, usually passed via context }; - renderWithProvider(testData, toggleNotificationTray); + // We are testing the onClick passed to component, not context toggle here specifically for the trigger click + const onClickProp = jest.fn(); + + renderWithProvider(testData, onClickProp); const notificationTrigger = screen.getByRole('button', { name: /Show notification tray/i }); expect(notificationTrigger).toBeInTheDocument(); + fireEvent.click(notificationTrigger); - expect(toggleNotificationTray).toHaveBeenCalledTimes(1); + + expect(onClickProp).toHaveBeenCalledTimes(1); + // Check SessionStorage update via module mock + expect(setSessionStorage).toHaveBeenCalledWith(`notificationTrayStatus.${mockData.courseId}`, 'open'); }); it('renders notification trigger icon with red dot when notificationStatus is active', async () => { @@ -77,61 +95,93 @@ describe('Notification Trigger', () => { }); it('renders notification trigger icon WITHOUT red dot within the same phase', async () => { + // FIX: Mock return value directly + getLocalStorage.mockReturnValue('sameState'); + const container = renderWithProvider({ upgradeNotificationLastSeen: 'sameState', upgradeNotificationCurrentState: 'sameState', }); - expect(container) - .toBeInTheDocument(); - expect(localStorage.getItem) - .toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); - expect(localStorage.getItem(`upgradeNotificationLastSeen.${mockData.courseId}`)) - .toBe('"sameState"'); + + expect(container).toBeInTheDocument(); + + // Check module mock call + expect(getLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); + const buttonIcon = container.querySelectorAll('svg'); - expect(buttonIcon) - .toHaveLength(1); - expect(screen.queryByRole('notification-dot')) - .not - .toBeInTheDocument(); + expect(buttonIcon).toHaveLength(1); + expect(screen.queryByRole('notification-dot')).not.toBeInTheDocument(); }); - // Rendering NotificationTrigger has the effect of calling UpdateUpgradeNotificationLastSeen(), - // if upgradeNotificationLastSeen is different than upgradeNotificationCurrentState - // it should update localStorage accordingly it('makes the right updates when rendering a new phase from an UpgradeNotification change (before -> after)', async () => { + // FIX: Mock implementation to return 'before' for lastSeen + getLocalStorage.mockImplementation((key) => { + if (key.includes('upgradeNotificationLastSeen')) { return 'before'; } + return null; + }); + const container = renderWithProvider({ - upgradeNotificationLastSeen: 'before', upgradeNotificationCurrentState: 'after', }); - expect(container).toBeInTheDocument(); - // verify localStorage get/set are called with correct arguments - expect(localStorage.getItem).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); - expect(localStorage.setItem).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, '"active"'); - expect(localStorage.setItem).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`, '"after"'); + expect(container).toBeInTheDocument(); - // verify localStorage is updated accordingly - expect(localStorage.getItem(`upgradeNotificationLastSeen.${mockData.courseId}`)).toBe('"after"'); - expect(localStorage.getItem(`notificationStatus.${mockData.courseId}`)).toBe('"active"'); + // Verify calls to module mocks + expect(getLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); + expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, 'active'); + expect(setLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`, 'after'); }); it('handles localStorage from a different course', async () => { - const courseMetadataSecondCourse = Factory.build('courseMetadata', { id: 'second_id' }); - // set localStorage for a different course before rendering NotificationTrigger - localStorage.setItem(`upgradeNotificationLastSeen.${courseMetadataSecondCourse.id}`, '"accessDateView"'); - localStorage.setItem(`notificationStatus.${courseMetadataSecondCourse.id}`, '"inactive"'); + // This test logic was checking if localStorage affects other courses. + // Since we mock the module, we verify that we call it with the CORRECT course ID. + + getLocalStorage.mockImplementation((key) => { + if (key === `upgradeNotificationLastSeen.${mockData.courseId}`) { return 'before'; } + return 'accessDateView'; // Simulate other data existing + }); const container = renderWithProvider({ - upgradeNotificationLastSeen: 'before', upgradeNotificationCurrentState: 'after', }); + expect(container).toBeInTheDocument(); - // Verify localStorage was updated for the original course - expect(localStorage.getItem(`upgradeNotificationLastSeen.${mockData.courseId}`)).toBe('"after"'); - expect(localStorage.getItem(`notificationStatus.${mockData.courseId}`)).toBe('"active"'); - // Verify the second course localStorage was not changed - expect(localStorage.getItem(`upgradeNotificationLastSeen.${courseMetadataSecondCourse.id}`)).toBe('"accessDateView"'); - expect(localStorage.getItem(`notificationStatus.${courseMetadataSecondCourse.id}`)).toBe('"inactive"'); + // Verify we updated OUR course + expect(setLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`, 'after'); + expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, 'active'); + + // Verify we did NOT update the other course (mock check) + expect(setLocalStorage).not.toHaveBeenCalledWith(expect.stringContaining('second_id'), expect.anything()); + }); + + // --- Coverage Tests --- + + it('initializes default localStorage values if they are missing', () => { + getLocalStorage.mockReturnValue(null); + + renderWithProvider({}); + + expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, 'active'); + expect(setLocalStorage).toHaveBeenCalledWith(`upgradeNotificationCurrentState.${mockData.courseId}`, 'initialize'); + }); + + it('automatically opens sidebar if notification tray is open in session and sidebar is closed', () => { + getSessionStorage.mockReturnValue('open'); + const contextData = { currentSidebar: null, toggleSidebar: jest.fn() }; + + renderWithProvider(contextData); + + // FIX 1: ID is now imported and defined + expect(contextData.toggleSidebar).toHaveBeenCalledWith(ID); + }); + + it('does NOT automatically open sidebar if it is already open (even if tray is open)', () => { + getSessionStorage.mockReturnValue('open'); + const contextData = { currentSidebar: 'discussions', toggleSidebar: jest.fn() }; + + renderWithProvider(contextData); + + expect(contextData.toggleSidebar).not.toHaveBeenCalled(); }); }); From 281a5bcaed65d622ad93dcf19200cc409eab6b78 Mon Sep 17 00:00:00 2001 From: "artur.filippovskii" Date: Wed, 26 Nov 2025 14:57:56 +0200 Subject: [PATCH 3/4] fix: fix bug and remove extra code --- .../course/sidebar/SidebarContextProvider.jsx | 1 + .../sidebar/SidebarContextProvider.test.jsx | 16 ++++++++++---- .../course/sidebar/common/SidebarBase.jsx | 4 ++-- .../NotificationTrigger.test.jsx | 22 ++----------------- webpack.dev-tutor.config.js | 0 5 files changed, 17 insertions(+), 26 deletions(-) create mode 100644 webpack.dev-tutor.config.js diff --git a/src/courseware/course/sidebar/SidebarContextProvider.jsx b/src/courseware/course/sidebar/SidebarContextProvider.jsx index 70a8f382de..1d74988be8 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.jsx @@ -62,6 +62,7 @@ const SidebarProvider = ({ }, [courseId]); const toggleSidebar = useCallback((sidebarId) => { + // Switch to new sidebar or hide the current sidebar const newSidebar = sidebarId === currentSidebar ? null : sidebarId; setCurrentSidebar(newSidebar); setLocalStorage(`sidebar.${courseId}`, newSidebar); diff --git a/src/courseware/course/sidebar/SidebarContextProvider.test.jsx b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx index 33411dfab7..bc45e5aeb7 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.test.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx @@ -1,5 +1,4 @@ import { useContext } from 'react'; -import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useWindowSize } from '@openedx/paragon'; @@ -7,6 +6,7 @@ import { useModel } from '@src/generic/model-store'; import { getLocalStorage, setLocalStorage } from '@src/data/localStorage'; import { getSessionStorage } from '@src/data/sessionStorage'; +import { initializeTestStore, render, screen } from '@src/setupTest'; import SidebarProvider from './SidebarContextProvider'; import SidebarContext from './SidebarContext'; import * as discussionsSidebar from './sidebars/discussions'; @@ -20,9 +20,13 @@ jest.mock('@openedx/paragon', () => ({ }, })); -jest.mock('@src/generic/model-store', () => ({ - useModel: jest.fn(), -})); +jest.mock('@src/generic/model-store', () => { + const actual = jest.requireActual('@src/generic/model-store'); + return { + ...actual, + useModel: jest.fn(), + }; +}); jest.mock('@src/data/localStorage', () => ({ getLocalStorage: jest.fn(), @@ -66,6 +70,10 @@ describe('SidebarContextProvider', () => { unitId: 'unit-1', }; + beforeAll(async () => { + await initializeTestStore(); + }); + beforeEach(() => { jest.clearAllMocks(); useWindowSize.mockReturnValue({ width: 1400 }); diff --git a/src/courseware/course/sidebar/common/SidebarBase.jsx b/src/courseware/course/sidebar/common/SidebarBase.jsx index 346b5ff530..9c26d6bdd7 100644 --- a/src/courseware/course/sidebar/common/SidebarBase.jsx +++ b/src/courseware/course/sidebar/common/SidebarBase.jsx @@ -57,8 +57,8 @@ const SidebarBase = ({ {shouldDisplayFullScreen ? (
toggleSidebar(null)} - onKeyDown={() => toggleSidebar(null)} + onClick={handleCloseNotificationTray} + onKeyDown={handleCloseNotificationTray} role="button" tabIndex="0" > diff --git a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx index 9e58811376..82eadb0782 100644 --- a/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx +++ b/src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx @@ -7,11 +7,8 @@ import { fireEvent, initializeTestStore, render, screen, } from '../../../../../setupTest'; import SidebarContext from '../../SidebarContext'; -// FIX 1: Імпортуємо ID import NotificationTrigger, { ID } from './NotificationTrigger'; -// FIX 2: Використовуємо модульні моки замість window.localStorage - jest.mock('@src/data/localStorage', () => ({ getLocalStorage: jest.fn(), setLocalStorage: jest.fn(), @@ -44,7 +41,6 @@ describe('Notification Trigger', () => { upgradeNotificationCurrentState: 'FPDdaysLeft', }; - // Default mocks getLocalStorage.mockReturnValue(null); getSessionStorage.mockReturnValue('closed'); }); @@ -69,9 +65,8 @@ describe('Notification Trigger', () => { const toggleNotificationTray = jest.fn(); const testData = { ...mockData, - toggleSidebar: toggleNotificationTray, // Fix naming for consistnecy if needed, usually passed via context + toggleSidebar: toggleNotificationTray, }; - // We are testing the onClick passed to component, not context toggle here specifically for the trigger click const onClickProp = jest.fn(); renderWithProvider(testData, onClickProp); @@ -82,7 +77,6 @@ describe('Notification Trigger', () => { fireEvent.click(notificationTrigger); expect(onClickProp).toHaveBeenCalledTimes(1); - // Check SessionStorage update via module mock expect(setSessionStorage).toHaveBeenCalledWith(`notificationTrayStatus.${mockData.courseId}`, 'open'); }); @@ -95,7 +89,6 @@ describe('Notification Trigger', () => { }); it('renders notification trigger icon WITHOUT red dot within the same phase', async () => { - // FIX: Mock return value directly getLocalStorage.mockReturnValue('sameState'); const container = renderWithProvider({ @@ -105,7 +98,6 @@ describe('Notification Trigger', () => { expect(container).toBeInTheDocument(); - // Check module mock call expect(getLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); const buttonIcon = container.querySelectorAll('svg'); @@ -114,7 +106,6 @@ describe('Notification Trigger', () => { }); it('makes the right updates when rendering a new phase from an UpgradeNotification change (before -> after)', async () => { - // FIX: Mock implementation to return 'before' for lastSeen getLocalStorage.mockImplementation((key) => { if (key.includes('upgradeNotificationLastSeen')) { return 'before'; } return null; @@ -126,19 +117,15 @@ describe('Notification Trigger', () => { expect(container).toBeInTheDocument(); - // Verify calls to module mocks expect(getLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`); expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, 'active'); expect(setLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`, 'after'); }); it('handles localStorage from a different course', async () => { - // This test logic was checking if localStorage affects other courses. - // Since we mock the module, we verify that we call it with the CORRECT course ID. - getLocalStorage.mockImplementation((key) => { if (key === `upgradeNotificationLastSeen.${mockData.courseId}`) { return 'before'; } - return 'accessDateView'; // Simulate other data existing + return 'accessDateView'; }); const container = renderWithProvider({ @@ -147,16 +134,12 @@ describe('Notification Trigger', () => { expect(container).toBeInTheDocument(); - // Verify we updated OUR course expect(setLocalStorage).toHaveBeenCalledWith(`upgradeNotificationLastSeen.${mockData.courseId}`, 'after'); expect(setLocalStorage).toHaveBeenCalledWith(`notificationStatus.${mockData.courseId}`, 'active'); - // Verify we did NOT update the other course (mock check) expect(setLocalStorage).not.toHaveBeenCalledWith(expect.stringContaining('second_id'), expect.anything()); }); - // --- Coverage Tests --- - it('initializes default localStorage values if they are missing', () => { getLocalStorage.mockReturnValue(null); @@ -172,7 +155,6 @@ describe('Notification Trigger', () => { renderWithProvider(contextData); - // FIX 1: ID is now imported and defined expect(contextData.toggleSidebar).toHaveBeenCalledWith(ID); }); diff --git a/webpack.dev-tutor.config.js b/webpack.dev-tutor.config.js new file mode 100644 index 0000000000..e69de29bb2 From 97e27a38e6fca27121cb5f476fc9cf1f5addf622 Mon Sep 17 00:00:00 2001 From: "artur.filippovskii" Date: Fri, 28 Nov 2025 12:55:22 +0200 Subject: [PATCH 4/4] fix: remove extra file and add him to gitignore --- .gitignore | 2 ++ webpack.dev-tutor.config.js | 0 2 files changed, 2 insertions(+) delete mode 100644 webpack.dev-tutor.config.js diff --git a/.gitignore b/.gitignore index 3fc643913a..fc47888426 100755 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,5 @@ module.config.js src/i18n/messages/ env.config.jsx + +webpack.dev-tutor.config.js diff --git a/webpack.dev-tutor.config.js b/webpack.dev-tutor.config.js deleted file mode 100644 index e69de29bb2..0000000000