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

Enhance error message on incorrect organisation/repo name #378

Merged
merged 11 commits into from
Jul 8, 2024
4 changes: 4 additions & 0 deletions src/app/core/services/error-message.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import { Injectable } from '@angular/core';
* Contains all error message prompts to user.
*/
export class ErrorMessageService {
public static organisationNotPresentMessage() {
return 'Invalid organisation name. Please provide a repository URL or repo name (Org/Repo Name) with a valid organisation name.';
}

public static repositoryNotPresentMessage() {
return 'Invalid repository name. Please provide Github repository URL or the repository name in the format Org/Repository Name.';
}
Expand Down
16 changes: 16 additions & 0 deletions src/app/core/services/github.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,22 @@ export class GithubService {
);
}

/**
* 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);
}),
catchError((err) => throwError(ErrorMessageService.organisationNotPresentMessage()))
);
}

/**
* Checks if the specified repository exists.
* @param owner - Owner of Specified Repository.
Expand Down
31 changes: 21 additions & 10 deletions src/app/core/services/view.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,32 @@ 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.
*/
async changeRepositoryIfValid(repo: Repo) {
this.isChangingRepo.next(true);
async verifyOrgAndRepo(org: string, repo: string): Promise<boolean> {
const isValidOrganisation = await this.githubService.isOrganisationPresent(org).toPromise();
if (!isValidOrganisation) {
throw new Error(ErrorMessageService.organisationNotPresentMessage());
}

const isValidRepository = await this.githubService.isRepositoryPresent(repo.owner, repo.name).toPromise();
const isValidRepository = await this.githubService.isRepositoryPresent(org, 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
*/
async changeRepositoryIfValid(repo: Repo) {
this.isChangingRepo.next(true);
await this.verifyOrgAndRepo(repo.owner, repo.name);
this.changeCurrentRepository(repo);
this.isChangingRepo.next(false);
}
Expand All @@ -150,10 +164,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.verifyOrgAndRepo(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.isRepositoryPresent(currentRepo.owner, currentRepo.name)
kokerinks marked this conversation as resolved.
Show resolved Hide resolved
);
}

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

describe('ViewService', () => {
beforeEach(() => {
githubServiceSpy = jasmine.createSpyObj('GithubService', ['isRepositoryPresent', 'storeViewDetails']);
githubServiceSpy = jasmine.createSpyObj('GithubService', ['isOrganisationPresent', 'isRepositoryPresent', 'storeViewDetails']);
activatedRouteSpy = jasmine.createSpyObj('ActivatedRoute', ['snapshot']);
routerSpy = jasmine.createSpyObj('Router', ['navigate']);
repoUrlCacheServiceSpy = jasmine.createSpyObj('RepoUrlCacheService', ['cache']);
Expand Down Expand Up @@ -61,6 +61,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 +75,16 @@ describe('ViewService', () => {
});

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

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

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 +113,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 +130,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
Loading