-
Notifications
You must be signed in to change notification settings - Fork 5
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
Ig 130 #136
base: master
Are you sure you want to change the base?
Conversation
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.
fix and TEST requests (if you have questions just ask)
|
||
export const LoginPage = (): JSX.Element => { | ||
export const LoginPage = (): any => { |
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.
dont have to set return type here
// TODO display notification from context | ||
console.log(error?.response.data || error?.response.status); | ||
return addNotification({ | ||
mode: NotificationMode.INFO, |
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.
why INFO when its an error?
return addNotification({ | ||
mode: NotificationMode.INFO, | ||
title: 'Login', | ||
message: error, |
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.
I think its error.message
try { | ||
// TODO save user data to reducer | ||
const userData = (await api.post('url..', form)) as User; | ||
const userData = (await api.post('/login', form)) as User; |
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.
wrong url, its /auth/login
|
||
const login = async () => { | ||
setIsLoading(true); |
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.
why remove?
|
||
async function register() { | ||
setLoading(true); | ||
try { | ||
// TODO save user data to reducer | ||
const userData = (await api.post('url...', form)) as User; | ||
const userData = (await api.post('/register', form)) as User; |
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.
check login component comments and fix ALL register
try { | ||
// TODO save user data to reducer | ||
const userData = (await api.post('url..', form)) as User; | ||
const userData = (await api.post('/login', form)) as User; |
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.
use generic types for POST
we want to declare with post requset will return
//
post method in api class should return Promise
setLocalStorage(USER_TOKEN, userData.token); | ||
dispatch(saveUserDataAction(form)); |
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.
we want to store userData that comes from api (check api response). not form. PASSWORD and token cannot be stored here :p
try { | ||
// TODO save user data to reducer | ||
const userData = (await api.post('url..', form)) as User; | ||
const userData = (await api.post('/login', form)) as User; | ||
setLocalStorage(USER_TOKEN, userData.token); |
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.
token doesnt exist in userData. its different (check response)
import { User } from '../../models/user'; | ||
import { SAVE_USER_DATA } from '../actions/userActions'; | ||
|
||
const initUserState: User = new User(); |
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.
we dont want initUserState. or if you want to just empty object
No description provided.