Skip to content

Commit 5197abe

Browse files
committed
Track review counts by staff/volunteers/both
1 parent cc204a5 commit 5197abe

5 files changed

Lines changed: 108 additions & 8 deletions

File tree

config.prod.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,5 +751,6 @@
751751
}
752752
}
753753
}
754-
}
754+
},
755+
"staff_github_team_name": "$CYF_STAFF_GITHUB_TEAM_NAME"
755756
}

src/config.rs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
1-
use std::{collections::BTreeMap, net::IpAddr};
1+
use std::{
2+
collections::{BTreeMap, BTreeSet},
3+
net::IpAddr,
4+
};
25

36
use chrono::NaiveDate;
47
use indexmap::IndexMap;
8+
use octocrab::Octocrab;
59
use serde::Deserialize;
610
use serde_env_field::EnvField;
711

8-
use crate::newtypes::Region;
12+
use crate::{
13+
Error,
14+
newtypes::{GithubLogin, Region},
15+
octocrab::all_pages,
16+
};
917

1018
#[derive(Clone, Deserialize)]
1119
pub struct Config {
@@ -39,6 +47,8 @@ pub struct Config {
3947
pub mentoring_records_sheet_id: String,
4048

4149
pub reviewer_staff_info_sheet_id: String,
50+
51+
pub staff_github_team_name: EnvField<String>,
4252
}
4353

4454
#[derive(Clone, Deserialize)]
@@ -77,6 +87,24 @@ impl Config {
7787
None
7888
}
7989
}
90+
91+
pub async fn get_staff_github_usernames(
92+
&self,
93+
octocrab: &Octocrab,
94+
) -> Result<BTreeSet<GithubLogin>, Error> {
95+
let members = all_pages("members", &octocrab, async || {
96+
octocrab
97+
.teams(&self.github_org)
98+
.members(self.staff_github_team_name.as_str())
99+
.send()
100+
.await
101+
})
102+
.await?;
103+
Ok(members
104+
.into_iter()
105+
.map(|member| GithubLogin::from(member.login))
106+
.collect())
107+
}
80108
}
81109

82110
#[derive(Clone, Deserialize)]

src/frontend.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use axum::{
66
extract::{OriginalUri, Path, Query, State},
77
response::{Html, IntoResponse, Response},
88
};
9-
use chrono::TimeDelta;
9+
use chrono::{Datelike, TimeDelta};
1010
use futures::future::join_all;
1111
use http::{HeaderMap, StatusCode, Uri, header::CONTENT_TYPE};
1212
use serde::Deserialize;
@@ -275,6 +275,12 @@ pub async fn get_review_metrics(
275275

276276
let octocrab = octocrab(&session, &server_state, original_uri).await?;
277277

278+
let staff_usernames = server_state
279+
.config
280+
.get_staff_github_usernames(&octocrab)
281+
.await
282+
.map_err(|err| err.context("Unable to get staff usernames"))?;
283+
278284
let module_futures = module_names
279285
.into_iter()
280286
.map(async |module_name| {
@@ -288,8 +294,13 @@ pub async fn get_review_metrics(
288294
let metrics_futures: Vec<_> = prs
289295
.into_iter()
290296
.map(async |pr| {
291-
crate::prs::get_review_metrics(&octocrab, &server_state.config.github_org, pr)
292-
.await
297+
crate::prs::get_review_metrics(
298+
&octocrab,
299+
&server_state.config.github_org,
300+
pr,
301+
&staff_usernames,
302+
)
303+
.await
293304
})
294305
.collect();
295306
let metrics = join_all(metrics_futures).await;

src/prs.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,8 @@ pub struct AggregatePrMetrics {
245245
pub p100_needs_review_to_complete: Option<TimeDelta>,
246246

247247
pub iteration_counts: BTreeMap<usize, usize>,
248+
249+
pub reviewed_by_counts: BTreeMap<ReviewedBy, usize>,
248250
}
249251

250252
impl AggregatePrMetrics {
@@ -271,10 +273,12 @@ impl AggregatePrMetrics {
271273
});
272274

273275
let mut iteration_counts = BTreeMap::new();
276+
let mut reviewed_by_counts = BTreeMap::new();
274277
for metric in metrics {
275278
if metric.first_complete.is_some() {
276279
*iteration_counts.entry(metric.iterations).or_default() += 1;
277280
}
281+
*reviewed_by_counts.entry(metric.reviewed_by).or_default() += 1;
278282
}
279283

280284
AggregatePrMetrics {
@@ -288,6 +292,7 @@ impl AggregatePrMetrics {
288292
p90_needs_review_to_complete,
289293
p100_needs_review_to_complete,
290294
iteration_counts,
295+
reviewed_by_counts,
291296
}
292297
}
293298

@@ -322,6 +327,7 @@ pub struct PrMetrics {
322327
pub first_reviewed: Option<chrono::DateTime<chrono::Utc>>,
323328
pub first_complete: Option<chrono::DateTime<chrono::Utc>>,
324329
pub iterations: usize,
330+
pub reviewed_by: ReviewedBy,
325331
}
326332

327333
impl PrMetrics {
@@ -343,6 +349,7 @@ impl PrMetrics {
343349
first_needs_review = Some(event.time);
344350
}
345351
} else if event.actor != pr.author {
352+
reviewed_by.observe_also(&event.actor, staff);
346353
if event.label == "Reviewed" {
347354
iterations += 1;
348355
if first_reviewed.is_none() {
@@ -365,6 +372,7 @@ impl PrMetrics {
365372
first_reviewed,
366373
first_complete,
367374
iterations,
375+
reviewed_by,
368376
}
369377
}
370378

@@ -385,6 +393,39 @@ impl PrMetrics {
385393
}
386394
}
387395

396+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, PartialOrd, Ord, strum_macros::Display)]
397+
pub enum ReviewedBy {
398+
NoOne,
399+
OnlyStaff,
400+
OnlyVolunteer,
401+
StaffAndVolunteer,
402+
}
403+
404+
impl ReviewedBy {
405+
fn observe_also(&mut self, user: &GithubLogin, staff: &BTreeSet<GithubLogin>) {
406+
match self {
407+
ReviewedBy::NoOne => {
408+
if staff.contains(user) {
409+
*self = ReviewedBy::OnlyStaff;
410+
} else {
411+
*self = ReviewedBy::OnlyVolunteer;
412+
}
413+
}
414+
ReviewedBy::OnlyStaff => {
415+
if !staff.contains(user) {
416+
*self = ReviewedBy::StaffAndVolunteer;
417+
}
418+
}
419+
ReviewedBy::OnlyVolunteer => {
420+
if staff.contains(user) {
421+
*self = ReviewedBy::StaffAndVolunteer;
422+
}
423+
}
424+
ReviewedBy::StaffAndVolunteer => {}
425+
}
426+
}
427+
}
428+
388429
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
389430
pub struct LabelAddEvent {
390431
pub actor: GithubLogin,
@@ -521,6 +562,7 @@ pub(crate) async fn get_review_metrics(
521562
octocrab: &Octocrab,
522563
github_org: &str,
523564
pr: Pr,
565+
staff: &BTreeSet<GithubLogin>,
524566
) -> Result<PrMetrics, Error> {
525567
let events = all_pages("timeline events", octocrab, async || {
526568
octocrab
@@ -561,7 +603,7 @@ pub(crate) async fn get_review_metrics(
561603
)
562604
.collect();
563605
let created_at = pr.created_at;
564-
Ok(PrMetrics::new(pr, created_at, label_add_events))
606+
Ok(PrMetrics::new(pr, created_at, label_add_events, staff))
565607
}
566608

567609
// Ideally this would be a more general shared function, but async closures aren't super stable yet.

templates/review-metrics.html

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
}
1111
.stats-container {
1212
display: grid;
13-
grid-template-columns: repeat(4, 1fr);
13+
grid-template-columns: repeat(5, 1fr);
1414
gap: 40px;
1515
}
1616
.reviews-container {
@@ -65,6 +65,14 @@ <h3>Iterations</h3>
6565
{% endfor %}
6666
</ul>
6767
</div>
68+
<div class="stats-card">
69+
<h3>Reviews by</h3>
70+
<ul>
71+
{% for (reviewed_by, count) in aggregate_metrics.reviewed_by_counts %}
72+
<li>{{reviewed_by}}: {{count}} PRs</li>
73+
{% endfor %}
74+
</ul>
75+
</div>
6876
</div>
6977

7078
{% for module in modules %}
@@ -105,6 +113,15 @@ <h3>Iterations</h3>
105113
{% endfor %}
106114
</ul>
107115
</div>
116+
<div class="stats-card">
117+
<h3>Reviews by</h3>
118+
<ul>
119+
{% for (reviewed_by, count) in module.aggregate_metrics.reviewed_by_counts %}
120+
<li>{{reviewed_by}}: {{count}} PRs</li>
121+
{% endfor %}
122+
</ul>
123+
</div>
124+
108125
</div>
109126
<div class="reviews-container">
110127
{% for pr in module.metrics %}
@@ -114,6 +131,7 @@ <h3>Iterations</h3>
114131
<div>Needs Review to Complete: {{format_duration(&pr.needs_review_to_complete())}}</div>
115132
<div>{{pr.iterations}} iterations</div>
116133
<div>Time since created: {{format_duration(&Some(pr.time_since_created()))}}</div>
134+
<div>Reviewed by: {{pr.reviewed_by}}</div>
117135
</div>
118136
{% endfor %}
119137
</div>

0 commit comments

Comments
 (0)