From f68b48cecec6ffc67871dbc0a61e402dabb0c9df Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 27 May 2016 11:34:50 -0700 Subject: [PATCH] Initial pr-triage --- .vscode/settings.json | 10 + design_notes.md | 24 +++ src/app/+sync/sync.component.html | 34 +++- src/app/+sync/sync.component.ts | 22 ++- src/app/+triage-pr/triage-pr.component.css | 11 ++ src/app/+triage-pr/triage-pr.component.html | 82 +++++++- src/app/+triage-pr/triage-pr.component.ts | 22 ++- src/app/github/store.ts | 207 ++++++++++++++++---- src/app/github/v3.ts | 12 +- src/app/periscope.component.html | 9 +- src/app/periscope.component.ts | 21 +- src/app/pr-board.service.spec.ts | 17 ++ src/app/pr-board.service.ts | 139 +++++++++++++ src/app/pr/index.ts | 1 + src/app/pr/pr.component.css | 0 src/app/pr/pr.component.html | 3 + src/app/pr/pr.component.spec.ts | 46 +++++ src/app/pr/pr.component.ts | 16 ++ src/system-config.ts | 1 + 19 files changed, 605 insertions(+), 72 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 design_notes.md create mode 100644 src/app/pr-board.service.spec.ts create mode 100644 src/app/pr-board.service.ts create mode 100644 src/app/pr/index.ts create mode 100644 src/app/pr/pr.component.css create mode 100644 src/app/pr/pr.component.html create mode 100644 src/app/pr/pr.component.spec.ts create mode 100644 src/app/pr/pr.component.ts diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..0a5a1ec --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,10 @@ +// Place your settings in this file to overwrite default and user settings. +{ + "files.exclude": { + "**/.git": true, + "**/.svn": true, + "**/.DS_Store": true, + "dist": true, + "tmp": true + } +} \ No newline at end of file diff --git a/design_notes.md b/design_notes.md new file mode 100644 index 0000000..0edeb22 --- /dev/null +++ b/design_notes.md @@ -0,0 +1,24 @@ +## FireBase Schema + +### `/github_webhook_events` + +This is a list of webhook posts from the github which needs to be synced into FireBase + +### `/github/prs/:number` + +Full details of a particular PR. + +### `/digest/prs/[open|closed]/:number` + +``` +{ + number: number, + title: string + assigned: string, + author: string, + created_at: number, + updated_at: number, + labels: string[], + comments: number +} +``` diff --git a/src/app/+sync/sync.component.html b/src/app/+sync/sync.component.html index 727cf6a..6870b55 100644 --- a/src/app/+sync/sync.component.html +++ b/src/app/+sync/sync.component.html @@ -1,7 +1,29 @@ - +

WebHook Queue

+ + + + + + + + + + + + + + + + + +
keyactionissuePRcommentlabel
+ {{event.$key}} + {{event?.action}}{{!!event?.issue}}{{!!event?.pull_request}}{{!!event?.comment}}{{!!event?.label}}
+

All Prs

+ + #{{pr.number}} + +
- +
+
diff --git a/src/app/+sync/sync.component.ts b/src/app/+sync/sync.component.ts index c1e4011..c2437dd 100644 --- a/src/app/+sync/sync.component.ts +++ b/src/app/+sync/sync.component.ts @@ -1,6 +1,6 @@ import { Component, OnInit } from '@angular/core'; -import { PullRequest, Issue, BaseIssue } from '../github/v3'; -import { GithubStore } from '../github/store'; +import { PullRequest, Issue, BaseIssue, Event } from '../github/v3'; +import { GithubStore, Digest } from '../github/store'; import { Observable } from 'rxjs/Observable'; import { FirebaseListObservable, FirebaseObjectObservable } from 'angularfire2'; @@ -11,17 +11,21 @@ import { FirebaseListObservable, FirebaseObjectObservable } from 'angularfire2'; templateUrl: 'sync.component.html', styleUrls: ['sync.component.css'] }) -export class SyncComponent implements OnInit { - prs: FirebaseListObservable; +export class SyncComponent { + prs: FirebaseListObservable; + events: FirebaseListObservable; constructor(private store: GithubStore) { - this.prs = store.getPrs(); - } - - ngOnInit() { + this.events = store.getWebhookEvents(); + this.prs = store.getOpenPrDigests(); } - sync(prNumber: string) { + syncPr(number: string) { + this.store.updatePr(Number.parseInt(number)); + } + + syncAllPRs() { this.store.updatePrs(); } + } diff --git a/src/app/+triage-pr/triage-pr.component.css b/src/app/+triage-pr/triage-pr.component.css index e69de29..33d3075 100644 --- a/src/app/+triage-pr/triage-pr.component.css +++ b/src/app/+triage-pr/triage-pr.component.css @@ -0,0 +1,11 @@ +table { + border: 1px solid black; + border-collapse: collapse; +} + +td,th { + vertical-align: top; + border: 1px solid black; + border-collapse: collapse; + padding: 0.3em; +} \ No newline at end of file diff --git a/src/app/+triage-pr/triage-pr.component.html b/src/app/+triage-pr/triage-pr.component.html index af48f13..d5a3da9 100644 --- a/src/app/+triage-pr/triage-pr.component.html +++ b/src/app/+triage-pr/triage-pr.component.html @@ -1,3 +1,79 @@ -

- triage-pr works! -

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
HourHoursDayDaysWeekWeeksMonthMonths
Untriaged {{state.untriaged.length}}
Assigned {{state.assigned.length}}
CleanUp {{state.cleanup.length}}
Reviewed {{state.reviewed.length}}
Ready {{state.ready.length}}
Other {{state.other.length}}
diff --git a/src/app/+triage-pr/triage-pr.component.ts b/src/app/+triage-pr/triage-pr.component.ts index e9d7b31..97f48f8 100644 --- a/src/app/+triage-pr/triage-pr.component.ts +++ b/src/app/+triage-pr/triage-pr.component.ts @@ -1,16 +1,24 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, Injectable, forwardRef } from '@angular/core'; +import { PrComponent } from '../pr/pr.component'; +import { PrBoardService, Staleness } from '../pr-board.service'; + @Component({ moduleId: module.id, selector: 'app-triage-pr', templateUrl: 'triage-pr.component.html', - styleUrls: ['triage-pr.component.css'] + styleUrls: ['triage-pr.component.css'], + directives: [PrComponent] }) -export class TriagePrComponent implements OnInit { - - constructor() {} +export class TriagePrComponent { + state: PrBoardService; - ngOnInit() { + constructor(prState: PrBoardService) { + console.log('Triage'); + this.state = prState; + } + + sum(s: Staleness) { + return s.length; } - } diff --git a/src/app/github/store.ts b/src/app/github/store.ts index 776344e..4965cde 100644 --- a/src/app/github/store.ts +++ b/src/app/github/store.ts @@ -1,76 +1,211 @@ import { Injectable } from '@angular/core'; -import { PullRequest, Issue, BaseIssue } from './v3'; -import { Http, Response } from '@angular/http'; +import { PullRequest, Issue, BaseIssue, LabelRef, Event } from './v3'; +import { Http, Response, Headers } from '@angular/http'; import { Observable } from 'rxjs/Observable'; import { AngularFire, FirebaseListObservable, - FirebaseObjectObservable + FirebaseObjectObservable, + FirebaseAuthState } from 'angularfire2'; +const EVENTS = '/github_webhook_events'; @Injectable() export class GithubStore { - constructor(private af: AngularFire, private http: Http) {} + constructor(private af: AngularFire, private http: Http) { + this._consumeEvents(); + } - getPrs(): FirebaseListObservable { - return this.af.database.list('/prs') + private _consumeEvents() { + var events = this.getWebhookEvents(1); + var subscription = events.subscribe((events: Event[]) => { + if (events.length) { + var event = events[0]; + this._consumeEvent(this.af.database.object(EVENTS + '/' + event['$key'])); + } + }) } - getPr(prNumber: number): FirebaseObjectObservable { - return this.af.database.object('/prs/' + prNumber); + private _consumeEvent(event: FirebaseObjectObservable) { + event.subscribe((e: Event) => { + if (!e) return; + console.log(e); + switch (e && e.action) { + case "synchronize": + case "labeled": + case "unlabeled": + case "assigned": + case "unassigned": + case "opened": + case "closed": + case "reopened": + case "edited": + case "created": + case "deleted": + if (e.issue) { + this._updateObject(ISSUE_EXTRACTOR, e.issue); + event.remove(); + } else if (e.pull_request) { + this._updateObject(PR_EXTRACTOR, e.pull_request); + event.remove(); + } else { + console.error('unknown event: ', e); + } + break; + case "deleted": + if (e.comment) { + // We don't process comments + event.remove(); + } + default: + console.error('unknown action: ' + (e.action)); + } + }) } - updatePr(prNumber: number): FirebaseObjectObservable { - this._get('/pulls/' + prNumber ) - .subscribe( - (prResponse: Response) => this._updateObject('/prs/', prResponse.json()), - (error: Response) => console.error(error) - ); - //TODO: how do I return FirebaseObjectObservable? - return null; + getOpenPrDigests(): FirebaseListObservable { + return this.af.database.list(PR_EXTRACTOR.digestUrl + '/open'); + } + + getWebhookEvents(limitTo: number = 10): FirebaseListObservable { + return this.af.database.list(EVENTS, {query: {limitToFirst: limitTo}}); + } + + updatePr(prNumber: number): void { + return this._update(PR_EXTRACTOR, prNumber); } updatePrs() { + return this._updateAll(PR_EXTRACTOR) + } + + private _updateAll(extractor: Extractor): void { var fetchPage = (page: number) => { this - ._get('/pulls', {page: page, state: 'all'}) + ._get(extractor.githubUrl, {page: page, state: 'all'}) .subscribe((prsResponse: Response) => { var prs: PullRequest[] = prsResponse.json(); - prs.forEach((pr: PullRequest) => this._updateObject('/prs/', pr)); + prs.forEach((pr: PullRequest) => { + this._updateObject(extractor, pr); + }); if (prs.length) fetchPage(page + 1); }); } fetchPage(0); } - - - getIssues(): FirebaseListObservable { - return this.af.database.list('/issues') - } - - getIssue(issueNumber: number): FirebaseObjectObservable { - return this.af.database.object('/issues/' + issueNumber); - } - updateIssue(issueNumber: number): FirebaseObjectObservable { - this._get('/issues/' + issueNumber ) + private _update(extractor: Extractor, number: number): void { + this + ._get(extractor.githubUrl + '/' + number) .subscribe( - (issueResponse: Response) => this._updateObject('/issues/', issueResponse.json()), + (prResponse: Response) => this._updateObject(extractor, prResponse.json()), (error: Response) => console.error(error) ); - //TODO: how do I return FirebaseObjectObservable? - return null; + //TODO: how do I return FirebaseObjectObservable? } - - private _updateObject(path: string, prOrIssue: PullRequest | Issue) { - this.af.database.object(path + prOrIssue.number).update(prOrIssue); + + private _updateObject(extractor: Extractor, prOrIssue: PullRequest | Issue): void { + this._get(ISSUE_EXTRACTOR.githubUrl + '/' + prOrIssue.number + '/labels') + .subscribe((res: Response) => { + var labels = res.json(); + console.log('Updating: ', prOrIssue.number, labels); + var db = this.af.database; + var open = db.object(extractor.rawUrl + '/open/' + prOrIssue.number); + var close = db.object(extractor.rawUrl + '/close/' + prOrIssue.number); + var openDigest = db.object(extractor.digestUrl + '/open/' + prOrIssue.number); + var closeDigest = db.object(extractor.digestUrl + '/close/' + prOrIssue.number); + var digest = extractor.digest(prOrIssue, labels) + if (prOrIssue.closed_at) { + close.set(prOrIssue); + closeDigest.set(digest); + open.remove(); + openDigest.remove(); + } else { + open.set(prOrIssue); + openDigest.set(digest); + close.remove(); + closeDigest.remove(); + } + }); } private _get(path: string, params: {[k:string]: any} = {}): Observable { + var authState: FirebaseAuthState = this.af.auth.getAuth(); + var accessToken: string = authState.github.accessToken; var qParams = []; Object.keys(params).forEach((key) => qParams.push(key + '=' + params[key])); return this.http.get('https://api.github.com/repos/angular/angular' - + path + '?' + qParams.join('&')); + + path + '?' + qParams.join('&'), { + headers: new Headers({"Authorization": "token " + accessToken}) + }); } } + +abstract class Extractor { + githubUrl: string; + rawUrl: string; + digestUrl: string; + + digest(prOrIssue: PullRequest | Issue, labels: LabelRef[]): Digest { + return { + number: prOrIssue.number, + title: prOrIssue.title, + assigned: prOrIssue.assignee ? prOrIssue.assignee.login : null, + author: prOrIssue.user ? prOrIssue.user.login : null, + created_at: parseDate(prOrIssue.created_at), + updated_at: parseDate(prOrIssue.updated_at), + labels: extractLabels(labels), + comments: prOrIssue.comments || 0 + }; + } +} + +class PrExtractor extends Extractor { + githubUrl = '/pulls'; + rawUrl = '/github/pulls'; + digestUrl = '/digest/pulls'; + +} + +class IssueExtractor extends Extractor { + githubUrl = '/issues'; + rawUrl = '/github/issues'; + digestUrl = '/digest/issues'; + +} + +export interface Digest { + number: number; + title: string; + assigned: string; + author: string; + created_at: number; + updated_at: number; + labels: {[key:string]: string}; + comments: number; +} + +const LABEL_REGEXP = /^(\w+)(\d*)\:\s*(.*)$/; + +function extractLabels(labels: LabelRef[]): {[key:string]: string} { + var labelsMap: {[key:string]: string} = {}; + (labels || []).map((ref) => { + var parts = LABEL_REGEXP.exec(ref.name); + var key = parts[1]; + var value = parts[3]; + if (parts[2]) { + value = parts[2] + ': ' + value; + } + labelsMap[key] = value; + }); + console.log('Labels', labelsMap, labels); + return labelsMap; +} + +function parseDate(date: string): number { + return date ? new Date(date).getTime() : null; +} + +const PR_EXTRACTOR = new PrExtractor(); +const ISSUE_EXTRACTOR = new IssueExtractor(); diff --git a/src/app/github/v3.ts b/src/app/github/v3.ts index 1a1e6c9..413c753 100644 --- a/src/app/github/v3.ts +++ b/src/app/github/v3.ts @@ -69,7 +69,6 @@ export interface BaseIssue { export interface Issue extends BaseIssue { repository_url: string, - labels: LabelRef[], state: string, locked: boolean, closed_at: any, @@ -102,4 +101,15 @@ export interface PullRequest extends BaseIssue { additions: number, deletions: number, changed_files: number +} + + +export interface Event { + action: string, + comment?: Comment, + issue?: Issue, + pull_request?: PullRequest, + organization: any, + repository: any, + sender: User } \ No newline at end of file diff --git a/src/app/periscope.component.html b/src/app/periscope.component.html index e1d43a0..08e4874 100644 --- a/src/app/periscope.component.html +++ b/src/app/periscope.component.html @@ -1,10 +1,9 @@ -

- {{title}} -

- [ Data Sync | Triage PR -| auth +| auth + + signout: {{authState?.github?.username}} + ] \ No newline at end of file diff --git a/src/app/periscope.component.ts b/src/app/periscope.component.ts index bd6ed1d..21033c4 100644 --- a/src/app/periscope.component.ts +++ b/src/app/periscope.component.ts @@ -3,13 +3,15 @@ import { TriagePrComponent } from './+triage-pr'; import { Routes , ROUTER_DIRECTIVES, ROUTER_PROVIDERS} from '@angular/router'; import { SyncComponent } from './+sync'; import { HTTP_PROVIDERS } from '@angular/http' +import { PrBoardService } from './pr-board.service'; import { AngularFire, FIREBASE_PROVIDERS, defaultFirebase, firebaseAuthConfig, AuthProviders, - AuthMethods + AuthMethods, + FirebaseAuthState } from 'angularfire2'; import { GithubStore } from './github/store'; @@ -25,10 +27,11 @@ import { GithubStore } from './github/store'; defaultFirebase('https://ngperiscope.firebaseio.com'), firebaseAuthConfig({ provider: AuthProviders.Github, - method: AuthMethods.Redirect + method: AuthMethods.Popup }), ROUTER_PROVIDERS, - GithubStore + GithubStore, + PrBoardService ] }) @Routes([ @@ -37,10 +40,18 @@ import { GithubStore } from './github/store'; ]) export class PeriscopeAppComponent { title = 'periscope works!'; + authState: FirebaseAuthState; - constructor(private af: AngularFire) { } + constructor(private af: AngularFire) { + this.authState = af.auth.getAuth(); + } auth() { - this.af.auth.login(); + this.af.auth.login().then((authState) => this.authState = authState); + } + + logout() { + this.af.auth.logout(); + this.authState = null; } } diff --git a/src/app/pr-board.service.spec.ts b/src/app/pr-board.service.spec.ts new file mode 100644 index 0000000..142eb9c --- /dev/null +++ b/src/app/pr-board.service.spec.ts @@ -0,0 +1,17 @@ +import { + beforeEachProviders, + it, + describe, + expect, + inject +} from '@angular/core/testing'; +import { PrBoardService } from './pr-board.service'; + +describe('PrBoard Service', () => { + beforeEachProviders(() => [PrBoardService]); + + it('should ...', + inject([PrBoardService], (service: PrBoardService) => { + expect(service).toBeTruthy(); + })); +}); diff --git a/src/app/pr-board.service.ts b/src/app/pr-board.service.ts new file mode 100644 index 0000000..b1aad25 --- /dev/null +++ b/src/app/pr-board.service.ts @@ -0,0 +1,139 @@ +import { GithubStore, Digest } from './github/store'; +import { Injectable } from '@angular/core'; + +const Label = { + LGTM: 'pr_state: LGTM', + Merge: 'pr_action: merge', + Cleanup: 'pr_action: cleanup' +} + +@Injectable() +export class PrBoardService { + + untriaged = new Staleness(); + assigned = new Staleness(); + reviewed = new Staleness(); + cleanup = new Staleness(); + ready = new Staleness(); + other = new Staleness(); + + constructor(store: GithubStore) { + store.getOpenPrDigests().subscribe((prs) => this.classify(prs)); + } + + classify(prs: Digest[]) { + this.untriaged.reset(); + this.assigned.reset(); + this.reviewed.reset(); + this.cleanup.reset(); + this.ready.reset(); + this.other.reset(); + prs.forEach(pr => this.categorize(pr)); + this.untriaged.sort(); + this.assigned.sort(); + this.reviewed.sort(); + this.cleanup.sort(); + this.ready.sort(); + this.other.sort(); + } + + + categorize(pr: Digest) { + var labels = pr.labels || {}; + if (pr.number == 8350) { + console.log(pr); + } + if (!pr.assigned) { + this.untriaged.add(pr) + } else if (labels['pr_action'] == 'cleanup') { + this.cleanup.add(pr); + } else if (labels['pr_state'] == 'LGTM') { + if (labels['pr_action'] == 'merge') { + this.ready.add(pr); + } else { + this.reviewed.add(pr); + } + } else { + this.assigned.add(pr); + } + } +} + +export class Staleness { + static MINUTE = 60*1000; + static HOUR = 60*Staleness.MINUTE; + static HOURS = 2*Staleness.HOUR; + static DAY = 24*Staleness.HOUR; + static DAYS = 2*Staleness.DAY; + static WEEK = 7*Staleness.DAY; + static WEEKS = 2*Staleness.WEEK; + static MONTH = 4*Staleness.WEEK; + static MONTHS = 2*Staleness.MONTH; + static BUCKETS = [ + Staleness.HOUR, + Staleness.HOURS, + Staleness.DAY, + Staleness.DAYS, + Staleness.WEEK, + Staleness.WEEKS, + Staleness.MONTH, + Staleness.MONTHS, + Number.MAX_VALUE + ]; + + hour: Digest[] = []; + hours: Digest[] = []; + day: Digest[] = []; + days: Digest[] = []; + week: Digest[] = []; + weeks: Digest[] = []; + month: Digest[] = []; + months: Digest[] = []; + + buckets: Digest[][]; + + constructor() { + this.buckets = [ + this.hour, + this.hours, + this.day, + this.days, + this.week, + this.weeks, + this.month, + this.months + ]; + } + + get length() { + return 0 + + this.day.length + + this.days.length + + this.week.length + + this.weeks.length + + this.month.length + + this.months.length; + } + + reset () { + this.buckets.forEach(bucket => bucket.length = 0); + } + + add(d: Digest) { + var delay = new Date().getTime() - d.updated_at; + for(var index = 0; index < Staleness.BUCKETS.length; index++ ) { + if (delay < Staleness.BUCKETS[index]) { + this.buckets[index].push(d); + return; + } + } + } + + sort() { + this.buckets.forEach(bucket => bucket.sort(Staleness.sortByAge)); + } + + static sortByAge(a: Digest, b: Digest): number { + return 0; + } +} diff --git a/src/app/pr/index.ts b/src/app/pr/index.ts new file mode 100644 index 0000000..9772630 --- /dev/null +++ b/src/app/pr/index.ts @@ -0,0 +1 @@ +export { PrComponent } from './pr.component'; diff --git a/src/app/pr/pr.component.css b/src/app/pr/pr.component.css new file mode 100644 index 0000000..e69de29 diff --git a/src/app/pr/pr.component.html b/src/app/pr/pr.component.html new file mode 100644 index 0000000..78a146e --- /dev/null +++ b/src/app/pr/pr.component.html @@ -0,0 +1,3 @@ +#{{prDigest.number}} diff --git a/src/app/pr/pr.component.spec.ts b/src/app/pr/pr.component.spec.ts new file mode 100644 index 0000000..34570f8 --- /dev/null +++ b/src/app/pr/pr.component.spec.ts @@ -0,0 +1,46 @@ +import { + beforeEach, + beforeEachProviders, + describe, + expect, + it, + inject, +} from '@angular/core/testing'; +import { ComponentFixture, TestComponentBuilder } from '@angular/compiler/testing'; +import { Component } from '@angular/core'; +import { By } from '@angular/platform-browser'; +import { PrComponent } from './pr.component'; + +describe('Component: Pr', () => { + let builder: TestComponentBuilder; + + beforeEachProviders(() => [PrComponent]); + beforeEach(inject([TestComponentBuilder], function (tcb: TestComponentBuilder) { + builder = tcb; + })); + + it('should inject the component', inject([PrComponent], + (component: PrComponent) => { + expect(component).toBeTruthy(); + })); + + it('should create the component', inject([], () => { + return builder.createAsync(PrComponentTestController) + .then((fixture: ComponentFixture) => { + let query = fixture.debugElement.query(By.directive(PrComponent)); + expect(query).toBeTruthy(); + expect(query.componentInstance).toBeTruthy(); + }); + })); +}); + +@Component({ + selector: 'test', + template: ` + + `, + directives: [PrComponent] +}) +class PrComponentTestController { +} + diff --git a/src/app/pr/pr.component.ts b/src/app/pr/pr.component.ts new file mode 100644 index 0000000..0d29b09 --- /dev/null +++ b/src/app/pr/pr.component.ts @@ -0,0 +1,16 @@ +import { Component, Input } from '@angular/core'; +import { Digest } from '../github/store'; + +@Component({ + moduleId: module.id, + selector: 'app-pr', + templateUrl: 'pr.component.html', + styleUrls: ['pr.component.css'] +}) +export class PrComponent { + prDigest: Digest; + @Input() + set pr(pr: number | Digest) { + this.prDigest = pr; + } +} diff --git a/src/system-config.ts b/src/system-config.ts index bcc5ed2..dac7f6f 100644 --- a/src/system-config.ts +++ b/src/system-config.ts @@ -37,6 +37,7 @@ const barrels: string[] = [ 'app/shared', 'app/+triage-pr', 'app/+sync', + 'app/pr', /** @cli-barrel */ ];