From 497311c6de83451c3aff4b19d7fc34c01ce74c00 Mon Sep 17 00:00:00 2001 From: Koh Jun Wei Date: Sat, 18 Apr 2020 23:17:38 +0800 Subject: [PATCH] Hotfix (#412) * Fix polling with large number of issues --- package.json | 2 +- src/app/core/services/github.service.ts | 35 ++++++++++++------- .../core/services/issue-comment.service.ts | 4 +-- src/app/core/services/issue.service.ts | 15 +++++--- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 58c0d773b..52b54ea46 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "CATcher", - "version": "3.2.2", + "version": "3.2.3", "main": "main.js", "scripts": { "postinstall": "npm run postinstall:electron && electron-builder install-app-deps", diff --git a/src/app/core/services/github.service.ts b/src/app/core/services/github.service.ts index bced7e41e..5a9f745cd 100644 --- a/src/app/core/services/github.service.ts +++ b/src/app/core/services/github.service.ts @@ -64,9 +64,9 @@ export class GithubService { /** * Will return an Observable with array of all of the issues in Github including the paginated issues. */ - fetchIssues(filter?: {}): Observable> { + fetchIssues(filter?: {}, forceFetch: boolean = false): Observable> { let responseInFirstPage: GithubResponse; - return from(this.getIssueAPICall(filter, 1)).pipe( + return from(this.getIssueAPICall(filter, 1, forceFetch)).pipe( map((response: GithubResponse) => { responseInFirstPage = response; return this.getNumberOfPages(response); @@ -74,7 +74,7 @@ export class GithubService { flatMap((numOfPages: number) => { const apiCalls: Observable>[] = []; for (let i = 2; i <= numOfPages; i++) { - apiCalls.push(from(this.getIssueAPICall(filter, i))); + apiCalls.push(from(this.getIssueAPICall(filter, i, forceFetch))); } return apiCalls.length === 0 ? of([]) : forkJoin(apiCalls); }), @@ -94,17 +94,18 @@ export class GithubService { ); } - fetchIssueComments(issueId: number): Observable> { + fetchIssueComments(issueId: number, forceFetch: boolean = false): Observable> { let responseInFirstPage: GithubResponse; - return from(this.getCommentsAPICall(issueId, 1)).pipe( + return from(this.getCommentsAPICall(issueId, 1, forceFetch)).pipe( map((response: GithubResponse) => { responseInFirstPage = response; + console.log('got number of pages in ', issueId); return this.getNumberOfPages(response); }), flatMap((numOfPages: number) => { const apiCalls: Observable>[] = []; for (let i = 2; i <= numOfPages; i++) { - apiCalls.push(from(this.getCommentsAPICall(issueId, i))); + apiCalls.push(from(this.getCommentsAPICall(issueId, i, forceFetch))); } return apiCalls.length === 0 ? of([]) : forkJoin(apiCalls); }), @@ -119,6 +120,7 @@ export class GithubService { collatedData.push(comment); } } + console.log('collated comments for issueId', issueId, ' ', collatedData); return collatedData; }) ); @@ -304,13 +306,22 @@ export class GithubService { return numberOfPages; } - private getIssueAPICall(filter: {}, pageNumber: number): Promise> { - return octokit.issues.listForRepo({...filter, owner: ORG_NAME, repo: REPO, sort: 'created', - direction: 'desc', per_page: 100, page: pageNumber, headers: { 'If-None-Match': this.issuesEtagManager.get(pageNumber) }}); + private getIssueAPICall(filter: {}, pageNumber: number, force: boolean = false): Promise> { + if (force) { + return octokit.issues.listForRepo({...filter, owner: ORG_NAME, repo: REPO, sort: 'created', + direction: 'desc', per_page: 100, page: pageNumber}); + } else { + return octokit.issues.listForRepo({...filter, owner: ORG_NAME, repo: REPO, sort: 'created', + direction: 'desc', per_page: 100, page: pageNumber, headers: { 'If-None-Match': this.issuesEtagManager.get(pageNumber) }}); + } } - private getCommentsAPICall(issueId: number, pageNumber: number): Promise> { - return octokit.issues.listComments({owner: ORG_NAME, repo: REPO, issue_number: issueId, page: pageNumber, per_page: 100, - headers: { 'If-None-Match': this.commentsEtagManager.get(issueId, pageNumber)}}); + private getCommentsAPICall(issueId: number, pageNumber: number, force: boolean = false): Promise> { + if (force) { + return octokit.issues.listComments({owner: ORG_NAME, repo: REPO, issue_number: issueId, page: pageNumber, per_page: 100}); + } else { + return octokit.issues.listComments({owner: ORG_NAME, repo: REPO, issue_number: issueId, page: pageNumber, per_page: 100, + headers: { 'If-None-Match': this.commentsEtagManager.get(issueId, pageNumber)}}); + } } } diff --git a/src/app/core/services/issue-comment.service.ts b/src/app/core/services/issue-comment.service.ts index faefcc34b..91446fb2c 100644 --- a/src/app/core/services/issue-comment.service.ts +++ b/src/app/core/services/issue-comment.service.ts @@ -15,8 +15,8 @@ export class IssueCommentService { constructor(private githubService: GithubService) {} - getGithubComments(issueId: number): Observable { - return this.githubService.fetchIssueComments(issueId).pipe( + getGithubComments(issueId: number, forceFetch: boolean = false): Observable { + return this.githubService.fetchIssueComments(issueId, forceFetch).pipe( map((githubComments: Array) => { this.updateLocalIssueComments(issueId, githubComments); return githubComments.map(rawJsonData => {...rawJsonData}); diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index 3c6646419..57d6c2a7d 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -29,6 +29,8 @@ export class IssueService { private issueTeamFilter = 'All Teams'; readonly MINIMUM_MATCHES = 1; + private toForceFetch = true; + constructor(private githubService: GithubService, private userService: UserService, private phaseService: PhaseService, @@ -266,10 +268,10 @@ export class IssueService { const issuesPerFilter = []; if (filters.length === 0) { - issuesPerFilter.push(this.githubService.fetchIssues({})); + issuesPerFilter.push(this.githubService.fetchIssues({}, this.toForceFetch)); } for (const filter of filters) { - issuesPerFilter.push(this.githubService.fetchIssues(filter)); + issuesPerFilter.push(this.githubService.fetchIssues(filter, this.toForceFetch)); } return forkJoin(issuesPerFilter).pipe( @@ -283,6 +285,9 @@ export class IssueService { return mappingFunctions.length === 0 ? of([]) : forkJoin(mappingFunctions); }), map(() => { + if (this.toForceFetch) { + this.toForceFetch = false; + } return Object.values(this.issues); }) ); @@ -359,20 +364,20 @@ export class IssueService { case Phase.phaseBugReporting: return of(Issue.createPhaseBugReportingIssue(githubIssue)); case Phase.phaseTeamResponse: - return this.issueCommentService.getGithubComments(githubIssue.number).pipe( + return this.issueCommentService.getGithubComments(githubIssue.number, this.toForceFetch).pipe( map((githubComments: GithubComment[]) => { return Issue.createPhaseTeamResponseIssue(githubIssue, githubComments, this.dataService.getTeam(this.extractTeamIdFromGithubIssue(githubIssue))); }) ); case Phase.phaseTesterResponse: - return this.issueCommentService.getGithubComments(githubIssue.number).pipe( + return this.issueCommentService.getGithubComments(githubIssue.number, this.toForceFetch).pipe( map((githubComments: GithubComment[]) => { return Issue.createPhaseTesterResponseIssue(githubIssue, githubComments); }) ); case Phase.phaseModeration: - return this.issueCommentService.getGithubComments(githubIssue.number).pipe( + return this.issueCommentService.getGithubComments(githubIssue.number, this.toForceFetch).pipe( map((githubComments: GithubComment[]) => { return Issue.createPhaseModerationIssue(githubIssue, githubComments, this.dataService.getTeam(this.extractTeamIdFromGithubIssue(githubIssue)));