-
Notifications
You must be signed in to change notification settings - Fork 25
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
Login to Github before entering repository name (dialog popup) #178
Merged
gycgabriel
merged 38 commits into
CATcher-org:main
from
chia-yh:login-no-select-session
Sep 11, 2023
Merged
Login to Github before entering repository name (dialog popup) #178
gycgabriel
merged 38 commits into
CATcher-org:main
from
chia-yh:login-no-select-session
Sep 11, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Will review this after merging #169 because this builds on that |
gycgabriel
approved these changes
Sep 11, 2023
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.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Attempt to resolve #154
Alternative to #169 as suggested in #154 (comment)
WATcher allows users to change repositories, i.e., the repository is not fixed like in CATcher. Having users login first, then being able to choose the repository they wish to see would allow for a smoother UX, e.g.
Currently displays a minimal login card that redirects users to sign in to Github. Future enhancements to the UI would be desirable.
Type of change:
Changes Made:
login.component
, which extracts theAuthService::startOAuthProcess()
call fromsession-selection.component.ts
that allows users to login to Githublogin.component
inlogin.component.spec.test
ConfirmLoginComponent::completeLoginProcess()
toAuthService::setupUserData()
, leaving the creation of the user model, after which authentication state is changed to authenticatedhandleAuthSuccess()
fromConfirmLoginComponent
toAuthService
ConfirmLoginComponent
tests as necessaryrepoSetSource
,repoSetState
,isRepoSet()
toAuthService
auth.component.html
to showLoginComponent
instead ofSessionSelectionComponent
by defaultauth.component.html
to prevent current session from being shown on the login card for confirming login account, as no session has been set at that point of the processinitializeRepoNameInTitle()
method call inHeaderComponent
now subscribes torepoSetState
instead ofcurrentAuthState
, and checksisAuthenticated() && isRepoSet()
before calling the method,html
usesisRepoSet()
to display buttons requiring a current repository after one has been setcurrentAuthState
inHeaderComponent
now callsopenChangeRepoDialog
after user is authenticatedRepoChangeFormComponent
now differs based on if a repository has already been selectedScreenshots:
Proposed Commit Message:
Checklist: