search

thedataincubator / resumake.io

📋 A website for automatically generating elegant LaTeX resumes.
https://resumake.io
MIT License
1 stars 2 forks source link

Handle auth errors #7

Closed rschroll closed 4 years ago

rschroll commented 4 years ago

Two fixes:

  1. Handle the two types of JSON errors we get back (see #6).
  2. Do appropriate things for 401 and 403 errors.

Putting react components in the actions file is really sloppy, but it gets the job done.

Please go ahead and push fixes to this branch; I'd like to deploy it tomorrow if possible.

rschroll commented 4 years ago

Note that this requires thedataincubator/website#1991 to function. I've deployed both this and that to staging for testing.

rschroll commented 4 years ago

In a bit of testing, the only problem I noticed was that, if you were supposed to load json data from the website on your first visit, but were forced to redirect to login first, you don't automatically load the data when you're directed back to the resumake page. Hitting the load data button works, though.

If there's an easy fix to this, please suggest it. Otherwise, don't worry about it.

Ampretuzo commented 4 years ago

In a bit of testing, the only problem I noticed was that, if you were supposed to load json data from the website on your first visit, but were forced to redirect to login first, you don't automatically load the data when you're directed back to the resumake page.

I tracked this down and found where it goes wrong. In store.js I'm explicitly excluding fellowData from persistence. As a result the store is hydrated with inconsistent state on page/application reload. That's wrong and I think we should persist the whole tdi state. (And any non-calculated application state in general.) Persisting the whole state does indeed fix this reloading issue, but I need to test it out better so that we don't end up with another batch of bugs - it would definitely be safer to use your solution now.

rschroll commented 4 years ago

I'm going to merge this in now, but we can worry about the state management in the future.