Skip to content

Commit

Permalink
refactor: Refactor references to examStore to use the useDispatch and…
Browse files Browse the repository at this point in the history
… useSelector hooks API. (#131)

* feat: rename examStore to specialExams in preparation for use in frontend-app-learning

This commit renames the examStore to specialExams. This is in preparation for the use of the exam reducer in the frontend-app-learning React application.

* refactor: replace use of context with thunks and Redux store in instructions components

This commit replaces the use of the ExamStateContext with the use of thunks and the Redux store in the instructions components.

The original pattern was to use the withExamStore higher-order component to provide context to the instructions components. This context contained provided the Redux store state and action creators as props by using the connect API. This posed a problem for our need to merge the frontend-app-learning and frontend-lib-special-exams stores, because the special exams store is initialized in this repository and used by the higher-order component. In order to eventually be able to remove the creation of the store in this repository, we have to remove references to the store by interfacing with the Redux more directly by using the useDispatch and useSelector hooks.

* test: remove references to ExamStateProvider from tests for instructions components

This commit removes references to the ExamStateProvider from tests for instructions components. Because these components have been refactored to no longe rely on the ExamStateProvider, they are not required in the component tree.

refactor: replace use of withExamStore higher-order component in TimerServiceProvider (#133)

This commit replaces the use of the withExamStore higher-order component with the useDispatch and useSelector hooks in TimerServiceProvider.

This commit also refactors components that use the TimerServiceProvider so that they no longer need to pass state and action creators via props. The TimerServiceProvider now gets whatever state it needs directly from the Redux store with a useSelector hook and imports and dispatches thunks directly by importing them from the data directory.

The original pattern was to use the withExamStore higher-order component to provide context to the TimerServiceProvider and its children. This context contained provided the Redux store state and action creators as props by using the connect API. This posed a problem for our need to merge the frontend-app-learning and frontend-lib-special-exams stores, because the special exams store is initialized in this repository and used by the higher-order component. In order to eventually be able to remove the creation of the store in this repository, we have to remove references to the store by interfacing with the Redux more directly by using the useDispatch and useSelector hooks.

feat: refactor non instruction components (#132)

Refactor public API functions to no longer import and use store. (#134)

* test: remove asynchronicity from initializing test store

The initializeTestStore test utility function used async...await keywords for initializing the test store, which isn't necessary. This commit removes the async...await keywords and refactors any tests that use that function.

* test: fix mocking of getExamAttemptsData function

* feat: refactor public API to use hooks instead of references to exported store

This commit refactors the public API, used by the frontend-app-learning application to interact with the frontend-lib-special-exams state, to export a series of hooks. Originally, the public API imported the frontend-lib-special-exams store directly and operated on it in a series of exported functions. This posed a problem for our need to merge the frontend-app-learning and frontend-lib-special-exams stores, because the special exams store is initialized in this repository and used by public API. In order to eventually be able to remove the creation of the store in this repository, we have to remove references to the store by interfacing with the Redux more directly by using the useDispatch and useSelector hooks.

This commit also exports the root reducer from this library. This root reducer will be imported by the frontend-app-learning application and used to configure its store.

fix: patch bugs found during bash (#135)
  • Loading branch information
MichaelRoytman authored and alangsto committed Feb 8, 2024
1 parent 7f6f442 commit 434bbf2
Show file tree
Hide file tree
Showing 47 changed files with 740 additions and 980 deletions.
29 changes: 17 additions & 12 deletions src/api.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
import { examRequiresAccessToken, store } from './data';
import { useDispatch, useSelector } from 'react-redux';
import { examRequiresAccessToken } from './data';

export const useIsExam = () => {
const { exam } = useSelector(state => state.specialExams);

export function isExam() {
const { exam } = store.getState().examState;
return !!exam?.id;
}
};

export const useExamAccessToken = () => {
const { exam, examAccessToken } = useSelector(state => state.specialExams);

export function getExamAccess() {
const { exam, examAccessToken } = store.getState().examState;
if (!exam) {
return '';
}

return examAccessToken.exam_access_token;
}
};

export const useFetchExamAccessToken = () => {
const { exam } = useSelector(state => state.specialExams);
const dispatch = useDispatch();

export async function fetchExamAccess() {
const { exam } = store.getState().examState;
const { dispatch } = store;
if (!exam) {
return Promise.resolve();
}
return dispatch(examRequiresAccessToken());
}
return () => dispatch(examRequiresAccessToken());
};
66 changes: 43 additions & 23 deletions src/api.test.jsx
Original file line number Diff line number Diff line change
@@ -1,60 +1,80 @@
import { Factory } from 'rosie';

import { isExam, getExamAccess, fetchExamAccess } from './api';
import { store } from './data';
import { useExamAccessToken, useFetchExamAccessToken, useIsExam } from './api';
import { initializeTestStore, render } from './setupTest';

/**
* Hooks must be run in the scope of a component. To run the hook, wrap it in a test component whose sole
* responsibility it is to run the hook and assign it to a return value that is returned by the function.
* @param {*} hook: the hook function to run
* @param {*} store: an initial store, passed to the call to render
* @returns: the return value of the hook
*/
const getHookReturnValue = (hook, store) => {
let returnVal;
const TestComponent = () => {
returnVal = hook();
return null;
};
render(<TestComponent />, { store });
return returnVal;
};

describe('External API integration tests', () => {
describe('Test isExam with exam', () => {
describe('Test useIsExam with exam', () => {
let store;

beforeAll(() => {
jest.mock('./data');
const mockExam = Factory.build('exam', { attempt: Factory.build('attempt') });
const mockToken = Factory.build('examAccessToken');
const mockState = { examState: { exam: mockExam, examAccessToken: mockToken } };
store.getState = jest.fn().mockReturnValue(mockState);
const mockState = { specialExams: { exam: mockExam, examAccessToken: mockToken } };
store = initializeTestStore(mockState);
});

afterAll(() => {
jest.clearAllMocks();
jest.resetAllMocks();
});

it('isExam should return true if exam is set', () => {
expect(isExam()).toBe(true);
it('useIsExam should return true if exam is set', () => {
expect(getHookReturnValue(useIsExam, store)).toBe(true);
});

it('getExamAccess should return exam access token if access token', () => {
expect(getExamAccess()).toBeTruthy();
it('useExamAccessToken should return exam access token if access token', () => {
expect(getHookReturnValue(useExamAccessToken, store)).toBeTruthy();
});

it('fetchExamAccess should dispatch get exam access token', () => {
const dispatchReturn = fetchExamAccess();
expect(dispatchReturn).toBeInstanceOf(Promise);
it('useFetchExamAccessToken should dispatch get exam access token', () => {
// The useFetchExamAccessToken hook returns a function that calls dispatch, so we must call the returned
// value to invoke dispatch.
expect(getHookReturnValue(useFetchExamAccessToken, store)()).toBeInstanceOf(Promise);
});
});

describe('Test isExam without exam', () => {
describe('Test useIsExam without exam', () => {
let store;

beforeAll(() => {
jest.mock('./data');
const mockState = { examState: { exam: null, examAccessToken: null } };
store.getState = jest.fn().mockReturnValue(mockState);
const mockState = { specialExams: { exam: null, examAccessToken: null } };
store = initializeTestStore(mockState);
});

afterAll(() => {
jest.clearAllMocks();
jest.resetAllMocks();
});

it('isExam should return false if exam is not set', () => {
expect(isExam()).toBe(false);
it('useIsExam should return false if exam is not set', () => {
expect(getHookReturnValue(useIsExam, store)).toBe(false);
});

it('getExamAccess should return empty string if exam access token not set', () => {
expect(getExamAccess()).toBeFalsy();
it('useExamAccessToken should return empty string if exam access token not set', () => {
expect(getHookReturnValue(useExamAccessToken, store)).toBeFalsy();
});

it('fetchExamAccess should not dispatch get exam access token', () => {
const dispatchReturn = fetchExamAccess();
expect(dispatchReturn).toBeInstanceOf(Promise);
it('useFetchExamAccessToken should not dispatch get exam access token', () => {
expect(getHookReturnValue(useFetchExamAccessToken, store)).toBeInstanceOf(Promise);
});
});
});
4 changes: 0 additions & 4 deletions src/context.jsx

This file was deleted.

35 changes: 0 additions & 35 deletions src/core/ExamStateProvider.jsx

This file was deleted.

32 changes: 12 additions & 20 deletions src/core/OuterExamTimer.jsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import React, { useEffect, useContext } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import PropTypes from 'prop-types';
import { AppContext } from '@edx/frontend-platform/react';
import ExamStateContext from '../context';
import { ExamTimerBlock } from '../timer';
import ExamAPIError from '../exam/ExamAPIError';
import ExamStateProvider from './ExamStateProvider';
import { getLatestAttemptData } from '../data';
import { IS_STARTED_STATUS } from '../constants';

const ExamTimer = ({ courseId }) => {
const state = useContext(ExamStateContext);
const { activeAttempt } = useSelector(state => state.specialExams);
const { authenticatedUser } = useContext(AppContext);
const {
activeAttempt, showTimer, stopExam, submitExam,
expireExam, pollAttempt, apiErrorMsg, pingAttempt,
getLatestAttemptData,
} = state;
const showTimer = !!(activeAttempt && IS_STARTED_STATUS(activeAttempt.attempt_status));

const { apiErrorMsg } = useSelector(state => state.specialExams);

const dispatch = useDispatch();

useEffect(() => {
getLatestAttemptData(courseId);
dispatch(getLatestAttemptData(courseId));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [courseId]);

Expand All @@ -29,14 +30,7 @@ const ExamTimer = ({ courseId }) => {
return (
<div className="d-flex flex-column justify-content-center">
{showTimer && (
<ExamTimerBlock
attempt={activeAttempt}
stopExamAttempt={stopExam}
submitExam={submitExam}
expireExamAttempt={expireExam}
pollExamAttempt={pollAttempt}
pingAttempt={pingAttempt}
/>
<ExamTimerBlock />
)}
{apiErrorMsg && <ExamAPIError />}
</div>
Expand All @@ -53,9 +47,7 @@ ExamTimer.propTypes = {
* will be shown.
*/
const OuterExamTimer = ({ courseId }) => (
<ExamStateProvider>
<ExamTimer courseId={courseId} />
</ExamStateProvider>
<ExamTimer courseId={courseId} />
);

OuterExamTimer.propTypes = {
Expand Down
17 changes: 10 additions & 7 deletions src/core/OuterExamTimer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import '@testing-library/jest-dom';
import { Factory } from 'rosie';
import React from 'react';
import OuterExamTimer from './OuterExamTimer';
import { store, getLatestAttemptData } from '../data';
import { render } from '../setupTest';
import { getLatestAttemptData } from '../data';
import { initializeTestStore, render } from '../setupTest';
import { ExamStatus } from '../constants';

jest.mock('../data', () => ({
store: {},
getLatestAttemptData: jest.fn(),
Emitter: {
on: () => jest.fn(),
Expand All @@ -17,18 +16,22 @@ jest.mock('../data', () => ({
},
}));
getLatestAttemptData.mockReturnValue(jest.fn());
store.subscribe = jest.fn();
store.dispatch = jest.fn();

describe('OuterExamTimer', () => {
const courseId = 'course-v1:test+test+test';

let store;

beforeEach(() => {
store = initializeTestStore();
});

it('is successfully rendered and shows timer if there is an exam in progress', () => {
const attempt = Factory.build('attempt', {
attempt_status: ExamStatus.STARTED,
});
store.getState = () => ({
examState: {
specialExams: {
activeAttempt: attempt,
exam: {
time_limit_mins: 60,
Expand All @@ -45,7 +48,7 @@ describe('OuterExamTimer', () => {

it('does not render timer if there is no exam in progress', () => {
store.getState = () => ({
examState: {
specialExams: {
activeAttempt: {},
exam: {},
},
Expand Down
5 changes: 1 addition & 4 deletions src/core/SequenceExamWrapper.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import ExamWrapper from '../exam/ExamWrapper';
import ExamStateProvider from './ExamStateProvider';

/**
* SequenceExamWrapper is the component responsible for handling special exams.
Expand All @@ -14,9 +13,7 @@ import ExamStateProvider from './ExamStateProvider';
* </SequenceExamWrapper>
*/
const SequenceExamWrapper = (props) => (
<ExamStateProvider>
<ExamWrapper {...props} />
</ExamStateProvider>
<ExamWrapper {...props} />

Check warning on line 16 in src/core/SequenceExamWrapper.jsx

View check run for this annotation

Codecov / codecov/patch

src/core/SequenceExamWrapper.jsx#L16

Added line #L16 was not covered by tests
);

export default SequenceExamWrapper;
2 changes: 1 addition & 1 deletion src/data/__factories__/examState.factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import './exam.factory';
import './proctoringSettings.factory';
import './examAccessToken.factory';

Factory.define('examState')
Factory.define('specialExams')
.attr('proctoringSettings', Factory.build('proctoringSettings'))
.attr('exam', Factory.build('exam'))
.attr('examAccessToken', Factory.build('examAccessToken'))
Expand Down
8 changes: 4 additions & 4 deletions src/data/__snapshots__/redux.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Object {

exports[`Data layer integration tests Test getExamAttemptsData Should get, and save exam and attempt 1`] = `
Object {
"examState": Object {
"specialExams": Object {
"activeAttempt": Object {
"attempt_code": "",
"attempt_id": 1,
Expand Down Expand Up @@ -93,7 +93,7 @@ Object {

exports[`Data layer integration tests Test getLatestAttemptData with edx-proctoring as a backend (no EXAMS_BASE_URL) Should get, and save latest attempt 1`] = `
Object {
"examState": Object {
"specialExams": Object {
"activeAttempt": Object {
"attempt_code": "",
"attempt_id": 1,
Expand Down Expand Up @@ -245,7 +245,7 @@ Object {

exports[`Data layer integration tests Test resetExam Should reset exam attempt 1`] = `
Object {
"examState": Object {
"specialExams": Object {
"activeAttempt": null,
"allowProctoringOptOut": false,
"apiErrorMsg": "",
Expand Down Expand Up @@ -314,7 +314,7 @@ Object {

exports[`Data layer integration tests Test resetExam with edx-proctoring as backend (no EXAMS_BASE_URL) Should reset exam attempt 1`] = `
Object {
"examState": Object {
"specialExams": Object {
"activeAttempt": null,
"allowProctoringOptOut": false,
"apiErrorMsg": "",
Expand Down
5 changes: 4 additions & 1 deletion src/data/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export {
createProctoredExamAttempt,
getExamAttemptsData,
getLatestAttemptData,
getProctoringSettings,
Expand All @@ -15,7 +16,9 @@ export {
resetExam,
getAllowProctoringOptOut,
examRequiresAccessToken,
checkExamEntry,
} from './thunks';

export { default as store } from './store';
export { default as reducer } from './slice';

export { default as Emitter } from './emitter';
Loading

0 comments on commit 434bbf2

Please sign in to comment.