freeCodeCamp / pantry-for-good

An open source food bank logistics and inventory management tool
Other
395 stars 189 forks source link

'Unknown user or invalid password' should be displayed to the user, in case, while trying to Sign In #370

Closed gomezvillegasdaniel closed 6 years ago

gomezvillegasdaniel commented 6 years ago

https://pantry-for-good.herokuapp.com/users/signin unknown user or invalid password pantry for food

gomezvillegasdaniel commented 6 years ago

I'm hands on it

gomezvillegasdaniel commented 6 years ago

@jspaine Hi, I don't find the SignIn response. I've inspected both this.props and this.props.signIn(this.state.email, this.state.password) call but it doesn't return anything, nor a promise. Could you help me pls?

jspaine commented 6 years ago

Hi, the signIn function just dispatches a redux action which sets the fetching state in the auth store and makes the api call. When that returns a success/failure action is dispatched. So you just need to watch the store to see the current state.

Try looking with the redux devtools open.

gomezvillegasdaniel commented 6 years ago

ok, but I noticed that if I jump to the auth/SIGNIN_FAILURE with the redux devtools it already displays the error on the screen, so I think I need to know how to get the store from the component and see if the status response is 400, then jump to that state?

jspaine commented 6 years ago

Oh yeah, it shows the error but then the CLEAR_FLAGS action clears it again. I don't know why that action is being dispatched then (and twice) but if you can stop that it should fix it.

kenjiO commented 6 years ago

I think this is what is going on the way the application is now:

Every time UserRouter renders it calls guestOrRedirect(SignIn). This results in both the GuestOrRedirect and SignIn components getting unmounted, re-created and then mounted again which dispatches the CLEAR_FLAGS action.

I think that we only want to have the GuestOrRedirect with SignIn component created one time. So either changing UserRouter to not call this repeatedly or changing guestOrRedirect to not create a new component each time should fix this.

gomezvillegasdaniel commented 6 years ago

Thanks. I've tried to singletonise the GuestOrRedirect class when the WrappedComponent is instance of SignIn, but the class is being redeclared each time the guestOrRedirect function is called, so I loss the singleton instance. I don't know any other way to do it.

kenjiO commented 6 years ago

I think moving the call to guestOrRedirect outside of the UserRouter function will fix this.

const guestOrRedirectSignIn = guestOrRedirect(SignIn)

const UserRouter = ({match}) =>
 <SwitchWithNotFound>
    .
    .
    .
    <Route path={`${match.url}/signin`} exact component={guestOrRedirectSignIn} />

If this does work we will probably want to look at using it with the other routes calling guestOrRedirect and userOrRedirect if similar issues are happening with them

gomezvillegasdaniel commented 6 years ago

It worked! No collateral effects and no similar issues in the other routes. Thanks.