Skip to content

Commit

Permalink
Enhance error message on incorrect organisation/repo name (#378)
Browse files Browse the repository at this point in the history
The original error message for incorrect repo name does not specify 
whether the mistake is in the org name or repo name.

Hence, let's add an additional error message to show if the 
organisation name could not be found. This will give users an easier 
time finding the error and rectifying it when keying in a repo.
  • Loading branch information
kokerinks authored Jul 8, 2024
1 parent 8b0103c commit 03ef9e0
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 15 deletions.
6 changes: 5 additions & 1 deletion src/app/core/services/error-message.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ import { Injectable } from '@angular/core';
* Contains all error message prompts to user.
*/
export class ErrorMessageService {
public static repoOwnerNotPresentMessage() {
return 'Invalid repo owner. Provide a repository URL or repo name (<ORG/USER>/<REPO>) with a valid organisation/user name';
}

public static repositoryNotPresentMessage() {
return 'Invalid repository name. Please provide Github repository URL or the repository name in the format Org/Repository Name.';
return 'Invalid repository name. Please provide a Github repository URL or repo name in the format <ORG/USER>/<REPO>.';
}

public static invalidUrlMessage() {
Expand Down
33 changes: 31 additions & 2 deletions src/app/core/services/github.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,36 @@ export class GithubService {
);
}

/**
* Checks if the specified username exists.
* @param userName - Name of Username
*/
isUsernamePresent(userName: string): Observable<boolean> {
return from(octokit.users.getByUsername({ username: userName, headers: GithubService.IF_NONE_MATCH_EMPTY })).pipe(
map((rawData: { status: number }) => {
return rawData.status !== ERRORCODE_NOT_FOUND;
}),
catchError((err) => {
return of(false);
})
);
}

/**
* Checks if the specified organisation exists.
* @param orgName - Name of Organisation.
*/
isOrganisationPresent(orgName: string): Observable<boolean> {
return from(octokit.orgs.get({ org: orgName, headers: GithubService.IF_NONE_MATCH_EMPTY })).pipe(
map((rawData: { status: number }) => {
return rawData.status !== ERRORCODE_NOT_FOUND;
}),
catchError((err) => {
return of(false);
})
);
}

/**
* Checks if the specified repository exists.
* @param owner - Owner of Specified Repository.
Expand All @@ -184,8 +214,7 @@ export class GithubService {
}),
catchError((err) => {
return of(false);
}),
catchError((err) => throwError(ErrorMessageService.repositoryNotPresentMessage()))
})
);
}

Expand Down
35 changes: 26 additions & 9 deletions src/app/core/services/view.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,38 @@ export class ViewService {
}

/**
* Change repository if a valid repository is provided
* @param repo New repository
* Verifies if the organisation and repository are present on GitHub.
* @param org Organisation name
* @param repo Repository name
* @returns Promise that resolves to true if the organisation and repository are present.
* @throws Error if the organisation or repository are not present.
*/
async changeRepositoryIfValid(repo: Repo) {
this.isChangingRepo.next(true);
async verifyOwnerAndRepo(owner: string, repo: string): Promise<boolean> {
const isValidOwner =
(await this.githubService.isOrganisationPresent(owner).toPromise()) ||
(await this.githubService.isUsernamePresent(owner).toPromise());
if (!isValidOwner) {
this.isChangingRepo.next(false);
throw new Error(ErrorMessageService.repoOwnerNotPresentMessage());
}

const isValidRepository = await this.githubService.isRepositoryPresent(repo.owner, repo.name).toPromise();
const isValidRepository = await this.githubService.isRepositoryPresent(owner, repo).toPromise();
if (!isValidRepository) {
this.isChangingRepo.next(false);
throw new Error(ErrorMessageService.repositoryNotPresentMessage());
}

return true;
}

/**
* Change repository if a valid repository is provided
* @param repo New repository
* @throws Error if the repository is not valid
*/
async changeRepositoryIfValid(repo: Repo) {
this.isChangingRepo.next(true);
await this.verifyOwnerAndRepo(repo.owner, repo.name);
this.changeCurrentRepository(repo);
this.isChangingRepo.next(false);
}
Expand All @@ -150,10 +170,7 @@ export class ViewService {
} else {
repo = new Repo(org, repoName);
}
const isValidRepository = await this.githubService.isRepositoryPresent(repo.owner, repo.name).toPromise();
if (!isValidRepository) {
throw new Error(ErrorMessageService.repositoryNotPresentMessage());
}
await this.verifyOwnerAndRepo(repo.owner, repo.name);
this.logger.info(`ViewService: Repo is ${repo}`);
this.setRepository(repo);
this.repoSetSource.next(true);
Expand Down
5 changes: 4 additions & 1 deletion src/app/issues-viewer/issues-viewer.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy {
return of(false);
}

return this.githubService.isRepositoryPresent(currentRepo.owner, currentRepo.name);
return (
(this.githubService.isOrganisationPresent(currentRepo.owner) || this.githubService.isUsernamePresent(currentRepo.owner)) &&
this.githubService.isRepositoryPresent(currentRepo.owner, currentRepo.name)
);
}

/**
Expand Down
15 changes: 13 additions & 2 deletions tests/services/view.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ let activatedRouteSpy: jasmine.SpyObj<ActivatedRoute>;

describe('ViewService', () => {
beforeEach(() => {
githubServiceSpy = jasmine.createSpyObj('GithubService', ['isRepositoryPresent', 'storeViewDetails']);
githubServiceSpy = jasmine.createSpyObj('GithubService', [
'isUsernamePresent',
'isOrganisationPresent',
'isRepositoryPresent',
'storeViewDetails'
]);
activatedRouteSpy = jasmine.createSpyObj('ActivatedRoute', ['snapshot']);
routerSpy = jasmine.createSpyObj('Router', ['navigate']);
repoUrlCacheServiceSpy = jasmine.createSpyObj('RepoUrlCacheService', ['cache']);
Expand Down Expand Up @@ -61,6 +66,7 @@ describe('ViewService', () => {

describe('changeRepositoryIfValid(Repo)', () => {
it('should set isChangingRepo to true at the start and false at the end', async () => {
githubServiceSpy.isOrganisationPresent.and.returnValue(of(true));
githubServiceSpy.isRepositoryPresent.and.returnValue(of(true));

const isChangingRepoNextSpy = spyOn(viewService.isChangingRepo, 'next');
Expand All @@ -74,14 +80,17 @@ describe('ViewService', () => {
});

it('should throw error if repository is not valid', async () => {
githubServiceSpy.isOrganisationPresent.and.returnValue(of(false));
githubServiceSpy.isUsernamePresent.and.returnValue(of(false));
githubServiceSpy.isRepositoryPresent.and.returnValue(of(false));

await expectAsync(viewService.changeRepositoryIfValid(WATCHER_REPO)).toBeRejectedWithError(
ErrorMessageService.repositoryNotPresentMessage()
ErrorMessageService.repoOwnerNotPresentMessage()
);
});

it('should set and navigate to new repo if repo is valid', async () => {
githubServiceSpy.isOrganisationPresent.and.returnValue(of(true));
githubServiceSpy.isRepositoryPresent.and.returnValue(of(true));

const repoChanged$Spy = spyOn(viewService.repoChanged$, 'next');
Expand Down Expand Up @@ -110,6 +119,7 @@ describe('ViewService', () => {
});

it('should set and navigate to new repo if repo is valid', async () => {
githubServiceSpy.isOrganisationPresent.and.returnValue(of(true));
githubServiceSpy.isRepositoryPresent.and.returnValue(of(true));

const repoSetSourceNext = spyOn(viewService.repoSetSource, 'next');
Expand All @@ -126,6 +136,7 @@ describe('ViewService', () => {
});

it('should throw error if repository is invalid', async () => {
githubServiceSpy.isOrganisationPresent.and.returnValue(of(true));
githubServiceSpy.isRepositoryPresent.and.returnValue(of(false));

await expectAsync(viewService.initializeCurrentRepository()).toBeRejectedWithError(ErrorMessageService.repositoryNotPresentMessage());
Expand Down

0 comments on commit 03ef9e0

Please sign in to comment.