Skip to content
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

Conversation

MichaelRoytman
Copy link
Contributor

Description:

Jira: COSMO-175

  • 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.
  • 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

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.
…ted 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.
@MichaelRoytman MichaelRoytman changed the title Refactor public API functions to no import and use store. Refactor public API functions to no longer import and use store. Feb 5, 2024
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (7f6f442) 94.14% compared to head (5fc54c0) 93.32%.

Files Patch % Lines
src/timer/TimerProvider.jsx 71.42% 2 Missing ⚠️
src/core/SequenceExamWrapper.jsx 0.00% 1 Missing ⚠️
src/exam/Exam.jsx 80.00% 1 Missing ⚠️
...arding_exam/EntranceOnboardingExamInstructions.jsx 66.66% 1 Missing ⚠️
...nboarding_exam/ErrorOnboardingExamInstructions.jsx 50.00% 1 Missing ⚠️
...arding_exam/RejectedOnboardingExamInstructions.jsx 66.66% 1 Missing ⚠️
...rding_exam/SubmittedOnboardingExamInstructions.jsx 66.66% 1 Missing ⚠️
...practice_exam/EntrancePracticeExamInstructions.jsx 50.00% 1 Missing ⚠️
...ns/practice_exam/ErrorPracticeExamInstructions.jsx 50.00% 1 Missing ⚠️
...ractice_exam/SubmittedPracticeExamInstructions.jsx 50.00% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #134      +/-   ##
==========================================
- Coverage   94.14%   93.32%   -0.83%     
==========================================
  Files          71       68       -3     
  Lines        1059     1048      -11     
  Branches      290      291       +1     
==========================================
- Hits          997      978      -19     
- Misses         57       65       +8     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MichaelRoytman MichaelRoytman changed the base branch from main to 2u/cosmonauts/refactor-store February 5, 2024 18:14
@@ -45,7 +45,7 @@ export function initializeMockApp() {

let globalStore;

export async function initializeTestStore(preloadedState = null, overrideStore = true) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this was marked async. There's nothing asynchronous about it. It might have been copy pasted from frontend-app-learning's initializeTestStore function, which does have asynchronicity.

Copy link
Contributor

@alangsto alangsto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions/nits, but otherwise looks good! Thanks for cleaning up the async related things with the test store!

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding in this comment - super helpful!

import { useDispatch, useSelector } from 'react-redux';
import { examRequiresAccessToken } from './data';

export const useIsExam = () => {
Copy link
Contributor

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).

Copy link
Contributor Author

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 than useIsExam, 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 with use.

@MichaelRoytman MichaelRoytman merged commit 37c550a into 2u/cosmonauts/refactor-store Feb 7, 2024
5 of 7 checks passed
@MichaelRoytman MichaelRoytman deleted the michaelroytman/COSMO-175-refactor-public-api branch February 7, 2024 17:58
alangsto pushed a commit that referenced this pull request Feb 8, 2024
… 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)
alangsto pushed a commit that referenced this pull request Feb 8, 2024
… 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)
alangsto added a commit that referenced this pull request Feb 8, 2024
… useSelector hooks API. (#131) (#136)

* 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)

Co-authored-by: Michael Roytman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants