From e6443ae3bd643a03e22ca58188d7b13c78a4b708 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Mon, 29 Jun 2020 09:35:59 +0200 Subject: [PATCH] [TNL-7268] Address the review comments --- .../course/sequence/Sequence.test.jsx | 322 ++++++++++-------- .../SequenceNavigationDropdown.test.jsx | 3 + .../SequenceNavigationTabs.test.jsx | 20 +- 3 files changed, 181 insertions(+), 164 deletions(-) diff --git a/src/courseware/course/sequence/Sequence.test.jsx b/src/courseware/course/sequence/Sequence.test.jsx index edc1f528..9e575c4f 100644 --- a/src/courseware/course/sequence/Sequence.test.jsx +++ b/src/courseware/course/sequence/Sequence.test.jsx @@ -70,171 +70,199 @@ describe('Sequence', () => { expect(beforeLoadingUnit).toMatchDiffSnapshot(asFragment()); }); - it('navigates to the previous sequence if the unit is the first in the sequence', async () => { - sendTrackEvent.mockClear(); - const unitId = '1'; - const sequenceId = '2'; - const previousSequenceHandler = jest.fn(); - render(); + describe('sequence and unit navigation buttons', () => { + it('navigates to the previous sequence if the unit is the first in the sequence', async () => { + sendTrackEvent.mockClear(); + const unitId = '1'; + const sequenceId = '2'; + const previousSequenceHandler = jest.fn(); + render(); - const sequencePreviousButton = screen.getByRole('button', { name: /previous/i }); - fireEvent.click(sequencePreviousButton); - expect(previousSequenceHandler).toHaveBeenCalledTimes(1); - expect(sendTrackEvent).toHaveBeenCalledTimes(1); - expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.previous_selected', { - current_tab: Number(unitId), id: unitId, tab_count: testUnits.length, widget_placement: 'top', + const sequencePreviousButton = screen.getByRole('button', { name: /previous/i }); + fireEvent.click(sequencePreviousButton); + expect(previousSequenceHandler).toHaveBeenCalledTimes(1); + expect(sendTrackEvent).toHaveBeenCalledTimes(1); + expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.previous_selected', { + current_tab: Number(unitId), + id: unitId, + tab_count: testUnits.length, + widget_placement: 'top', + }); + + window.postMessage(messageEvent, '*'); + await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); + const unitPreviousButton = screen.getAllByRole('button', { name: /previous/i }) + .filter(button => button !== sequencePreviousButton)[0]; + fireEvent.click(unitPreviousButton); + expect(previousSequenceHandler).toHaveBeenCalledTimes(2); + expect(sendTrackEvent).toHaveBeenCalledTimes(2); + expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.previous_selected', { + current_tab: Number(unitId), + id: unitId, + tab_count: testUnits.length, + widget_placement: 'bottom', + }); }); - window.postMessage(messageEvent, '*'); - await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - const unitPreviousButton = screen.getAllByRole('button', { name: /previous/i }) - .filter(button => button !== sequencePreviousButton)[0]; - fireEvent.click(unitPreviousButton); - expect(previousSequenceHandler).toHaveBeenCalledTimes(2); - expect(sendTrackEvent).toHaveBeenCalledTimes(2); - expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.previous_selected', { - current_tab: Number(unitId), id: unitId, tab_count: testUnits.length, widget_placement: 'bottom', - }); - }); + it('navigates to the next sequence if the unit is the last in the sequence', async () => { + sendTrackEvent.mockClear(); + const unitId = String(testUnits.length); + const sequenceId = '1'; + const nextSequenceHandler = jest.fn(); + render(); - it('navigates to the next sequence if the unit is the last in the sequence', async () => { - sendTrackEvent.mockClear(); - const unitId = String(testUnits.length); - const sequenceId = '1'; - const nextSequenceHandler = jest.fn(); - render(); + const sequenceNextButton = screen.getByRole('button', { name: /next/i }); + fireEvent.click(sequenceNextButton); + expect(nextSequenceHandler).toHaveBeenCalledTimes(1); + expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.next_selected', { + current_tab: Number(unitId), + id: unitId, + tab_count: testUnits.length, + widget_placement: 'top', + }); - const sequenceNextButton = screen.getByRole('button', { name: /next/i }); - fireEvent.click(sequenceNextButton); - expect(nextSequenceHandler).toHaveBeenCalledTimes(1); - expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.next_selected', { - current_tab: Number(unitId), id: unitId, tab_count: testUnits.length, widget_placement: 'top', + window.postMessage(messageEvent, '*'); + await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); + const unitNextButton = screen.getAllByRole('button', { name: /next/i }) + .filter(button => button !== sequenceNextButton)[0]; + fireEvent.click(unitNextButton); + expect(nextSequenceHandler).toHaveBeenCalledTimes(2); + expect(sendTrackEvent).toHaveBeenCalledTimes(2); + expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.next_selected', { + current_tab: Number(unitId), + id: unitId, + tab_count: testUnits.length, + widget_placement: 'bottom', + }); }); - window.postMessage(messageEvent, '*'); - await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - const unitNextButton = screen.getAllByRole('button', { name: /next/i }) - .filter(button => button !== sequenceNextButton)[0]; - fireEvent.click(unitNextButton); - expect(nextSequenceHandler).toHaveBeenCalledTimes(2); - expect(sendTrackEvent).toHaveBeenCalledTimes(2); - expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.next_selected', { - current_tab: Number(unitId), id: unitId, tab_count: testUnits.length, widget_placement: 'bottom', + it('navigates to the previous/next unit if the unit is not in the corner of the sequence', () => { + sendTrackEvent.mockClear(); + const unitNavigationHandler = jest.fn(); + const previousSequenceHandler = jest.fn(); + const nextSequenceHandler = jest.fn(); + render(); + + fireEvent.click(screen.getByRole('button', { name: /previous/i })); + expect(previousSequenceHandler).not.toHaveBeenCalled(); + expect(unitNavigationHandler).toHaveBeenCalledWith(String(Number(mockData.unitId) - 1)); + + fireEvent.click(screen.getByRole('button', { name: /next/i })); + expect(nextSequenceHandler).not.toHaveBeenCalled(); + // As `previousSequenceHandler` and `nextSequenceHandler` are mocked, we aren't really changing the position here. + // Therefore the next unit will still be `the initial one + 1`. + expect(unitNavigationHandler).toHaveBeenNthCalledWith(2, String(Number(mockData.unitId) + 1)); + + expect(sendTrackEvent).toHaveBeenCalledTimes(2); }); - }); - it('navigates to the previous/next unit if the unit is not in the corner of the sequence', () => { - sendTrackEvent.mockClear(); - const unitNavigationHandler = jest.fn(); - const previousSequenceHandler = jest.fn(); - const nextSequenceHandler = jest.fn(); - render(); + it('handles the `Previous` buttons for the first unit in the first sequence', async () => { + sendTrackEvent.mockClear(); + const unitNavigationHandler = jest.fn(); + const previousSequenceHandler = jest.fn(); + const unitId = '1'; + render(); + window.postMessage(messageEvent, '*'); + await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - fireEvent.click(screen.getByRole('button', { name: /previous/i })); - expect(previousSequenceHandler).not.toHaveBeenCalled(); - expect(unitNavigationHandler).toHaveBeenCalledWith(String(Number(mockData.unitId) - 1)); + screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button)); - fireEvent.click(screen.getByRole('button', { name: /next/i })); - expect(nextSequenceHandler).not.toHaveBeenCalled(); - expect(unitNavigationHandler).toHaveBeenNthCalledWith(2, String(Number(mockData.unitId) + 1)); - - expect(sendTrackEvent).toHaveBeenCalledTimes(2); - }); - - it('handles the `Previous` buttons for the first unit in the first sequence', async () => { - sendTrackEvent.mockClear(); - const unitNavigationHandler = jest.fn(); - const previousSequenceHandler = jest.fn(); - const unitId = '1'; - render(); - window.postMessage(messageEvent, '*'); - await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - - screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button)); - - expect(previousSequenceHandler).not.toHaveBeenCalled(); - expect(unitNavigationHandler).not.toHaveBeenCalled(); - expect(sendTrackEvent).not.toHaveBeenCalled(); - }); - - it('handles the `Next` buttons for the last unit in the last sequence', async () => { - sendTrackEvent.mockClear(); - const unitNavigationHandler = jest.fn(); - const nextSequenceHandler = jest.fn(); - const unitId = String(testUnits.length); - const sequenceId = String(Object.keys(initialState.models.sequences).length); - render(); - window.postMessage(messageEvent, '*'); - await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - - screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button)); - - expect(nextSequenceHandler).toHaveBeenCalledTimes(1); - expect(unitNavigationHandler).not.toHaveBeenCalled(); - expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.next_selected', { - current_tab: Number(unitId), id: unitId, tab_count: testUnits.length, widget_placement: 'top', + expect(previousSequenceHandler).not.toHaveBeenCalled(); + expect(unitNavigationHandler).not.toHaveBeenCalled(); + expect(sendTrackEvent).not.toHaveBeenCalled(); }); - }); - it('handles the navigation buttons for empty sequence', async () => { - sendTrackEvent.mockClear(); - const testState = cloneDeep(initialState); - testState.models.sequences['1'].unitIds = []; + it('handles the `Next` buttons for the last unit in the last sequence', async () => { + sendTrackEvent.mockClear(); + const unitNavigationHandler = jest.fn(); + const nextSequenceHandler = jest.fn(); + const unitId = String(testUnits.length); + const sequenceId = String(Object.keys(initialState.models.sequences).length); + render(); + window.postMessage(messageEvent, '*'); + await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); - const unitNavigationHandler = jest.fn(); - const previousSequenceHandler = jest.fn(); - const nextSequenceHandler = jest.fn(); - render(, { initialState: testState }); - window.postMessage(messageEvent, '*'); - await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); + screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button)); - screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button)); - expect(previousSequenceHandler).toHaveBeenCalledTimes(2); - expect(unitNavigationHandler).not.toHaveBeenCalled(); - - screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button)); - expect(nextSequenceHandler).toHaveBeenCalledTimes(2); - expect(unitNavigationHandler).not.toHaveBeenCalled(); - - expect(sendTrackEvent).toHaveBeenNthCalledWith(1, 'edx.ui.lms.sequence.previous_selected', { - current_tab: 1, id: mockData.unitId, tab_count: 0, widget_placement: 'top', + expect(nextSequenceHandler).toHaveBeenCalledTimes(1); + expect(unitNavigationHandler).not.toHaveBeenCalled(); + expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.next_selected', { + current_tab: Number(unitId), + id: unitId, + tab_count: testUnits.length, + widget_placement: 'top', + }); }); - expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.previous_selected', { - current_tab: 1, id: mockData.unitId, tab_count: 0, widget_placement: 'bottom', - }); - expect(sendTrackEvent).toHaveBeenNthCalledWith(3, 'edx.ui.lms.sequence.next_selected', { - current_tab: 1, id: mockData.unitId, tab_count: 0, widget_placement: 'top', - }); - expect(sendTrackEvent).toHaveBeenNthCalledWith(4, 'edx.ui.lms.sequence.next_selected', { - current_tab: 1, id: mockData.unitId, tab_count: 0, widget_placement: 'bottom', - }); - }); - it('handles unit navigation button', () => { - sendTrackEvent.mockClear(); - const unitNavigationHandler = jest.fn(); - const targetUnit = '4'; - render(); + it('handles the navigation buttons for empty sequence', async () => { + sendTrackEvent.mockClear(); + const testState = cloneDeep(initialState); + testState.models.sequences['1'].unitIds = []; - fireEvent.click(screen.getByRole('button', { name: targetUnit })); - expect(unitNavigationHandler).toHaveBeenCalledWith(targetUnit); - expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.tab_selected', { - current_tab: Number(mockData.unitId), id: mockData.unitId, target_tab: Number(targetUnit), tab_count: testUnits.length, widget_placement: 'top', + const unitNavigationHandler = jest.fn(); + const previousSequenceHandler = jest.fn(); + const nextSequenceHandler = jest.fn(); + render(, { initialState: testState }); + window.postMessage(messageEvent, '*'); + await waitFor(() => expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument()); + + screen.getAllByRole('button', { name: /previous/i }).forEach(button => fireEvent.click(button)); + expect(previousSequenceHandler).toHaveBeenCalledTimes(2); + expect(unitNavigationHandler).not.toHaveBeenCalled(); + + screen.getAllByRole('button', { name: /next/i }).forEach(button => fireEvent.click(button)); + expect(nextSequenceHandler).toHaveBeenCalledTimes(2); + expect(unitNavigationHandler).not.toHaveBeenCalled(); + + expect(sendTrackEvent).toHaveBeenNthCalledWith(1, 'edx.ui.lms.sequence.previous_selected', { + current_tab: 1, + id: mockData.unitId, + tab_count: 0, + widget_placement: 'top', + }); + expect(sendTrackEvent).toHaveBeenNthCalledWith(2, 'edx.ui.lms.sequence.previous_selected', { + current_tab: 1, + id: mockData.unitId, + tab_count: 0, + widget_placement: 'bottom', + }); + expect(sendTrackEvent).toHaveBeenNthCalledWith(3, 'edx.ui.lms.sequence.next_selected', { + current_tab: 1, + id: mockData.unitId, + tab_count: 0, + widget_placement: 'top', + }); + expect(sendTrackEvent).toHaveBeenNthCalledWith(4, 'edx.ui.lms.sequence.next_selected', { + current_tab: 1, + id: mockData.unitId, + tab_count: 0, + widget_placement: 'bottom', + }); + }); + + it('handles unit navigation button', () => { + sendTrackEvent.mockClear(); + const unitNavigationHandler = jest.fn(); + const targetUnit = '4'; + render(); + + fireEvent.click(screen.getByRole('button', { name: targetUnit })); + expect(unitNavigationHandler).toHaveBeenCalledWith(targetUnit); + expect(sendTrackEvent).toHaveBeenCalledWith('edx.ui.lms.sequence.tab_selected', { + current_tab: Number(mockData.unitId), + id: mockData.unitId, + target_tab: Number(targetUnit), + tab_count: testUnits.length, + widget_placement: 'top', + }); }); }); }); diff --git a/src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx b/src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx index af397639..68e33c45 100644 --- a/src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx +++ b/src/courseware/course/sequence/sequence-navigation/SequenceNavigationDropdown.test.jsx @@ -61,5 +61,8 @@ describe('Sequence Navigation Dropdown', () => { screen.getAllByText(/^\d+$/).forEach(element => fireEvent.click(element)); expect(onNavigate).toHaveBeenCalledTimes(testUnits.length); + testUnits.forEach(unit => { + expect(onNavigate).toHaveBeenNthCalledWith(Number(unit), unit); + }); }); }); diff --git a/src/courseware/course/sequence/sequence-navigation/SequenceNavigationTabs.test.jsx b/src/courseware/course/sequence/sequence-navigation/SequenceNavigationTabs.test.jsx index c10e3577..4d45699d 100644 --- a/src/courseware/course/sequence/sequence-navigation/SequenceNavigationTabs.test.jsx +++ b/src/courseware/course/sequence/sequence-navigation/SequenceNavigationTabs.test.jsx @@ -1,5 +1,7 @@ import React from 'react'; -import { render, screen } from '../../../../setupTest'; +import { + initialState, render, screen, testUnits, +} from '../../../../setupTest'; import SequenceNavigationTabs from './SequenceNavigationTabs'; import useIndexOfLastVisibleChild from '../../../../tabs/useIndexOfLastVisibleChild'; @@ -7,22 +9,6 @@ import useIndexOfLastVisibleChild from '../../../../tabs/useIndexOfLastVisibleCh jest.mock('../../../../tabs/useIndexOfLastVisibleChild'); describe('Sequence Navigation Tabs', () => { - const testUnits = [...Array(10).keys()].map(i => String(i + 1)); - - const initialState = { - models: { - units: testUnits.reduce( - (acc, unitId) => Object.assign(acc, { - [unitId]: { - contentType: 'other', - title: unitId, - }, - }), - {}, - ), - }, - }; - const mockData = { unitId: '1', onNavigate: () => {