-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor public API functions to no longer import and use store. #134
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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().specialExams; | ||
return !!exam?.id; | ||
} | ||
}; | ||
|
||
export const useExamAccessToken = () => { | ||
const { exam, examAccessToken } = useSelector(state => state.specialExams); | ||
|
||
export function getExamAccess() { | ||
const { exam, examAccessToken } = store.getState().specialExams; | ||
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().specialExams; | ||
const { dispatch } = store; | ||
if (!exam) { | ||
return Promise.resolve(); | ||
} | ||
return dispatch(examRequiresAccessToken()); | ||
} | ||
return () => dispatch(examRequiresAccessToken()); | ||
}; |
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for adding in this comment - super helpful! |
||
* 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 = { specialExams: { exam: mockExam, examAccessToken: mockToken } }; | ||
store.getState = jest.fn().mockReturnValue(mockState); | ||
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 = { specialExams: { exam: null, examAccessToken: null } }; | ||
store.getState = jest.fn().mockReturnValue(mockState); | ||
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); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ export function initializeMockApp() { | |
|
||
let globalStore; | ||
|
||
export async function initializeTestStore(preloadedState = null, overrideStore = true) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why this was marked |
||
export function initializeTestStore(preloadedState = null, overrideStore = true) { | ||
let store = configureStore({ | ||
reducer: { | ||
specialExams: examReducer, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about renaming this function. Is the intent to express that a selector is being used? I feel like the previous names expressed what the function was doing in a more clear way (i.e. what it returns).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. I agree that
isExam
is a better name thanuseIsExam
, but because I've turned an ordinary JavaScript function into a React hook, and because hooks names always start with use, I have to prefix the function name withuse
.