Skip to content

Commit

Permalink
Login to Github before entering repository name (dialog popup) (#178)
Browse files Browse the repository at this point in the history
Change auth process to login before selecting repo

Users select a repository before login.

WATcher does not fix a single repository, and allows users to change the
selected repository.

Having users login to Github prior to selecting a repository would
facilitate a smoother UX.

Let's allow users to login before choosing a repository.
  • Loading branch information
chia-yh authored Sep 11, 2023
1 parent 0829bab commit 67c2ab8
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/app/core/services/phase.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class PhaseService {

if (this.currentPhase === Phase.issuesViewer) {
/** Adds past repositories to phase */
this.otherRepos.push(this.currentRepo);
(this.otherRepos || []).push(this.currentRepo);
}
this.setRepository(repo, this.otherRepos);

Expand Down
4 changes: 2 additions & 2 deletions src/app/shared/layout/header.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@

<div *ngIf="auth.isAuthenticated() && phaseService.isRepoSet()">
<span id="phase-descriptor" style="margin-left: 10px">
{{ this.currentRepo }}
{{ this.currentRepo || "No Repository Set"}}
</span>
<button mat-button matTooltip="Change Repository" (click)="this.openChangeRepoDialog()">
<button mat-button matTooltip="{{ phaseService.isRepoSet() ? 'Change Repository' : 'Select Repository'}}" (click)="this.openChangeRepoDialog()">
<mat-icon>edit</mat-icon>
</button>
</div>
Expand Down
33 changes: 32 additions & 1 deletion src/app/shared/layout/header.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IssueService } from '../../core/services/issue.service';
import { LabelService } from '../../core/services/label.service';
import { LoggingService } from '../../core/services/logging.service';
import { PhaseDescription, PhaseService } from '../../core/services/phase.service';
import { RepoUrlCacheService } from '../../core/services/repo-url-cache.service';
import { UserService } from '../../core/services/user.service';

const ISSUE_TRACKER_URL = 'https://github.com/CATcher-org/WATcher/issues';
Expand Down Expand Up @@ -50,6 +51,7 @@ export class HeaderComponent implements OnInit {
private location: Location,
private githubEventService: GithubEventService,
private issueService: IssueService,
private repoUrlCacheService: RepoUrlCacheService,
private labelService: LabelService,
private errorHandlingService: ErrorHandlingService,
private githubService: GithubService,
Expand All @@ -64,6 +66,12 @@ export class HeaderComponent implements OnInit {
this.prevUrl = e[0].urlAfterRedirects;
});

this.auth.currentAuthState.subscribe((state) => {
if (auth.isAuthenticated()) {
this.openChangeRepoDialog();
}
});

this.phaseService.repoSetState.subscribe((state) => {
if (auth.isAuthenticated() && phaseService.isRepoSet()) {
this.initializeRepoNameInTitle();
Expand Down Expand Up @@ -233,7 +241,30 @@ export class HeaderComponent implements OnInit {
}
const newRepo = Repo.of(res);

this.changeRepositoryIfValid(newRepo, res);
if (this.phaseService.isRepoSet()) {
this.changeRepositoryIfValid(newRepo, res);
} else {
/**
* From session-selection.component.ts
*
* Persist repo information in local browser storage
* To retrieve after authentication redirects back to WATcher
*
* Since localStorage::setItem with an undefined value can result in
* the subsequent value being stored as a string being 'undefined', check
* if undefined before storing it. Let's reset the items before setting them.
*/
window.localStorage.removeItem('org');
window.localStorage.removeItem('dataRepo');

if (newRepo) {
window.localStorage.setItem('org', newRepo.owner);
window.localStorage.setItem('dataRepo', newRepo.name);

this.repoUrlCacheService.cache(newRepo.toString());
}
this.auth.setRepo().subscribe();
}
});
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<h1 mat-dialog-title class="change-repo-form-title">Change repository</h1>
<h1 mat-dialog-title class="change-repo-form-title"> {{ data.repoName ? "Change repository" : "Select repository" }} </h1>
<div mat-dialog-content>
<form (ngSubmit)="onYesClick()">
<mat-form-field appearance="fill">
Expand All @@ -14,5 +14,5 @@ <h1 mat-dialog-title class="change-repo-form-title">Change repository</h1>
</div>
<div mat-dialog-actions>
<button mat-button (click)="onNoClick()" color="warn">Cancel</button>
<button mat-button (click)="onYesClick()" color="primary">Change Repo</button>
<button mat-button (click)="onYesClick()" color="primary"> {{ data.repoName ? "Change Repo" : "Select Repo" }} </button>
</div>
4 changes: 2 additions & 2 deletions tests/app/auth/login/login.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { LoginComponent } from '../../../../src/app/auth/login/login.component';
import { AuthService } from '../../../../src/app/core/services/auth.service';
import { ErrorHandlingService } from '../../../../src/app/core/services/error-handling.service';
Expand All @@ -11,7 +11,7 @@ describe('LoginComponent', () => {
let component: LoginComponent;
let fixture: ComponentFixture<LoginComponent>;

beforeEach(async(() => {
beforeEach(waitForAsync(() => {
authService = jasmine.createSpyObj<AuthService>('AuthService', ['startOAuthProcess']);
logger = jasmine.createSpyObj<LoggingService>('LoggingService', ['info']);
errorHandlingService = jasmine.createSpyObj<ErrorHandlingService>('ErrorHandlingService', ['handleError']);
Expand Down

0 comments on commit 67c2ab8

Please sign in to comment.