From 19f5181ecf259caf3bed7540b46d755c1d7f7a1c Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:17:41 -0400 Subject: [PATCH 1/9] fix: add onClose methods (dummies) to IconButton and AlertModal these 2 snapshots have anonymous functions... trying to decide if those should be made classes to notate the stub method... --- src/components/ConfirmModal.jsx | 1 + src/components/InfoPopover.jsx | 1 + src/components/__snapshots__/ConfirmModal.test.jsx.snap | 2 ++ src/components/__snapshots__/InfoPopover.test.jsx.snap | 1 + 4 files changed, 5 insertions(+) diff --git a/src/components/ConfirmModal.jsx b/src/components/ConfirmModal.jsx index b60b214..1fdbb77 100644 --- a/src/components/ConfirmModal.jsx +++ b/src/components/ConfirmModal.jsx @@ -15,6 +15,7 @@ export const ConfirmModal = ({ ({})} isOpen={isOpen} footerNode={( diff --git a/src/components/InfoPopover.jsx b/src/components/InfoPopover.jsx index 9200474..653b8d1 100644 --- a/src/components/InfoPopover.jsx +++ b/src/components/InfoPopover.jsx @@ -29,6 +29,7 @@ export const InfoPopover = ({ children }) => ( src={InfoOutline} alt="criterion info" iconAs={Icon} + onClick={() => {}} /> ); diff --git a/src/components/__snapshots__/ConfirmModal.test.jsx.snap b/src/components/__snapshots__/ConfirmModal.test.jsx.snap index dad16c1..5b278c2 100644 --- a/src/components/__snapshots__/ConfirmModal.test.jsx.snap +++ b/src/components/__snapshots__/ConfirmModal.test.jsx.snap @@ -20,6 +20,7 @@ exports[`ConfirmModal snapshot: closed 1`] = ` } isOpen={false} + onClose={[Function]} title="test-title" >

@@ -48,6 +49,7 @@ exports[`ConfirmModal snapshot: open 1`] = ` } isOpen={true} + onClose={[Function]} title="test-title" >

diff --git a/src/components/__snapshots__/InfoPopover.test.jsx.snap b/src/components/__snapshots__/InfoPopover.test.jsx.snap index d1ae88f..50e8a37 100644 --- a/src/components/__snapshots__/InfoPopover.test.jsx.snap +++ b/src/components/__snapshots__/InfoPopover.test.jsx.snap @@ -21,6 +21,7 @@ exports[`Info Popover Component snapshot 1`] = ` alt="criterion info" className="criteria-help-icon" iconAs={[MockFunction Icon]} + onClick={[Function]} src={[MockFunction icons.InfoOutline]} /> From ed215a27b2fa7c4118ba5b4fafa867fb0660a7c7 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:20:16 -0400 Subject: [PATCH 2/9] fix: add child content to Hyperlink in breadcrumb --- src/containers/ListView/ListViewBreadcrumb.jsx | 8 +++++--- src/containers/ListView/ListViewBreadcrumb.test.jsx | 1 + .../__snapshots__/ListViewBreadcrumb.test.jsx.snap | 6 +++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/containers/ListView/ListViewBreadcrumb.jsx b/src/containers/ListView/ListViewBreadcrumb.jsx index 025135c..d19b5ac 100644 --- a/src/containers/ListView/ListViewBreadcrumb.jsx +++ b/src/containers/ListView/ListViewBreadcrumb.jsx @@ -2,8 +2,8 @@ import React from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; -import { ArrowBack } from '@edx/paragon/icons'; -import { Hyperlink } from '@edx/paragon'; +import { ArrowBack, Launch } from '@edx/paragon/icons'; +import { Hyperlink, Icon } from '@edx/paragon'; import selectors from 'data/selectors'; import { locationId } from 'data/constants/app'; @@ -20,7 +20,9 @@ export const ListViewBreadcrumb = ({ courseId, oraName }) => (

{oraName} - + + +

); diff --git a/src/containers/ListView/ListViewBreadcrumb.test.jsx b/src/containers/ListView/ListViewBreadcrumb.test.jsx index 0b8ca97..dcd4812 100644 --- a/src/containers/ListView/ListViewBreadcrumb.test.jsx +++ b/src/containers/ListView/ListViewBreadcrumb.test.jsx @@ -16,6 +16,7 @@ import { jest.mock('@edx/paragon', () => ({ Hyperlink: () => 'Hyperlink', + Icon: () => 'Icon', })); jest.mock('data/selectors', () => ({ diff --git a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap index 030b5f0..4a97ff4 100644 --- a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap +++ b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap @@ -18,7 +18,11 @@ exports[`ListViewBreadcrumb component component snapshot: empty (no list data) 1 + > + +

`; From 89ec764e2c46c43a5d6198dd1871d665e403a9a3 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:20:31 -0400 Subject: [PATCH 3/9] feat: add helper classes to navigation buttons --- src/containers/ListView/__snapshots__/index.test.jsx.snap | 1 + src/containers/ListView/index.jsx | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/containers/ListView/__snapshots__/index.test.jsx.snap b/src/containers/ListView/__snapshots__/index.test.jsx.snap index 4cfdd76..c14a914 100644 --- a/src/containers/ListView/__snapshots__/index.test.jsx.snap +++ b/src/containers/ListView/__snapshots__/index.test.jsx.snap @@ -110,6 +110,7 @@ exports[`ListView component component render tests snapshots snapshot: happy pat Array [ Object { "buttonText": "View all responses", + "className": "view-all-responses-btn", "handleClick": [MockFunction this.handleViewAllResponsesClick], "variant": "primary", }, diff --git a/src/containers/ListView/index.jsx b/src/containers/ListView/index.jsx index 3328316..29a65fa 100644 --- a/src/containers/ListView/index.jsx +++ b/src/containers/ListView/index.jsx @@ -58,6 +58,7 @@ export class ListView extends React.Component { selectedBulkAction(selectedFlatRows) { return { buttonText: `View selected responses (${selectedFlatRows.length})`, + className: 'view-selected-responses-btn', handleClick: this.handleViewAllResponsesClick, variant: 'primary', }; @@ -86,6 +87,7 @@ export class ListView extends React.Component { { buttonText: 'View all responses', handleClick: this.handleViewAllResponsesClick, + className: 'view-all-responses-btn', variant: 'primary', }, ]} From eccc4e36a3bfac19b03cd989beb72d35a45bd1a2 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:21:08 -0400 Subject: [PATCH 4/9] fix: only load response from current into neighbor when navigating --- src/data/reducers/grading.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/data/reducers/grading.js b/src/data/reducers/grading.js index 1181614..c7636ca 100644 --- a/src/data/reducers/grading.js +++ b/src/data/reducers/grading.js @@ -100,7 +100,7 @@ const app = createReducer(initialState, { [actions.grading.preloadPrev]: (state, { payload }) => ({ ...state, prev: payload }), [actions.grading.loadNext]: (state, { payload }) => ({ ...state, - prev: state.current, + prev: { response: state.current.response }, current: { response: state.next.response, ...payload }, activeIndex: state.activeIndex + 1, gradeData: { @@ -111,7 +111,7 @@ const app = createReducer(initialState, { }), [actions.grading.loadPrev]: (state, { payload }) => ({ ...state, - next: state.current, + next: { response: state.current.response }, current: { response: state.prev.response, ...payload }, gradeData: { ...state.gradeData, From 39047272cabf313c2747f210977d21e28378f77c Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:26:19 -0400 Subject: [PATCH 5/9] feat: split cancel and stop grading actions --- src/data/thunkActions/grading.js | 17 ++++++++++++++- src/data/thunkActions/grading.test.js | 31 ++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/data/thunkActions/grading.js b/src/data/thunkActions/grading.js index 3fdd326..aa21244 100644 --- a/src/data/thunkActions/grading.js +++ b/src/data/thunkActions/grading.js @@ -140,7 +140,21 @@ export const startGrading = () => (dispatch, getState) => { }; /** - * Stops the grading process for the current submisison + * Cancels the grading process for the current submisison. + * Releases the lock and dispatches stopGrading on success. + */ +export const cancelGrading = () => (dispatch, getState) => { + dispatch(requests.setLock({ + value: false, + submissionId: selectors.grading.selected.submissionId(getState()), + onSuccess: () => { + dispatch(module.stopGrading()); + }, + })); +}; + +/** + * Stops the grading process for the current submission (local only) * Clears the local grade data for the current submission and sets grading state * to False */ @@ -154,5 +168,6 @@ export default StrictDict({ loadNext, loadPrev, startGrading, + cancelGrading, stopGrading, }); diff --git a/src/data/thunkActions/grading.test.js b/src/data/thunkActions/grading.test.js index 6cfccc3..1fa8355 100644 --- a/src/data/thunkActions/grading.test.js +++ b/src/data/thunkActions/grading.test.js @@ -313,9 +313,38 @@ describe('grading thunkActions', () => { }); }); + describe('cancelGrading', () => { + let stopGrading; + beforeAll(() => { + stopGrading = thunkActions.stopGrading; + thunkActions.stopGrading = () => 'stop grading'; + }); + beforeEach(() => { + getDispatched(thunkActions.cancelGrading()); + actionArgs = dispatched.setLock; + }); + afterAll(() => { + thunkActions.stopGrading = stopGrading; + }); + test('dispatches setLock with selected submissionId and value: false', () => { + expect(actionArgs).not.toEqual(undefined); + expect(actionArgs.value).toEqual(false); + expect(actionArgs.submissionId).toEqual(selectors.grading.selected.submissionId(testState)); + }); + describe('onSuccess', () => { + beforeEach(() => { + dispatch.mockClear(); + }); + test('dispatches stopGrading thunkAction', () => { + actionArgs.onSuccess(); + expect(dispatch.mock.calls).toContainEqual([thunkActions.stopGrading()]); + }); + }); + }); + describe('stopGrading', () => { it('dispatches grading.clearGrade and app.setGrading(false)', () => { - thunkActions.stopGrading()(dispatch); + thunkActions.stopGrading()(dispatch, getState); expect(dispatch.mock.calls).toEqual([ [actions.grading.clearGrade()], [actions.app.setGrading(false)], From 955fea510e5084f5967b98c6d05d664079f14ffb Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 12:26:23 -0400 Subject: [PATCH 6/9] feat: integration tests --- src/test/app.test.jsx | 249 ++++++++++++++++++++++++++++++++++++++++++ src/test/utils.js | 7 ++ 2 files changed, 256 insertions(+) create mode 100644 src/test/app.test.jsx create mode 100644 src/test/utils.js diff --git a/src/test/app.test.jsx b/src/test/app.test.jsx new file mode 100644 index 0000000..d9a75ce --- /dev/null +++ b/src/test/app.test.jsx @@ -0,0 +1,249 @@ +import React from 'react'; +import * as redux from 'redux'; +import { Provider } from 'react-redux'; +import { + act, + render, + waitFor, + within, +} from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import thunk from 'redux-thunk'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; + +import fakeData from 'data/services/lms/fakeData'; +import api from 'data/services/lms/api'; +import reducers from 'data/reducers'; +import { gradingStatuses } from 'data/services/lms/constants'; +import messages from 'i18n'; + +import App from 'App'; + +jest.mock('@edx/frontend-platform/auth', () => ({ + getAuthenticatedHttpClient: jest.fn(), + getLoginRedirectUrl: jest.fn(), +})); + +const configureStore = () => redux.createStore( + reducers, + redux.compose(redux.applyMiddleware(thunk)), +); + +let el; +let store; +let state; + +/** + * Simple wrapper for updating the top-level state variable, that also returns the new value + * @return {obj} - current redux store state + */ +const getState = () => { + state = store.getState(); + return state; +}; + +/** Fake Data for quick access */ +const submissionIds = [ + fakeData.ids.submissionId(0), + fakeData.ids.submissionId(1), + fakeData.ids.submissionId(2), + fakeData.ids.submissionId(3), + fakeData.ids.submissionId(4), +]; +const submissions = submissionIds.map(id => fakeData.mockSubmission(id)); +const responses = submissions.map(({ response }) => response); +const statuses = submissionIds.map(id => fakeData.mockSubmissionStatus(id)); + +const resolveFns = {}; +/** + * Mock the api with jest functions that can be tested against. + */ +const mockApi = () => { + api.initializeApp = jest.fn(() => new Promise( + (resolve) => { + resolveFns.initialize = () => resolve({ + oraMetadata: fakeData.oraMetadata, + courseMetadata: fakeData.courseMetadata, + submissions: fakeData.submissions, + }); + }, + )); + api.fetchSubmission = jest.fn((submissionId) => new Promise( + (resolve) => resolve(fakeData.mockSubmission(submissionId)), + )); + api.fetchSubmissionStatus = jest.fn((submissionId) => new Promise( + (resolve) => resolve(fakeData.mockSubmissionStatus(submissionId)), + )); + api.fetchSubmissionResponse = jest.fn((submissionId) => new Promise( + (resolve) => resolve({ response: fakeData.mockSubmission(submissionId).response }), + )); +}; + +/** + * load and configure the store, render the element, and populate the top-level state object + */ +const renderEl = async () => { + store = configureStore(); + el = await render( + + + + + , + ); + getState(); +}; + +/** + * resolve the initalization promise, and update state object + */ +const initialize = async () => { + resolveFns.initialize(); + await act(() => el.findByText(fakeData.ids.username(0))); + getState(); +}; + +/** + * Select the first 5 entries in the table and click the "View Selected Responses" button + * Wait for the review page to show and update the top-level state object. + */ +const makeTableSelections = async () => { + const table = el.getByRole('table'); + const rows = table.querySelectorAll('tbody tr'); + const checkbox = (index) => within(rows.item(index)).getByTitle('Toggle Row Selected'); + const clickIndex = (index) => userEvent.click(checkbox(index)); + [0, 1, 2, 3, 4].forEach(clickIndex); + userEvent.click(el.container.querySelector('.view-selected-responses-btn')); + await act(() => el.findByText('Show Rubric')); + getState(); +}; + +/** + * Click the "next" button in review modal + */ +const clickNext = async () => { + userEvent.click(el.getByLabelText('Load next submission')); +}; + +/** + * Click the "next" button in review modal + */ +const clickPrev = async () => { + userEvent.click(el.getByLabelText('Load previous submission')); +}; + +/** + * Wait for the prev and next values to be populated based on the current selection index + */ +const waitForNeighbors = async (currentIndex) => { + await waitFor( + () => { + const { prev, next } = getState().grading; + expect(prev).toEqual( + (currentIndex > 0) ? { response: responses[currentIndex - 1] } : null, + ); + expect(next).toEqual( + (currentIndex < 4) ? { response: responses[currentIndex + 1] } : null, + ); + }, + ); +}; + +/** + * Wait for neighbors, and then verify prev, current, and next grading fields have the appropriate + * data. Also ensure that the app is "grading" iff the "current" response's lockStatus is inProgress. + */ +const checkLoadedResponses = async (currentIndex) => { + await waitForNeighbors(currentIndex); + const { prev, current, next } = state.grading; + expect({ prev, current, next }).toEqual({ + prev: currentIndex > 0 ? ({ response: responses[currentIndex - 1] }) : null, + current: { + submissionId: submissionIds[currentIndex], + response: submissions[currentIndex].response, + ...statuses[currentIndex], + }, + next: currentIndex < 4 ? ({ response: responses[currentIndex + 1] }) : null, + }); + expect(state.app.showReview).toEqual(true); + const shouldBeGrading = statuses[currentIndex].lockStatus === gradingStatuses.inProgress; + expect(state.app.isGrading).toEqual(shouldBeGrading); +}; + +describe('ESG app integration tests', () => { + beforeAll(() => mockApi()); + + test('initialState', async () => { + await renderEl(); + expect(state.app).toEqual(jest.requireActual('data/reducers/app').initialState); + expect(state.submissions).toEqual( + jest.requireActual('data/reducers/submissions').initialState, + ); + expect(state.grading).toEqual(jest.requireActual('data/reducers/grading').initialState); + }); + + test('initialization', async () => { + await renderEl(); + await initialize(); + expect(state.app.courseMetadata).toEqual(fakeData.courseMetadata); + expect(state.app.oraMetadata).toEqual(fakeData.oraMetadata); + expect(state.submissions.allSubmissions).toEqual(fakeData.submissions); + }); + + describe('table selection', () => { + beforeAll(async () => { + await renderEl(); + await initialize(); + await makeTableSelections(); + }); + it('loads selected submission ids', () => { + expect(state.grading.selected).toEqual(submissionIds); + }); + test('app flags, { showReview: true, isGrading: false, showRubric: false }', () => { + expect(state.app.showReview).toEqual(true); + expect(state.app.isGrading).toEqual(false); + expect(state.app.showRubric).toEqual(false); + }); + it('loads current submission', () => { + const submissionId = fakeData.ids.submissionId(0); + expect(state.grading.current).toEqual({ + submissionId, + ...fakeData.mockSubmission(submissionId), + }); + }); + it('loads response for next submission', () => { + expect(state.grading.next).toEqual({ + response: fakeData.mockSubmission(submissionIds[1]).response, + }); + }); + }); + + describe('review navigation', () => { + test('loads full submission for current, and response for neighbors when navigating', async () => { + await renderEl(); + await initialize(); + await makeTableSelections(); + await clickNext(); + await checkLoadedResponses(1); + await clickNext(); + await checkLoadedResponses(2); + await clickPrev(); + await checkLoadedResponses(1); + await clickNext(); + await checkLoadedResponses(2); + await clickNext(); + await checkLoadedResponses(3); + await clickNext(); + await checkLoadedResponses(4); + await clickPrev(); + await checkLoadedResponses(3); + await clickPrev(); + await checkLoadedResponses(2); + await clickPrev(); + await checkLoadedResponses(1); + await clickPrev(); + await checkLoadedResponses(0); + }); + }); +}); diff --git a/src/test/utils.js b/src/test/utils.js new file mode 100644 index 0000000..763259a --- /dev/null +++ b/src/test/utils.js @@ -0,0 +1,7 @@ +export const mockSuccess = (returnValFn) => (...args) => ( + new Promise((resolve) => resolve(returnValFn(...args))) +); + +export const mockFailure = (returnValFn) => (...args) => ( + new Promise((resolve, reject) => reject(returnValFn(...args))) +); From fcdbc5034e03b02ffa426440f9d21fd8a7b37b11 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 15:34:49 -0400 Subject: [PATCH 7/9] fix: update snapshot mocking --- src/containers/ListView/ListViewBreadcrumb.test.jsx | 4 ++++ .../ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/containers/ListView/ListViewBreadcrumb.test.jsx b/src/containers/ListView/ListViewBreadcrumb.test.jsx index dcd4812..28da8cb 100644 --- a/src/containers/ListView/ListViewBreadcrumb.test.jsx +++ b/src/containers/ListView/ListViewBreadcrumb.test.jsx @@ -18,6 +18,10 @@ jest.mock('@edx/paragon', () => ({ Hyperlink: () => 'Hyperlink', Icon: () => 'Icon', })); +jest.mock('@edx/paragon/icons', () => ({ + ArrowBack: 'icons.ArrowBack', + Launch: 'icons.Launch', +})); jest.mock('data/selectors', () => ({ __esModule: true, diff --git a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap index 4a97ff4..8faf79a 100644 --- a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap +++ b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap @@ -6,7 +6,7 @@ exports[`ListViewBreadcrumb component component snapshot: empty (no list data) 1 className="py-4" destination="openResponseUrl(test-course-id)" > - Back to all open responses @@ -20,7 +20,7 @@ exports[`ListViewBreadcrumb component component snapshot: empty (no list data) 1 target="_blank" >

From 2001ce57b04db9febaba6fe3972a3aa36576bd9c Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 15:38:14 -0400 Subject: [PATCH 8/9] feat: add gitattributes to stop hiding snapshots --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..44f4f7b --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +*.snap linguist-generated=false From e288579ccbc5288ce5bb36374a077b5104a00f20 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 26 Oct 2021 15:41:27 -0400 Subject: [PATCH 9/9] fix: clean up ListViewBreadcrumbs snapshot --- src/containers/ListView/ListViewBreadcrumb.jsx | 2 +- .../ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/containers/ListView/ListViewBreadcrumb.jsx b/src/containers/ListView/ListViewBreadcrumb.jsx index d19b5ac..707a0fa 100644 --- a/src/containers/ListView/ListViewBreadcrumb.jsx +++ b/src/containers/ListView/ListViewBreadcrumb.jsx @@ -15,7 +15,7 @@ import urls from 'data/services/lms/urls'; export const ListViewBreadcrumb = ({ courseId, oraName }) => ( <> - + Back to all open responses

diff --git a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap index 8faf79a..3030053 100644 --- a/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap +++ b/src/containers/ListView/__snapshots__/ListViewBreadcrumb.test.jsx.snap @@ -6,8 +6,9 @@ exports[`ListViewBreadcrumb component component snapshot: empty (no list data) 1 className="py-4" destination="openResponseUrl(test-course-id)" > - Back to all open responses