diff --git a/src/components/GradesView/ImportResultToast/hooks.js b/src/components/GradesView/ImportResultToast/hooks.js new file mode 100644 index 00000000..6108c384 --- /dev/null +++ b/src/components/GradesView/ImportResultToast/hooks.js @@ -0,0 +1,62 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; + +import { actions, selectors } from 'data/redux/hooks'; +import { views } from 'data/constants/app'; +import messages from './messages'; + +/** + * + * Reports the outcome of a grade upload, whether it succeeded or failed, and links to the + * Bulk Management History tab. + */ +export const useImportResultToastData = () => { + const { formatMessage } = useIntl(); + + const showSuccess = selectors.app.useShowImportSuccessToast(); + const showError = selectors.app.useShowImportErrorToast(); + const details = selectors.grades.useBulkImportErrorMessages(); + const setAppView = actions.app.useSetView(); + const setShowSuccess = actions.app.useSetShowImportSuccessToast(); + const setShowError = actions.app.useSetShowImportErrorToast(); + + // A failure needs its message to say anything useful, so it is only reportable once both + // are present. A flagged failure suppresses the success claim either way: with no message + // there is nothing to show, but reporting success alongside a known failure is the one + // outcome never worth risking. + const isError = !!showError && !!details; + const isSuccess = !showError && !!showSuccess; + + // Each outcome requires its own flag, so neither can borrow the other's wording. Starting + // a second upload clears both while this toast is still fading, and an empty string is + // the right thing to render for the instant it takes to leave the screen. + let description = ''; + if (isError) { + description = formatMessage(messages.errorDescription, { details }); + } else if (isSuccess) { + description = formatMessage(messages.successDescription); + } + + const hide = () => { + setShowSuccess(false); + setShowError(false); + }; + + return { + action: { + label: formatMessage(messages.showHistoryViewBtn), + onClick: () => { + setAppView(views.bulkManagementHistory); + hide(); + }, + }, + onClose: hide, + show: isError || isSuccess, + // Paragon defaults its Toast to auto-dismissing, and spreads extra props after that, + // so this reaches react-bootstrap. A failure waits to be dismissed; a success asks + // nothing of the reader and can time out as usual. + autohide: !isError, + description, + }; +}; + +export default useImportResultToastData; diff --git a/src/components/GradesView/ImportResultToast/hooks.test.js b/src/components/GradesView/ImportResultToast/hooks.test.js new file mode 100644 index 00000000..05098830 --- /dev/null +++ b/src/components/GradesView/ImportResultToast/hooks.test.js @@ -0,0 +1,181 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; + +import { views } from 'data/constants/app'; +import { actions, selectors } from 'data/redux/hooks'; + +import useImportResultToastData from './hooks'; +import messages from './messages'; + +jest.mock('data/redux/hooks', () => ({ + actions: { + app: { + useSetView: jest.fn(), + useSetShowImportSuccessToast: jest.fn(), + useSetShowImportErrorToast: jest.fn(), + }, + }, + selectors: { + app: { + useShowImportSuccessToast: jest.fn(), + useShowImportErrorToast: jest.fn(), + }, + grades: { useBulkImportErrorMessages: jest.fn() }, + }, +})); + +jest.mock('react', () => ({ + ...jest.requireActual('react'), + useContext: jest.fn((context) => context), +})); + +jest.mock('@edx/frontend-platform/i18n', () => ({ + ...jest.requireActual('@edx/frontend-platform/i18n'), + useIntl: jest.fn(() => ({ + formatMessage: (message, values) => ( + values ? message.defaultMessage.replace('{details}', values.details) : message.defaultMessage + ), + })), +})); + +const setView = jest.fn().mockName('hooks.setView'); +const setShowSuccess = jest.fn().mockName('hooks.setShowImportSuccessToast'); +const setShowError = jest.fn().mockName('hooks.setShowImportErrorToast'); + +const DETAILS = 'No grades were changed.'; + +/** Put redux in one of the states the hook can see. */ +const mockState = ({ success = false, error = false, details = '' } = {}) => { + actions.app.useSetView.mockReturnValue(setView); + actions.app.useSetShowImportSuccessToast.mockReturnValue(setShowSuccess); + actions.app.useSetShowImportErrorToast.mockReturnValue(setShowError); + selectors.app.useShowImportSuccessToast.mockReturnValue(success); + selectors.app.useShowImportErrorToast.mockReturnValue(error); + selectors.grades.useBulkImportErrorMessages.mockReturnValue(details); +}; + +describe('ImportResultToast hooks', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockState(); + }); + + it('initializes intl and redux hooks', () => { + useImportResultToastData(); + expect(useIntl).toHaveBeenCalledWith(); + expect(selectors.app.useShowImportSuccessToast).toHaveBeenCalled(); + expect(selectors.app.useShowImportErrorToast).toHaveBeenCalled(); + expect(selectors.grades.useBulkImportErrorMessages).toHaveBeenCalled(); + expect(actions.app.useSetView).toHaveBeenCalled(); + expect(actions.app.useSetShowImportSuccessToast).toHaveBeenCalled(); + expect(actions.app.useSetShowImportErrorToast).toHaveBeenCalled(); + }); + + describe('a successful import', () => { + it('shows the success message and lets it time out', () => { + mockState({ success: true }); + const out = useImportResultToastData(); + expect(out.show).toBe(true); + expect(out.description).toEqual(messages.successDescription.defaultMessage); + expect(out.autohide).toBe(true); + }); + }); + + describe('a failed import', () => { + it('shows the message the server returned and waits to be dismissed', () => { + mockState({ error: true, details: DETAILS }); + const out = useImportResultToastData(); + expect(out.show).toBe(true); + expect(out.description).toEqual(`Import failed. ${DETAILS}`); + // A failure must not disappear on its own; the reader has to act on it. + expect(out.autohide).toBe(false); + }); + + it('takes precedence if a success is somehow flagged too', () => { + mockState({ success: true, error: true, details: DETAILS }); + expect(useImportResultToastData().description).toEqual(`Import failed. ${DETAILS}`); + }); + }); + + describe('nothing to report', () => { + it('stays hidden with no message', () => { + const out = useImportResultToastData(); + expect(out.show).toBe(false); + expect(out.description).toEqual(''); + }); + + it('stays hidden when a failure is flagged with no message to show', () => { + mockState({ error: true, details: '' }); + expect(useImportResultToastData().show).toBe(false); + }); + + it('never borrows the other outcome\'s wording', () => { + // Starting another upload clears both flags while the toast is still fading. Neither + // outcome may inherit the other's message on the way off screen. + mockState({ error: true, details: DETAILS }); + expect(useImportResultToastData().description).toEqual(`Import failed. ${DETAILS}`); + + mockState(); + const fading = useImportResultToastData(); + expect(fading.show).toBe(false); + expect(fading.description).toEqual(''); + expect(fading.description).not.toEqual(messages.successDescription.defaultMessage); + }); + }); + + describe('every combination of the state it can see', () => { + // Exhaustive, because the one outcome never worth risking is claiming success while a + // failure is flagged -- which is the bug this component exists to prevent. + const SUCCESS = messages.successDescription.defaultMessage; + const FAILURE = `Import failed. ${DETAILS}`; + const cases = [ + // showSuccess, showError, details, show, description + [false, false, '', false, ''], + [false, false, DETAILS, false, ''], + [true, false, '', true, SUCCESS], + [true, false, DETAILS, true, SUCCESS], + [false, true, '', false, ''], + [false, true, DETAILS, true, FAILURE], + [true, true, '', false, ''], + [true, true, DETAILS, true, FAILURE], + ]; + + test.each(cases)( + 'success=%p error=%p details=%p -> show=%p', + (success, error, details, show, description) => { + mockState({ success, error, details }); + const out = useImportResultToastData(); + expect(out.show).toBe(show); + expect(out.description).toEqual(description); + }, + ); + + it('never claims success while a failure is flagged', () => { + cases + .filter(([, error]) => error) + .forEach(([success, error, details]) => { + mockState({ success, error, details }); + expect(useImportResultToastData().description).not.toEqual(SUCCESS); + }); + }); + }); + + describe('dismissing', () => { + it('onClose clears both flags', () => { + useImportResultToastData().onClose(); + expect(setShowSuccess).toHaveBeenCalledWith(false); + expect(setShowError).toHaveBeenCalledWith(false); + }); + + it('the action opens the history view and clears both flags', () => { + useImportResultToastData().action.onClick(); + expect(setView).toHaveBeenCalledWith(views.bulkManagementHistory); + expect(setShowSuccess).toHaveBeenCalledWith(false); + expect(setShowError).toHaveBeenCalledWith(false); + }); + + it('labels the action button', () => { + expect(useImportResultToastData().action.label) + .toEqual(messages.showHistoryViewBtn.defaultMessage); + }); + }); +}); diff --git a/src/components/GradesView/ImportResultToast/index.jsx b/src/components/GradesView/ImportResultToast/index.jsx new file mode 100644 index 00000000..f1e786f1 --- /dev/null +++ b/src/components/GradesView/ImportResultToast/index.jsx @@ -0,0 +1,35 @@ +import React from 'react'; + +import { Toast } from '@openedx/paragon'; + +import useImportResultToastData from './hooks'; + +/** + * + * Toast component triggered by a grade upload, reporting either that it succeeded or that + * it failed -- rejected by the server, or applied no grades at all. + * Provides a link to view the Bulk Management History tab. + */ +export const ImportResultToast = () => { + const { + action, + onClose, + show, + autohide, + description, + } = useImportResultToastData(); + return ( + + {description} + + ); +}; + +ImportResultToast.propTypes = {}; + +export default ImportResultToast; diff --git a/src/components/GradesView/ImportResultToast/index.test.jsx b/src/components/GradesView/ImportResultToast/index.test.jsx new file mode 100644 index 00000000..d81344d3 --- /dev/null +++ b/src/components/GradesView/ImportResultToast/index.test.jsx @@ -0,0 +1,73 @@ +import React from 'react'; + +import { render, initializeMocks, screen } from 'testUtilsExtra'; + +import ImportResultToast from '.'; +import useImportResultToastData from './hooks'; + +jest.mock('data/redux/hooks', () => ({ + actions: { + app: { + useSetView: jest.fn(), + useSetShowImportSuccessToast: jest.fn(), + useSetShowImportErrorToast: jest.fn(), + }, + }, + selectors: { + app: { + useShowImportSuccessToast: jest.fn(), + useShowImportErrorToast: jest.fn(), + }, + grades: { useBulkImportErrorMessages: jest.fn() }, + }, +})); + +jest.mock('./hooks', () => jest.fn()); + +initializeMocks(); + +const SUCCESS = 'Import Successful! Grades will be updated momentarily.'; +const FAILURE = 'Import failed. No grades were changed.'; + +const mockData = (overrides = {}) => useImportResultToastData.mockReturnValue({ + action: { label: 'View Activity Log', onClick: jest.fn() }, + onClose: jest.fn(), + show: true, + autohide: true, + description: SUCCESS, + ...overrides, +}); + +describe('ImportResultToast', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders the toast container but no message with show false', () => { + mockData({ show: false, description: '' }); + render(); + const toastRoot = document.getElementById('toast-root'); + expect(toastRoot).toBeInTheDocument(); + expect(toastRoot).toHaveClass('toast-container'); + expect(screen.queryByText(SUCCESS)).toBeNull(); + expect(useImportResultToastData).toHaveBeenCalled(); + }); + + it('shows the success message', () => { + mockData(); + render(); + expect(screen.getByText(SUCCESS)).toBeInTheDocument(); + }); + + it('shows the failure message', () => { + mockData({ description: FAILURE, autohide: false }); + render(); + expect(screen.getByText(FAILURE)).toBeInTheDocument(); + }); + + it('renders the action button', () => { + mockData(); + render(); + expect(screen.getByText('View Activity Log')).toBeInTheDocument(); + }); +}); diff --git a/src/components/GradesView/ImportSuccessToast/messages.js b/src/components/GradesView/ImportResultToast/messages.js similarity index 59% rename from src/components/GradesView/ImportSuccessToast/messages.js rename to src/components/GradesView/ImportResultToast/messages.js index 90791f6c..f8eef34f 100644 --- a/src/components/GradesView/ImportSuccessToast/messages.js +++ b/src/components/GradesView/ImportResultToast/messages.js @@ -1,11 +1,18 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; +// The success ids keep their original `ImportSuccessToast` names: they are already in the +// translation pipeline, and renaming an id orphans every translation of it. const messages = defineMessages({ - description: { + successDescription: { id: 'gradebook.GradesView.ImportSuccessToast.description', defaultMessage: 'Import Successful! Grades will be updated momentarily.', description: 'A message congratulating a successful Import of grades', }, + errorDescription: { + id: 'gradebook.GradesView.ImportErrorToast.description', + defaultMessage: 'Import failed. {details}', + description: 'Message shown when a grade import could not be applied', + }, showHistoryViewBtn: { id: 'gradebook.GradesView.ImportSuccessToast.showHistoryViewBtn', defaultMessage: 'View Activity Log', diff --git a/src/components/GradesView/ImportSuccessToast/hooks.js b/src/components/GradesView/ImportSuccessToast/hooks.js deleted file mode 100644 index 75db5507..00000000 --- a/src/components/GradesView/ImportSuccessToast/hooks.js +++ /dev/null @@ -1,39 +0,0 @@ -import { useIntl } from '@edx/frontend-platform/i18n'; - -import { actions, selectors } from 'data/redux/hooks'; -import { views } from 'data/constants/app'; -import messages from './messages'; - -/** - * - * Toast component triggered by successful grade upload. - * Provides a link to view the Bulk Management History tab. - */ -export const useImportSuccessToastData = () => { - const { formatMessage } = useIntl(); - - const show = selectors.app.useShowImportSuccessToast(); - const setAppView = actions.app.useSetView(); - const setShow = actions.app.useSetShowImportSuccessToast(); - - const onClose = () => { - setShow(false); - }; - - const handleShowHistoryView = () => { - setAppView(views.bulkManagementHistory); - setShow(false); - }; - - return { - action: { - label: formatMessage(messages.showHistoryViewBtn), - onClick: handleShowHistoryView, - }, - onClose, - show, - description: formatMessage(messages.description), - }; -}; - -export default useImportSuccessToastData; diff --git a/src/components/GradesView/ImportSuccessToast/hooks.test.js b/src/components/GradesView/ImportSuccessToast/hooks.test.js deleted file mode 100644 index faa98478..00000000 --- a/src/components/GradesView/ImportSuccessToast/hooks.test.js +++ /dev/null @@ -1,70 +0,0 @@ -import { useIntl } from '@edx/frontend-platform/i18n'; - -import { formatMessage } from 'testUtils'; -import { views } from 'data/constants/app'; -import { actions, selectors } from 'data/redux/hooks'; - -import useImportSuccessToastData from './hooks'; -import messages from './messages'; - -jest.mock('data/redux/hooks', () => ({ - actions: { - app: { - useSetView: jest.fn(), - useSetShowImportSuccessToast: jest.fn(), - }, - }, - selectors: { - app: { useShowImportSuccessToast: jest.fn() }, - }, -})); - -jest.mock('react', () => ({ - ...jest.requireActual('react'), - useContext: jest.fn(context => context), -})); - -jest.mock('@edx/frontend-platform/i18n', () => ({ - ...jest.requireActual('@edx/frontend-platform/i18n'), - useIntl: jest.fn(() => ({ - formatMessage: (message) => message.defaultMessage, - })), -})); - -const setView = jest.fn().mockName('hooks.setView'); -const setShowToast = jest.fn().mockName('hooks.setShowImportSuccessToast'); -actions.app.useSetView.mockReturnValue(setView); -actions.app.useSetShowImportSuccessToast.mockReturnValue(setShowToast); -const showImportSuccessToast = 'test-show-import-success-toast'; -selectors.app.useShowImportSuccessToast.mockReturnValue(showImportSuccessToast); - -let out; -describe('ImportSuccessToast component', () => { - beforeAll(() => { - out = useImportSuccessToastData(); - }); - describe('behavior', () => { - it('initializes intl hook', () => { - expect(useIntl).toHaveBeenCalledWith(); - }); - it('initializes redux hooks', () => { - expect(selectors.app.useShowImportSuccessToast).toHaveBeenCalled(); - expect(actions.app.useSetView).toHaveBeenCalled(); - expect(actions.app.useSetShowImportSuccessToast).toHaveBeenCalled(); - }); - }); - describe('output', () => { - test('action label', () => { - expect(out.action.label).toEqual(formatMessage(messages.showHistoryViewBtn)); - }); - test('action click event', () => { - out.action.onClick(); - expect(setView).toHaveBeenCalledWith(views.bulkManagementHistory); - expect(setShowToast).toHaveBeenCalledWith(false); - }); - test('onClose', () => { - out.onClose(); - expect(setShowToast).toHaveBeenCalledWith(false); - }); - }); -}); diff --git a/src/components/GradesView/ImportSuccessToast/index.jsx b/src/components/GradesView/ImportSuccessToast/index.jsx deleted file mode 100644 index 01cab726..00000000 --- a/src/components/GradesView/ImportSuccessToast/index.jsx +++ /dev/null @@ -1,28 +0,0 @@ -import React from 'react'; - -import { Toast } from '@openedx/paragon'; - -import useImportSuccessToastData from './hooks'; - -/** - * - * Toast component triggered by successful grade upload. - * Provides a link to view the Bulk Management History tab. - */ -export const ImportSuccessToast = () => { - const { - action, - onClose, - show, - description, - } = useImportSuccessToastData(); - return ( - - {description} - - ); -}; - -ImportSuccessToast.propTypes = {}; - -export default ImportSuccessToast; diff --git a/src/components/GradesView/ImportSuccessToast/index.test.jsx b/src/components/GradesView/ImportSuccessToast/index.test.jsx deleted file mode 100644 index 59c8936d..00000000 --- a/src/components/GradesView/ImportSuccessToast/index.test.jsx +++ /dev/null @@ -1,88 +0,0 @@ -import React from 'react'; - -import { render, initializeMocks, screen } from 'testUtilsExtra'; - -import ImportSuccessToast from '.'; -import useImportSuccessToastData from './hooks'; - -jest.mock('data/redux/hooks', () => ({ - actions: { - app: { - useSetView: jest.fn(), - useSetShowImportSuccessToast: jest.fn(), - }, - }, - selectors: { - app: { - useShowImportSuccessToast: jest.fn(), - }, - }, -})); - -jest.mock('./hooks', () => jest.fn()); - -initializeMocks(); - -describe('ImportSuccessToast', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('renders with show false', () => { - useImportSuccessToastData.mockReturnValue({ - action: { - label: 'View Activity Log', - onClick: jest.fn(), - }, - onClose: jest.fn(), - show: false, - description: 'Import Successful! Grades will be updated momentarily.', - }); - - render(); - - const toastRoot = document.getElementById('toast-root'); - expect(toastRoot).toBeInTheDocument(); - expect(toastRoot).toHaveClass('toast-container'); - - const toastMessage = screen.queryByText('Import Successful! Grades will be updated momentarily.'); - expect(toastMessage).toBeNull(); - expect(useImportSuccessToastData).toHaveBeenCalled(); - }); - - it('renders with show true', () => { - useImportSuccessToastData.mockReturnValue({ - action: { - label: 'View Activity Log', - onClick: jest.fn(), - }, - onClose: jest.fn(), - show: true, - description: 'Import Successful! Grades will be updated momentarily.', - }); - - render(); - const toastMessage = screen.getByText('Import Successful! Grades will be updated momentarily.'); - expect(toastMessage).toBeInTheDocument(); - expect(useImportSuccessToastData).toHaveBeenCalled(); - }); - - it('passes correct props to Toast component', () => { - const mockOnClose = jest.fn(); - const mockOnClick = jest.fn(); - - useImportSuccessToastData.mockReturnValue({ - action: { - label: 'View Activity Log', - onClick: mockOnClick, - }, - onClose: mockOnClose, - show: true, - description: 'Import Successful! Grades will be updated momentarily.', - }); - - const { container } = render(); - expect(container).toBeInTheDocument(); - expect(useImportSuccessToastData).toHaveBeenCalled(); - }); -}); diff --git a/src/components/GradesView/index.jsx b/src/components/GradesView/index.jsx index 8e8f5ef6..30411a40 100644 --- a/src/components/GradesView/index.jsx +++ b/src/components/GradesView/index.jsx @@ -8,7 +8,7 @@ import FilterBadges from './FilterBadges'; import FilteredUsersLabel from './FilteredUsersLabel'; import FilterMenuToggle from './FilterMenuToggle'; import GradebookTable from './GradebookTable'; -import ImportSuccessToast from './ImportSuccessToast'; +import ImportResultToast from './ImportResultToast'; import InterventionsReport from './InterventionsReport'; import PageButtons from './PageButtons'; import ScoreViewInput from './ScoreViewInput'; @@ -57,7 +57,7 @@ export const GradesView = ({ updateQueryParams }) => {

* {mastersHint}

- + ); }; diff --git a/src/data/actions/app.js b/src/data/actions/app.js index 633351d8..52f745d9 100644 --- a/src/data/actions/app.js +++ b/src/data/actions/app.js @@ -61,6 +61,13 @@ const setModalState = createAction('setModalState', (modalState) => ({ */ const setShowImportSuccessToast = createAction('setShowImportSuccessToast'); +/** + * setShowImportErrorToast(shouldShow) + * Set whether or not to show the Import Grades error toast + * @param {bool} shouldShow - should show the toast? + */ +const setShowImportErrorToast = createAction('setShowImportErrorToast'); + /** * setView(viewId) * sets the UI to display the tab indcated by the passed view id @@ -76,6 +83,7 @@ export default StrictDict({ setModalState, setModalStateFromTable, setSearchValue, + setShowImportErrorToast, setShowImportSuccessToast, setView, }); diff --git a/src/data/actions/app.test.js b/src/data/actions/app.test.js index 763c9e3e..0891a9ef 100644 --- a/src/data/actions/app.test.js +++ b/src/data/actions/app.test.js @@ -12,6 +12,7 @@ describe('actions', () => { actions.setLocalFilter, actions.setModalStateFromTable, actions.setShowImportSuccessToast, + actions.setShowImportErrorToast, actions.setView, ].map(action => action.toString()); testActionTypes(actionTypes, dataKey); @@ -22,6 +23,9 @@ describe('actions', () => { test('setModalStateFromTable action', () => testAction(actions.setModalStateFromTable)); test('setSearchValue action', () => testAction(actions.setSearchValue)); test('setView action', () => testAction(actions.setView)); + test('setShowImportErrorToast action', () => ( + testAction(actions.setShowImportErrorToast) + )); test('setShowImportSuccessToast action', () => ( testAction(actions.setShowImportSuccessToast) )); diff --git a/src/data/reducers/app.js b/src/data/reducers/app.js index 192a586a..b7762de4 100644 --- a/src/data/reducers/app.js +++ b/src/data/reducers/app.js @@ -29,6 +29,7 @@ const initialState = { open: false, transitioning: false, }, + showImportErrorToast: false, showImportSuccessToast: false, searchValue: '', }; @@ -84,10 +85,19 @@ const app = (state = initialState, { type, payload } = {}) => { } case actions.setSearchValue.toString(): return { ...state, searchValue: payload }; + case actions.setShowImportErrorToast.toString(): + return { ...state, showImportErrorToast: payload }; case actions.setShowImportSuccessToast.toString(): return { ...state, showImportSuccessToast: payload }; + // A failure waits to be dismissed, so leaving the tab has to retire it. Otherwise it + // re-announces itself every time the user comes back to the Grades view. case actions.setView.toString(): - return { ...state, activeView: payload }; + return { + ...state, + activeView: payload, + showImportSuccessToast: false, + showImportErrorToast: false, + }; // initialize the filter fields that are locally stored case filterActions.initialize.toString(): return { @@ -111,8 +121,13 @@ const app = (state = initialState, { type, payload } = {}) => { }, }), { ...state }); } + // A new upload supersedes whatever the last one reported. + case gradesActions.csvUpload.started.toString(): + return { ...state, showImportSuccessToast: false, showImportErrorToast: false }; case gradesActions.csvUpload.finished.toString(): - return { ...state, showImportSuccessToast: true }; + return { ...state, showImportSuccessToast: true, showImportErrorToast: false }; + case gradesActions.csvUpload.error.toString(): + return { ...state, showImportErrorToast: true, showImportSuccessToast: false }; default: return state; } diff --git a/src/data/reducers/app.test.js b/src/data/reducers/app.test.js index 1799c62a..9c2ff225 100644 --- a/src/data/reducers/app.test.js +++ b/src/data/reducers/app.test.js @@ -185,12 +185,32 @@ describe('app reducer', () => { ).toEqual({ ...testingState, showImportSuccessToast: testValue }); }); }); + describe('appActions.setShowImportErrorToast', () => { + it('loads showImportErrorToast from payload', () => { + expect( + app(testingState, appActions.setShowImportErrorToast(testValue)), + ).toEqual({ ...testingState, showImportErrorToast: testValue }); + }); + }); describe('appActions.setView', () => { it('loads activeView from payload', () => { expect( app(testingState, appActions.setView(testValue)), ).toEqual({ ...testingState, activeView: testValue }); }); + it('retires both toasts, since a failure waits to be dismissed', () => { + expect( + app( + { ...testingState, showImportSuccessToast: true, showImportErrorToast: true }, + appActions.setView(testValue), + ), + ).toEqual({ + ...testingState, + activeView: testValue, + showImportSuccessToast: false, + showImportErrorToast: false, + }); + }); }); describe('filterActions.initialize', () => { it('loads relevant filter values', () => { @@ -232,10 +252,41 @@ describe('app reducer', () => { }); }); describe('grade actions csvUpload.finished', () => { - it('sets showImportSuccessToast to true', () => { + it('sets showImportSuccessToast to true and clears the error toast', () => { expect( app(testingState, gradesActions.csvUpload.finished()), - ).toEqual({ ...testingState, showImportSuccessToast: true }); + ).toEqual({ + ...testingState, + showImportSuccessToast: true, + showImportErrorToast: false, + }); + }); + }); + + describe('grade actions csvUpload.error', () => { + it('sets showImportErrorToast to true and clears the success toast', () => { + expect( + app(testingState, gradesActions.csvUpload.error({ errorMessages: ['nope'] })), + ).toEqual({ + ...testingState, + showImportErrorToast: true, + showImportSuccessToast: false, + }); + }); + }); + + describe('grade actions csvUpload.started', () => { + it('clears both toasts, so one upload is never described by the last', () => { + expect( + app( + { ...testingState, showImportSuccessToast: true, showImportErrorToast: true }, + gradesActions.csvUpload.started(), + ), + ).toEqual({ + ...testingState, + showImportSuccessToast: false, + showImportErrorToast: false, + }); }); }); }); diff --git a/src/data/redux/hooks/actions.js b/src/data/redux/hooks/actions.js index 526971d8..9668adf6 100644 --- a/src/data/redux/hooks/actions.js +++ b/src/data/redux/hooks/actions.js @@ -5,6 +5,7 @@ import { actionHook } from './utils'; const app = StrictDict({ useSetLocalFilter: actionHook(actions.app.setLocalFilter), useSetSearchValue: actionHook(actions.app.setSearchValue), + useSetShowImportErrorToast: actionHook(actions.app.setShowImportErrorToast), useSetShowImportSuccessToast: actionHook(actions.app.setShowImportSuccessToast), useSetView: actionHook(actions.app.setView), useCloseModal: actionHook(actions.app.closeModal), diff --git a/src/data/redux/hooks/actions.test.js b/src/data/redux/hooks/actions.test.js index 9f7de29e..5d166f93 100644 --- a/src/data/redux/hooks/actions.test.js +++ b/src/data/redux/hooks/actions.test.js @@ -24,6 +24,7 @@ describe('action hooks', () => { testActionHook(hookKeys.useSetLocalFilter, actions.app.setLocalFilter); testActionHook(hookKeys.useSetSearchValue, actions.app.setSearchValue); testActionHook(hookKeys.useSetShowImportSuccessToast, actions.app.setShowImportSuccessToast); + testActionHook(hookKeys.useSetShowImportErrorToast, actions.app.setShowImportErrorToast); testActionHook(hookKeys.useSetView, actions.app.setView); testActionHook(hookKeys.useCloseModal, actions.app.closeModal); testActionHook(hookKeys.useSetModalState, actions.app.setModalState); diff --git a/src/data/redux/hooks/selectors.js b/src/data/redux/hooks/selectors.js index 6f27ab0c..790dfc63 100644 --- a/src/data/redux/hooks/selectors.js +++ b/src/data/redux/hooks/selectors.js @@ -28,6 +28,7 @@ export const app = StrictDict({ useCourseId: selectorHook(selectors.app.courseId), useModalData: selectorHook(selectors.app.modalData), useSearchValue: selectorHook(selectors.app.searchValue), + useShowImportErrorToast: selectorHook(selectors.app.showImportErrorToast), useShowImportSuccessToast: selectorHook(selectors.app.showImportSuccessToast), }); @@ -52,6 +53,7 @@ export const filters = StrictDict({ export const grades = StrictDict({ useAllGrades: selectorHook(selectors.grades.allGrades), + useBulkImportErrorMessages: selectorHook(selectors.grades.bulkImportErrorMessages), useUserCounts: () => ({ filteredUsersCount: useSelector(selectors.grades.filteredUsersCount), totalUsersCount: useSelector(selectors.grades.totalUsersCount), diff --git a/src/data/redux/hooks/selectors.test.js b/src/data/redux/hooks/selectors.test.js index ad74e30a..18f0d540 100644 --- a/src/data/redux/hooks/selectors.test.js +++ b/src/data/redux/hooks/selectors.test.js @@ -64,6 +64,7 @@ describe('selector hooks', () => { testHook(hookKeys.useModalData, selKeys.modalData); testHook(hookKeys.useSearchValue, selKeys.searchValue); testHook(hookKeys.useShowImportSuccessToast, selKeys.showImportSuccessToast); + testHook(hookKeys.useShowImportErrorToast, selKeys.showImportErrorToast); }); describe('assignmentTypes', () => { loadSelectorGroup(selectorHooks.assignmentTypes, selectors.assignmentTypes); diff --git a/src/data/selectors/app.js b/src/data/selectors/app.js index 4ef2f7c8..994c841c 100644 --- a/src/data/selectors/app.js +++ b/src/data/selectors/app.js @@ -111,6 +111,7 @@ const simpleSelectors = simpleSelectorFactory( 'courseId', 'filters', 'searchValue', + 'showImportErrorToast', 'showImportSuccessToast', ], ); diff --git a/src/data/selectors/app.test.js b/src/data/selectors/app.test.js index e8d2966d..77086559 100644 --- a/src/data/selectors/app.test.js +++ b/src/data/selectors/app.test.js @@ -164,5 +164,6 @@ describe('app selectors', () => { testSimpleSelector('filters'); testSimpleSelector('searchValue'); testSimpleSelector('showImportSuccessToast'); + testSimpleSelector('showImportErrorToast'); }); }); diff --git a/src/data/selectors/grades.js b/src/data/selectors/grades.js index e5d22b68..303fac0b 100644 --- a/src/data/selectors/grades.js +++ b/src/data/selectors/grades.js @@ -191,9 +191,21 @@ export const allGrades = ({ grades: { results } }) => results; * @return {string} - bulk import error messages joined into a display form * (or empty string if there are none) */ -export const bulkImportError = ({ grades: { bulkManagement } }) => ( +export const bulkImportError = (state) => { + const messages = module.bulkImportErrorMessages(state); + return messages ? `Errors while processing: ${messages}` : ''; +}; + +/** + * bulkImportErrorMessages(state) + * returns the import error messages on their own, with no wrapper text, for the import + * error toast. '' when the last upload raised none. + * @param {object} state - redux state + * @return {string} - the messages, or '' + */ +export const bulkImportErrorMessages = ({ grades: { bulkManagement } }) => ( (!!bulkManagement && bulkManagement.errorMessages) - ? `Errors while processing: ${bulkManagement.errorMessages.join('; ')};` + ? bulkManagement.errorMessages.join('; ') : '' ); @@ -288,6 +300,7 @@ const gradeData = ({ grades }) => ({ export default StrictDict({ bulkImportError, + bulkImportErrorMessages, formatGradeOverrideForDisplay, formatMinAssignmentGrade, formatMaxAssignmentGrade, diff --git a/src/data/selectors/grades.test.js b/src/data/selectors/grades.test.js index ff3d6c8c..a3b9e5a8 100644 --- a/src/data/selectors/grades.test.js +++ b/src/data/selectors/grades.test.js @@ -324,9 +324,39 @@ describe('grades selectors', () => { expect( selectors.bulkImportError({ grades: { bulkManagement: { errorMessages } } }), ).toEqual( - `Errors while processing: ${errorMessages[0]}; ${errorMessages[1]};`, + `Errors while processing: ${errorMessages[0]}; ${errorMessages[1]}`, ); }); + + it('does not leave a trailing separator after a single message', () => { + expect( + selectors.bulkImportError({ + grades: { bulkManagement: { errorMessages: ['No grades were changed.'] } }, + }), + ).toEqual('Errors while processing: No grades were changed.'); + }); + }); + + describe('bulkImportErrorMessages', () => { + it('returns an empty string when there are no messages', () => { + expect( + selectors.bulkImportErrorMessages({ grades: { bulkManagement: { uploadSuccess: true } } }), + ).toEqual(''); + }); + + it('returns an empty string when bulkManagement not run', () => { + expect( + selectors.bulkImportErrorMessages({ grades: { bulkManagement: null } }), + ).toEqual(''); + }); + + it('returns the messages with no wrapper text, for the toast', () => { + expect( + selectors.bulkImportErrorMessages({ + grades: { bulkManagement: { errorMessages: ['first', 'second'] } }, + }), + ).toEqual('first; second'); + }); }); describe('bulkManagementHistory', () => {