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 settings form with Copilot fields and adoption metrics #65

Closed
wants to merge 29 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
26a0491
Enhance settings form: add fields for Copilot licenses and developer …
MattG57 Nov 22, 2024
9649a20
Refactor dashboard layout: implement tabbed interface and enhance gri…
MattG57 Nov 22, 2024
cf150f3
Add database connection status to setup response and implement databa…
austenstone Nov 22, 2024
33e5219
Implement database connection and refactor database initialization logic
austenstone Nov 22, 2024
615299a
Refactor survey and metrics services, update component templates, and…
austenstone Nov 23, 2024
66d9b81
Refactor logging level, remove console logs, and update settings serv…
austenstone Nov 23, 2024
7dab661
Refactor setup service to simplify getSetupStatus method, enhance err…
austenstone Nov 23, 2024
1f4b47d
Refactor services and tests: update service imports, remove unused te…
austenstone Nov 24, 2024
6eec8c4
Refactor GitHub disconnect method, enhance usage controller query log…
austenstone Nov 25, 2024
fd62476
Refactor GitHub query service logging, enforce required relationships…
austenstone Nov 25, 2024
a33b843
Refactor application code: remove console logs, enhance error handlin…
austenstone Nov 25, 2024
2e49b34
continued improvement of the Status grid and status array logic
MattG57 Nov 25, 2024
9cf43d3
Refactor services: enforce required relationships in SeatsService, im…
austenstone Nov 25, 2024
14b3323
Refactor services: enhance error logging in SeatsController, improve …
austenstone Nov 25, 2024
6d9883b
Refactor services: update method names for clarity, enhance logging l…
austenstone Nov 26, 2024
1566b45
Merge branch 'main' into Enhance-Settings-w/License-count
austenstone Nov 26, 2024
6c33960
Comment out the Copilot Licenses input field in settings component
austenstone Nov 26, 2024
a1a3260
Refactor services: update survey owner to org, enhance error logging,…
austenstone Nov 26, 2024
4786002
Refactor services: remove console logs from various components to enh…
austenstone Nov 26, 2024
c192669
Refactor tests: remove survey test file to streamline test suite and …
austenstone Nov 26, 2024
9124941
Delete backend/.env copy
austenstone Nov 26, 2024
582b82b
Merge branch 'main' into enterprise
austenstone Nov 26, 2024
e6917df
Refactor database connection: use parameterized query for database cr…
austenstone Nov 26, 2024
3e5d4c7
Merge branch 'enterprise' of github.com:austenstone/github-value into…
austenstone Nov 26, 2024
23a6c67
Refactor services and components: streamline imports, enhance type sa…
austenstone Nov 26, 2024
9a870d5
Remove unused OAuth properties from GitHub configuration and add comm…
austenstone Nov 26, 2024
db3c752
Refactor code structure: update target values method for improved rea…
austenstone Nov 26, 2024
9a8c2c3
Merge pull request #69 from austenstone/enterprise
austenstone Nov 26, 2024
29ab3ae
Update service configurations and improve code structure
austenstone Nov 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactor services: update survey owner to org, enhance error logging,…
… and modify database synchronization strategy
austenstone committed Nov 26, 2024
commit a1a326011368e4e0e4cdc08d54b5d3677b59c3ec
7 changes: 7 additions & 0 deletions backend/src/app.ts
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ import whyIsNodeRunning from 'why-is-node-running';

class App {
eListener?: http.Server;
baseUrl?: string;

constructor(
public e: Express,
@@ -45,6 +46,9 @@ class App {
await this.github.smee.connect();
logger.debug(error);
logger.error('Failed to start application ❌');
if (error instanceof Error) {
logger.error(error.message);
}
}
}

@@ -102,6 +106,9 @@ class App {
if (settings.metricsCronExpression) {
this.github.cronExpression = settings.metricsCronExpression;
}
if (settings.baseUrl) {
this.baseUrl = settings.baseUrl;
}
})
.finally(async () => {
await this.github.smee.connect()
59 changes: 29 additions & 30 deletions backend/src/controllers/survey.controller.ts
Original file line number Diff line number Diff line change
@@ -6,47 +6,46 @@ import app from '../app.js';

class SurveyController {
async createSurvey(req: Request, res: Response): Promise<void> {
let survey: Survey;
try {
const survey = await surveyService.updateSurvey({
const _survey = await surveyService.updateSurvey({
...req.body,
status: 'completed'
})
if (!survey) throw new Error('Survey not found');
if (!_survey) throw new Error('Survey not found');
survey = _survey;
res.status(201).json(survey);
try {
const { installation, octokit } = await app.github.getInstallation(survey.org);
const surveyUrl = new URL(`copilot/surveys/${survey.id}`, installation.html_url);
} catch (error) {
res.status(500).json(error);
return;
}
try {
const { installation, octokit } = await app.github.getInstallation(survey.org);
const surveyUrl = new URL(`copilot/surveys/${survey.id}`, app.baseUrl);

if (!survey.repo || !survey.org || !survey.prNumber) {
logger.warn('Cannot process survey comment: missing survey data');
return;
}
const comments = await octokit.rest.issues.listComments({
if (!survey.repo || !survey.org || !survey.prNumber) {
logger.warn('Cannot process survey comment: missing survey data');
return;
}
const comments = await octokit.rest.issues.listComments({
owner: survey.org,
repo: survey.repo,
issue_number: survey.prNumber
});
const comment = comments.data.find(comment => comment.user?.login.startsWith(installation.app_slug));
if (comment) {
octokit.rest.issues.updateComment({
owner: survey.org,
repo: survey.repo,
issue_number: survey.prNumber
comment_id: comment.id,
body: `Thanks for filling out the [copilot survey](${surveyUrl.toString()}) @${survey.userId}!`
});
if (installation.account?.login) {
logger.warn('Cannot process survey comment: GitHub App installation or slug not found');
return;
}
const comment = comments.data.find(comment => comment.user?.login.startsWith(survey.userId));
if (comment) {
octokit.rest.issues.updateComment({
owner: survey.org,
repo: survey.repo,
comment_id: comment.id,
body: `Thanks for filling out the [copilot survey](${surveyUrl.toString()}) @${survey.userId}!`
});
} else {
logger.info(`No comment found for survey from ${installation.account?.login}`);
}
} catch (error) {
logger.error('Error updating survey comment', error);
throw error;
} else {
logger.info(`No comment found for survey from ${survey.org}`);
}
} catch (error) {
res.status(500).json(error);
logger.error('Error updating survey comment', error);
throw error;
}
}

3 changes: 1 addition & 2 deletions backend/src/controllers/webhook.controller.ts
Original file line number Diff line number Diff line change
@@ -20,8 +20,7 @@ export const setupWebhookListeners = (github: App) => {
timeUsedFor: '',
})

const installation = app.github.installations.find(i => i.installation.id === payload.installation?.id);
const surveyUrl = new URL(`copilot/surveys/new/${survey.id}`, installation?.installation.html_url);
const surveyUrl = new URL(`copilot/surveys/new/${survey.id}`, app.baseUrl);

surveyUrl.searchParams.append('url', payload.pull_request.html_url);
surveyUrl.searchParams.append('author', payload.pull_request.user.login);
2 changes: 1 addition & 1 deletion backend/src/database.ts
Original file line number Diff line number Diff line change
@@ -80,7 +80,7 @@ class Database {
await sequelize.authenticate()
await this.initializeModels(sequelize);
this.sequelize = sequelize;
await sequelize.sync({ force: true }).then(() => {
await sequelize.sync({ alter: true }).then(() => {
logger.info('Database models were synchronized successfully');
})
} catch (error) {
11 changes: 5 additions & 6 deletions backend/src/github.ts
Original file line number Diff line number Diff line change
@@ -155,14 +155,13 @@ class GitHub {
}>((resolve, reject) => {
this.app?.eachInstallation(async ({ installation, octokit }) => {
if (
(typeof id === 'string' && id !== installation.account?.login) ||
id !== installation.id
(typeof id === 'string' && id === installation.account?.login) ||
id === installation.id
) {
return;
console.log('FOUND IT!!!')
resolve({ installation, octokit });
}
resolve({ installation, octokit });
});
reject('Installation not found');
}).finally(() => reject('Installation not found'));
});
}

10 changes: 5 additions & 5 deletions backend/src/services/copilot.seats.service.ts
Original file line number Diff line number Diff line change
@@ -82,7 +82,7 @@ class SeatsService {
async insertSeats(org: string, data: SeatEntry[], team?: string) {
const queryAt = new Date();
for (const seat of data) {
const assignee = await Member.findOrCreate({
const [assignee] = await Member.findOrCreate({
where: { id: seat.assignee.id },
defaults: {
org,
@@ -108,7 +108,7 @@ class SeatsService {
}
});

const assigningTeam = seat.assigning_team ? await Team.findOrCreate({
const [assigningTeam] = seat.assigning_team ? await Team.findOrCreate({
where: { id: seat.assigning_team.id },
defaults: {
org,
@@ -125,7 +125,7 @@ class SeatsService {
members_url: seat.assigning_team.members_url,
repositories_url: seat.assigning_team.repositories_url
}
}) : null;
}) : [null];

await Seat.create({
queryAt,
@@ -137,8 +137,8 @@ class SeatsService {
last_activity_at: seat.last_activity_at,
last_activity_editor: seat.last_activity_editor,
plan_type: seat.plan_type,
assignee_id: assignee[0].id,
assigning_team_id: assigningTeam?.[0].id
assignee_id: assignee.id,
assigning_team_id: assigningTeam?.id
});
}
}
5 changes: 3 additions & 2 deletions backend/src/services/logger.ts
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ export const appName = packageJson.name || 'GitHub Value';

const logger = bunyan.createLogger({
name: appName,
level: 'debug',
serializers: {
...bunyan.stdSerializers,
req: (req: Request) => ({
@@ -28,7 +29,7 @@ const logger = bunyan.createLogger({
}),
res: (res: Response) => ({
statusCode: res.statusCode
})
}),
},
streams: [
{
@@ -40,7 +41,7 @@ const logger = bunyan.createLogger({
stream: process.stderr
},
{
path: `${logsDir}/debug.log`,
path: `${logsDir}/debug.json`,
period: '1d',
count: 14,
level: 'debug'
3 changes: 2 additions & 1 deletion backend/src/services/query.service.ts
Original file line number Diff line number Diff line change
@@ -120,7 +120,8 @@ class QueryService {

logger.info(`${org} seat assignments updated`);
} catch (error) {
logger.error('Error querying copilot seat assignments', error);
logger.debug(error)
logger.error('Error querying copilot seat assignments');
}
}

Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import { CopilotMetrics } from '../../../services/api/metrics.service.interfaces
import { ActivityResponse, Seat, SeatService } from '../../../services/api/seat.service';
import { MembersService } from '../../../services/api/members.service';
import { CopilotSurveyService, Survey } from '../../../services/api/copilot-survey.service';
import { forkJoin, Subject, takeUntil } from 'rxjs';
import { forkJoin, takeUntil } from 'rxjs';
import { AdoptionChartComponent } from '../copilot-value/adoption-chart/adoption-chart.component';
import { DailyActivityChartComponent } from '../copilot-value/daily-activity-chart/daily-activity-chart.component';
import { TimeSavedChartComponent } from '../copilot-value/time-saved-chart/time-saved-chart.component';
@@ -86,7 +86,6 @@ export class CopilotDashboardComponent implements OnInit {
}

activityTotals?: Record<string, number>;
onDestroy$ = new Subject<boolean>();

constructor(
private metricsService: MetricsService,
@@ -103,7 +102,7 @@ export class CopilotDashboardComponent implements OnInit {
const formattedSince = since.toISOString().split('T')[0];

this.installationsService.currentInstallation.pipe(
takeUntil(this.onDestroy$)
takeUntil(this.installationsService.destroy$)
).subscribe(installation => {
this.activityTotals = undefined;
this.allSeats = undefined;
@@ -176,9 +175,4 @@ export class CopilotDashboardComponent implements OnInit {
});
});
}

ngOnDestroy() {
this.onDestroy$.next(true);
this.onDestroy$.unsubscribe();
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Component } from '@angular/core';
import { Component, OnInit } from '@angular/core';
import { DateRangeSelectComponent } from "../../../shared/date-range-select/date-range-select.component";
import { MetricsService } from '../../../services/api/metrics.service';
import { CopilotMetrics } from '../../../services/api/metrics.service.interfaces';
import { CopilotMetricsPieChartComponent } from './copilot-metrics-pie-chart/copilot-metrics-pie-chart.component';
import { MatCardModule } from '@angular/material/card';
import { InstallationsService } from '../../../services/api/installations.service';
import { Installation, InstallationsService } from '../../../services/api/installations.service';
import { takeUntil } from 'rxjs';

@Component({
selector: 'app-metrics',
@@ -20,29 +21,39 @@ import { InstallationsService } from '../../../services/api/installations.servic
'../copilot-dashboard/dashboard.component.scss'
]
})
export class CopilotMetricsComponent {
export class CopilotMetricsComponent implements OnInit {
metrics?: CopilotMetrics[];
metricsTotals?: CopilotMetrics;
installation?: Installation = undefined;

constructor(
private metricsService: MetricsService,
private installationsService: InstallationsService
) { }

ngOnInit() {
this.installationsService.currentInstallation.pipe(
takeUntil(this.installationsService.destroy$)
).subscribe(installation => {
this.installation = installation;
});
}

dateRangeChange(event: {start: Date, end: Date}) {
const utcStart = Date.UTC(event.start.getFullYear(), event.start.getMonth(), event.start.getDate());
const utcEnd = Date.UTC(event.end.getFullYear(), event.end.getMonth(), event.end.getDate());
const startModified = new Date(utcStart - 1);
const endModified = new Date(utcEnd + 1);

this.metricsService.getMetrics({
org: this.installationsService.currentInstallation.value?.account?.login,
org: this.installation?.account?.login,
since: startModified.toISOString(),
until: endModified.toISOString()
}).subscribe((metrics) => {
this.metrics = metrics;
});
this.metricsService.getMetricsTotals({
org: this.installationsService.currentInstallation.value?.account?.login,
org: this.installation?.account?.login,
since: startModified.toISOString(),
until: endModified.toISOString()
}).subscribe((metricsTotals) => {
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@ import { ColumnOptions, TableComponent } from '../../../shared/table/table.compo
import { Seat, SeatService } from '../../../services/api/seat.service';
import { SortDirection } from '@angular/material/sort';
import { Router } from '@angular/router';
import { InstallationsService } from '../../../services/api/installations.service';
import { takeUntil } from 'rxjs';

@Component({
selector: 'app-seats',
@@ -70,12 +72,17 @@ export class CopilotSeatsComponent implements OnInit {

constructor(
private seatsService: SeatService,
private router: Router
private router: Router,
private installationsService: InstallationsService
) {}

ngOnInit() {
this.seatsService.getAllSeats().subscribe(seats => {
this.seats = seats as Seat[];
this.installationsService.currentInstallation.pipe(
takeUntil(this.installationsService.destroy$)
).subscribe(installation => {
this.seatsService.getAllSeats(installation?.account?.login).subscribe(seats => {
this.seats = seats as Seat[];
});
});
}

Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ <h1>Survey #{{ survey.id }}</h1>
<p><strong>Reason: </strong>{{ survey.reason }}</p>
<p><strong>Time Used For: </strong>{{ survey.timeUsedFor }}</p>
</ng-container>
<p><strong>PR: </strong><a [href]="'https://github.com/' + survey.owner + '/' + survey.repo + '/pull/' + survey.prNumber">{{ survey.repo }}#{{ survey.prNumber }}</a></p>
<p><strong>PR: </strong><a [href]="'https://github.com/' + survey.org + '/' + survey.repo + '/pull/' + survey.prNumber">{{ survey.repo }}#{{ survey.prNumber }}</a></p>
</mat-card-content>
</mat-card>
</div>
Original file line number Diff line number Diff line change
@@ -4,6 +4,8 @@ import { AppModule } from '../../../app.module';
import { CopilotSurveyService, Survey } from '../../../services/api/copilot-survey.service';
import { ColumnOptions } from '../../../shared/table/table.component';
import { Router } from '@angular/router';
import { InstallationsService } from '../../../services/api/installations.service';
import { takeUntil } from 'rxjs';

@Component({
selector: 'app-copilot-surveys',
@@ -23,20 +25,25 @@ export class CopilotSurveysComponent implements OnInit {
{ columnDef: 'usedCopilot', header: 'Used Copilot', cell: (element: Survey) => element.status === 'pending' ? 'more_horiz' : element.usedCopilot ? 'svg:github-copilot' : 'close', isIcon: true, iconColor: () => 'var(--sys-on-surface)' },
{ columnDef: 'percentTimeSaved', header: 'Time Saved', cell: (element: Survey) => element.percentTimeSaved < 0 ? '-' : `${element.percentTimeSaved}%` },
{ columnDef: 'timeUsedFor', header: 'Time Used For', cell: (element: Survey) => this.formatTimeUsedFor(element.timeUsedFor) },
{ columnDef: 'prNumber', header: 'PR', cell: (element: Survey) => `${element.repo}#${element.prNumber}`, link: (element: Survey) => `https://github.com/${element.owner}/${element.repo}/pull/${element.prNumber}` },
{ columnDef: 'prNumber', header: 'PR', cell: (element: Survey) => `${element.repo}#${element.prNumber}`, link: (element: Survey) => `https://github.com/${element.org}/${element.repo}/pull/${element.prNumber}` },
{ columnDef: 'createdAt', header: 'Updated', cell: (element: Survey) => new Date(element.updatedAt!).toLocaleString([], { dateStyle: 'short', timeStyle: 'short' }) },
{ columnDef: 'status', header: 'Status', cell: (element: Survey) => `${element.status}`, chipList: true, chipListIcon: (el: Survey) => el.status === 'pending' ? 'pending' : el.status === 'completed' ? 'check' : 'close' },
// { columnDef: 'reason', header: 'Reason', cell: (element: Survey) => element.reason || '-' },
];

constructor(
private copilotSurveyService: CopilotSurveyService,
private router: Router
private router: Router,
private installationsService: InstallationsService
) { }

ngOnInit() {
this.copilotSurveyService.getAllSurveys().subscribe((surveys) => {
this.surveys = surveys;
this.installationsService.currentInstallation.pipe(
takeUntil(this.installationsService.destroy$)
).subscribe(installation => {
this.copilotSurveyService.getAllSurveys(installation?.account?.login).subscribe((surveys) => {
this.surveys = surveys;
});
});
}

Original file line number Diff line number Diff line change
@@ -57,22 +57,24 @@ export class NewCopilotSurveyComponent implements OnInit {
try {
urlObj = new URL(url);
} catch {
return { owner: '', repo: '', prNumber: NaN };
return { org: '', repo: '', prNumber: NaN };
}
const pathSegments = urlObj.pathname.split('/');

const owner = pathSegments[1]; const repo = pathSegments[2]; const prNumber = Number(pathSegments[4]);
return { owner, repo, prNumber };

const org = pathSegments[1];
const repo = pathSegments[2];
const prNumber = Number(pathSegments[4]);
return { org, repo, prNumber };
}

onSubmit() {
const { owner, repo, prNumber } = this.parseGitHubPRUrl(this.params['url']);
const { org, repo, prNumber } = this.parseGitHubPRUrl(this.params['url']);
this.copilotSurveyService.createSurvey({
id: this.id,
userId: this.params['author'],
owner: owner,
repo: repo,
prNumber: prNumber,
org,
repo,
prNumber,
usedCopilot: this.surveyForm.value.usedCopilot,
percentTimeSaved: Number(this.surveyForm.value.percentTimeSaved),
reason: this.surveyForm.value.reason,
10 changes: 2 additions & 8 deletions frontend/src/app/main/copilot/copilot-value/value.component.ts
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ import { TimeSavedChartComponent } from './time-saved-chart/time-saved-chart.com
import { CopilotMetrics } from '../../../services/api/metrics.service.interfaces';
import { MetricsService } from '../../../services/api/metrics.service';
import { FormControl } from '@angular/forms';
import { combineLatest, startWith, Subject, takeUntil } from 'rxjs';
import { combineLatest, startWith, takeUntil } from 'rxjs';
import { CopilotSurveyService, Survey } from '../../../services/api/copilot-survey.service';
import * as Highcharts from 'highcharts';
import HC_exporting from 'highcharts/modules/exporting';
@@ -71,7 +71,6 @@ export class CopilotValueComponent implements OnInit {
}
}
};
onDestroy$ = new Subject<boolean>();

constructor(
private seatService: SeatService,
@@ -82,7 +81,7 @@ export class CopilotValueComponent implements OnInit {

ngOnInit() {
this.installationsService.currentInstallation.pipe(
takeUntil(this.onDestroy$)
takeUntil(this.installationsService.destroy$)
).subscribe(installation => {
combineLatest([
this.daysInactive.valueChanges.pipe(startWith(this.daysInactive.value || 30)),
@@ -103,11 +102,6 @@ export class CopilotValueComponent implements OnInit {
});
}

ngOnDestroy() {
this.onDestroy$.next(true);
this.onDestroy$.unsubscribe();
}

chartChanged(chart: Highcharts.Chart, include = true) {
if (chart && !this.charts.includes(chart)) {
const _chart = chart;
2 changes: 1 addition & 1 deletion frontend/src/app/services/api/copilot-survey.service.ts
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ import { serverUrl } from '../server.service';

export interface Survey {
id?: number;
owner: string;
org: string;
repo: string;
prNumber: number;
status?: 'pending' | 'completed';
21 changes: 16 additions & 5 deletions frontend/src/app/services/api/installations.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Injectable } from '@angular/core';
import { BehaviorSubject, of, tap } from 'rxjs';
import { Injectable, OnDestroy } from '@angular/core';
import { BehaviorSubject, of, Subject, tap } from 'rxjs';
import { serverUrl } from '../server.service';
import { Endpoints } from '@octokit/types';
import { HttpClient } from '@angular/common/http';
@@ -16,22 +16,33 @@ export interface statusResponse {
dbConnected: boolean;
installations: InstallationStatus[],
}

export type Installations = Endpoints["GET /app/installations"]["response"]["data"]
export type Installation = Installations[number]
@Injectable({
providedIn: 'root'
})
export class InstallationsService {
export class InstallationsService implements OnDestroy {
private apiUrl = `${serverUrl}/api/setup`;
status?: statusResponse;
installations = new BehaviorSubject<Endpoints["GET /app/installations"]["response"]["data"]>([]);
currentInstallation = new BehaviorSubject<Endpoints["GET /app/installations"]["response"]["data"][number] | undefined>(undefined);
installations = new BehaviorSubject<Installations>([]);
currentInstallation = new BehaviorSubject<Installation | undefined>(undefined);
currentInstallationId = localStorage.getItem('installation') ? parseInt(localStorage.getItem('installation')!) : 0;
private readonly _destroy$ = new Subject<void>();
readonly destroy$ = this._destroy$.asObservable();

constructor(private http: HttpClient) {
const id = localStorage.getItem('installation');
if (id) {
this.setInstallation(Number(id));
}
}

public ngOnDestroy(): void {
this._destroy$.next();
this._destroy$.complete();
}

getStatus() {
if (!this.status) {
return this.refreshStatus();