-
Notifications
You must be signed in to change notification settings - Fork 1
Redux for Auth #12
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
Redux for Auth #12
Conversation
|
CI is failing on coveralls. Need test coverage... |
liammoyn
left a comment
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.
As far as I can tell you're on the right track. More comments would definitely help and the refresh route will be an interesting challenge.
I definitely agree that a code walk is necessary
| void | ||
| >(authenticateUser.key); | ||
|
|
||
| const reducers = ( |
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.
Is this the top level reducer? If so, would all API calls that get data have to be included in this down the line?
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.
Everything in this auth/ducks/ folder will be solely for authentication-related data and actions. Each container typically will have its own ducks. The reducers, along with the action types, are combined in store.ts.
src/auth/ducks/thunks.ts
Outdated
| return authClient | ||
| .login(loginRequest) | ||
| .then((response: TokenResponse) => { | ||
| tokenService.setAccessToken(response.accessToken); |
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.
It seems weird that we have this "tokenService" file that is storing our tokens if we're now also storing the token information in state. Would it make sense to consolidate where we keep that data?
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.
tokenService is separated purely because it relies upon local storage. At first, I thought keeping the tokens in Redux may be insecure or bad practice, but either way they'll be easily accessible in the browser once you've logged in, which isn't necessarily a problem. I think it'd be a lot easier to have in the Redux state, we just have to account for storing the refresh token in localStorage and accessing it upon application load.
| const Signup: React.FC = () => { | ||
| const dispatch = useDispatch(); | ||
| const onFinish = (values: any) => { | ||
| // TODO: what if backend says the values are invalid? need to handle this |
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.
What happens in this case now?
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.
The AsyncRequest utility file will dictate the state of any action defined as an AsyncRequest. It defines notStarted, loading, loaded, and failed. So depending on the state of the user authentication async request, we'll conditionally render data. The backend-scaffold also currently only says "Malformed JSON request" so I can't yet implement an Error type on the AsyncRequest. The util optionally allows an Error data payload to be stored, so we should be able to strongly type the backend responses to populate errors on form inputs.
What
Redux is a framework for application state management. It defines clear patterns for fetching, accessing, and updating data.
This PR
Instantiates a Redux store in
index.tsx. The store includes type for State and Action, where state is the data and actions are the only method of interacting with that data store.Adds a new directory
authfor all user authentication concerns. This directory includes a client which makes calls to the backend, a token service for interacting with JSON Web Tokens used to identify users, a custom Axios instance which uses those tokens, and finally,ducks....If it looks like a duck, quacks like a duck, and walks like a duck, it's probably a duck.
len("ducks") == len("redux")?Basically, we define ducks to interact with our Redux store. Our
auth/ducksinclude thunks, reducers, actions, and types. Thunks are special actions which can do async tasks. We have thunks for login and signup. Those thunks, on a response, will dispatch an action to update data, reflecting the change. The action is processed by the reducers, which return an updated state object based on the action.That's not a great description, and these changes are still incomplete... but we are at a place where logging in dispatches the login thunk, which dispatches the auth success action, which updates the store to include the user's ID and privilege level.
Next Steps