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

Sign in process needs some sort of notification it hasn't crashed #16266

Closed
QuincyLarson opened this issue Dec 22, 2017 · 11 comments
Closed

Sign in process needs some sort of notification it hasn't crashed #16266

QuincyLarson opened this issue Dec 22, 2017 · 11 comments
Assignees
Labels
scope: UI Threads for addressing UX changes and improvements to user interface

Comments

@QuincyLarson
Copy link
Contributor

QuincyLarson commented Dec 22, 2017

When signing in, there is a moment where the button turns a strange shade of green and nothing happens for about 30 seconds. This is a really long wait, and we should definitely find a way to make the wait shorter. But we should also show some sort of visual confirmation that it's working and it hasn't crashed.

sign_in_to_freecodecamp_using_your_email_address___freecodecamp

@raisedadead is there anything we can do to speed up the email? Even if it takes a moment to receive the email, the UI should seem as snappy as possible.

@raisedadead
Copy link
Member

This is inherent from the infrastructure challenges.

Currently, whenever a login request is made:

  1. An AJAX request is made to the auth api.
  2. It looks up through the accounts, and creates a token.
  3. Then a request is made to AWS api for sending out the email.
  4. As the requests cascade and resolve, the AJAX completes.

The button in the UI is in a disabled state, while the above 4 steps happen.
This is to prevent clicking and generating multiple requests.

We have #14902 to make steps 3-4 async, and speed up some of this.
We can add a notification on this page, to warn of this delay.

But, the real enhancement is to implement the Job Queue.

@raisedadead raisedadead added enhancement scope: UI Threads for addressing UX changes and improvements to user interface labels Dec 23, 2017
@raisedadead
Copy link
Member

raisedadead commented Dec 23, 2017

A delayed method can be added in between the below lines, which will set a notification

disableMagicButton(true);
var $form = $(event.target);

that reads,

There are lot of requests being made, please bear with us, while we send you the email to sign-in.
This should not take too long.

And such a notification should be triggered only after 3-4 seconds after the button was clicked.

@raisedadead raisedadead added the help wanted Open for all. You do not need permission to work on these. label Dec 23, 2017
@QuincyLarson
Copy link
Contributor Author

QuincyLarson commented Dec 23, 2017

@raisedadead Couldn't we just say "OK - we're sending a magic link to [email address]" in the UI and remove the button? Then by the time they get into their email, it will probably already be there. That would be more conventional, simpler, and - most importantly - less astonishing to the user.

@raisedadead
Copy link
Member

@QuincyLarson yes, that can be done.

@QuincyLarson
Copy link
Contributor Author

@raisedadead Awesome! Thanks for doing this. This is such a critical part of the onboarding, so we want to get it just right. Once this is ready, I will go through and fine-tune the copy throughout the signing in process. Thanks for keeping me posted on this :)

@StephenMayeux
Copy link
Contributor

Chatted with @raisedadead and I'm gonna take this one.

@raisedadead raisedadead added status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. and removed enhancement help wanted Open for all. You do not need permission to work on these. labels Dec 27, 2017
@raisedadead
Copy link
Member

@StephenMayeux thanks a lot for your interest in this issue, but I happened to figure out that we are having a refactor of some of the backend in progress.

This will affect the work on this issue.

I am adding a blocked tag until then. Please feel free to take a look at other open issues.

@QuincyLarson
Copy link
Contributor Author

@StephenMayeux It's great to have you here! Since @raisedadead wrote the original logic and has already made good progress on it, I recommend jumping on another one of the help wanted issues.

@raisedadead raisedadead removed the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Jan 4, 2018
@raisedadead
Copy link
Member

@QuincyLarson quick updates,

With some wizardry from @BerkeleyTrue as usual (which was a huge overhaul actually!!).

image

We are seeing a slight drop in the time it takes the request to complete the flight!

And currently, the external email request to AWS is taking by far the longest to pass thru.

image

So as I said the real fix is the async job queue.

But, for now for some client side feedback, I'll see and proceed with the above discussion earlier.

@BerkeleyTrue
Copy link
Contributor

To add some information, this is not unique to beta and is a general issue with sending information through aws. It's just slow. Client side feedback would be the quickest fix. I don't think we can do anything serverside to make it quicker without sacrificing something else. The long run a separate email service to run in production (rabbitmq and a separate node server?) would be the solution.

@raisedadead raisedadead self-assigned this Jan 4, 2018
@raisedadead raisedadead added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jan 4, 2018
@raisedadead
Copy link
Member

raisedadead commented Jan 4, 2018

The long run a separate email service to run in production (rabbitmq and a separate node server?) would be the solution.

Yes, I am exploring Kue by the Automattic team which uses redis as its DB. It seems to be really light.
We can have a dedicated nodemailer (node server) hooked to it or use it as it is with our AWS setup.

@BerkeleyTrue BerkeleyTrue removed the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jan 6, 2018
mhatout pushed a commit to mhatout/freeCodeCamp that referenced this issue Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: UI Threads for addressing UX changes and improvements to user interface
Projects
None yet
Development

No branches or pull requests

4 participants