Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

change sign in url to go directly to the confirm login page #76

Merged
merged 3 commits into from Jan 23, 2021

Conversation

JillianK
Copy link
Contributor

@JillianK JillianK commented Dec 15, 2020

NOTE: this should not be merged until the related PR in Code Studio is deployed.
A few changes here:

  1. When a user was already logged in, they were seeing an error instead of the "magic code" to paste into Maker App. This takes them directly to the code page if they're already logged in.
  2. If a user tries to log in without a google authentication option, show them a more useful message (final wording still in progress). This is actually handled in the related PR in Code Studio

Here's a video for when a user is not logged in and uses a valid google auth option: https://drive.google.com/file/d/1_fvrYv2oMNnOcluvxi2LFgRiSGbIGlJW/view?usp=sharing
Here's a video for when a user is not logged in and uses a non google auth option: https://drive.google.com/file/d/1SAux-C79c6dOOyJu_IxR2nL3v60vgZhQ/view?usp=sharing
If the user is already logged in, it will skip directly to that last page (with either the code or the error message depending on the existence of a google authentication option)

Links

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@epeach
Copy link
Contributor

epeach commented Jan 4, 2021

Howdy! This looks good! However, if you end up making this change (https://github.com/code-dot-org/code-dot-org/pull/38245/files#r551561080), I think we can also remove the 'maker=true' from here (

require('electron').shell.openExternal(SIGN_IN_URL + '?user_return_to=/maker/display_google_oauth_code&maker=true');
) while we are cleaning these paths up. Let me know if you have any questions or want to talk through things!

Copy link
Contributor

@epeach epeach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@epeach
Copy link
Contributor

epeach commented Jan 14, 2021

@JillianK - I think if you merge the latest main branch to this thread, we'll hopefully get a green test run!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants