Track review counts by staff/volunteers/both

Author: illicitonion

config.prod.json

@@ -871,5 +871,6 @@
         }
       }
     }
-  }
+  },
+  "staff_github_team_name": "$CYF_STAFF_GITHUB_TEAM_NAME"
 }

src/config.rs

@@ -1,11 +1,19 @@
-use std::{collections::BTreeMap, net::IpAddr};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    net::IpAddr,
+};
 
 use chrono::NaiveDate;
 use indexmap::IndexMap;
+use octocrab::Octocrab;
 use serde::Deserialize;
 use serde_env_field::EnvField;
 
-use crate::newtypes::Region;
+use crate::{
+    Error,
+    newtypes::{GithubLogin, Region},
+    octocrab::all_pages,
+};
 
 #[derive(Clone, Deserialize)]
 pub struct Config {
@@ -39,6 +47,8 @@ pub struct Config {
     pub mentoring_records_sheet_id: String,
 
     pub reviewer_staff_info_sheet_id: String,
+
+    pub staff_github_team_name: EnvField<String>,
 }
 
 #[derive(Clone, Deserialize)]
@@ -77,6 +87,24 @@ impl Config {
             None
         }
     }
+
+    pub async fn get_staff_github_usernames(
+        &self,
+        octocrab: &Octocrab,
+    ) -> Result<BTreeSet<GithubLogin>, Error> {
+        let members = all_pages("members", &octocrab, async || {
+            octocrab
+                .teams(&self.github_org)
+                .members(self.staff_github_team_name.as_str())
+                .send()
+                .await
+        })
+        .await?;
+        Ok(members
+            .into_iter()
+            .map(|member| GithubLogin::from(member.login))
+            .collect())
+    }
 }
 
 #[derive(Clone, Deserialize)]

src/frontend.rs

@@ -275,6 +275,12 @@ pub async fn get_review_metrics(
 
     let octocrab = octocrab(&session, &server_state, original_uri).await?;
 
+    let staff_usernames = server_state
+        .config
+        .get_staff_github_usernames(&octocrab)
+        .await
+        .map_err(|err| err.context("Unable to get staff usernames"))?;
+
     let module_futures = module_names
         .into_iter()
         .map(async |module_name| {
@@ -288,8 +294,13 @@ pub async fn get_review_metrics(
             let metrics_futures: Vec<_> = prs
                 .into_iter()
                 .map(async |pr| {
-                    crate::prs::get_review_metrics(&octocrab, &server_state.config.github_org, pr)
-                        .await
+                    crate::prs::get_review_metrics(
+                        &octocrab,
+                        &server_state.config.github_org,
+                        pr,
+                        &staff_usernames,
+                    )
+                    .await
                 })
                 .collect();
             let metrics = join_all(metrics_futures).await;

src/prs.rs

@@ -245,6 +245,8 @@ pub struct AggregatePrMetrics {
     pub p100_needs_review_to_complete: Option<TimeDelta>,
 
     pub iteration_counts: BTreeMap<usize, usize>,
+
+    pub reviewed_by_counts: BTreeMap<ReviewedBy, usize>,
 }
 
 impl AggregatePrMetrics {
@@ -271,10 +273,12 @@ impl AggregatePrMetrics {
         });
 
         let mut iteration_counts = BTreeMap::new();
+        let mut reviewed_by_counts = BTreeMap::new();
         for metric in metrics {
             if metric.first_complete.is_some() {
                 *iteration_counts.entry(metric.iterations).or_default() += 1;
             }
+            *reviewed_by_counts.entry(metric.reviewed_by).or_default() += 1;
         }
 
         AggregatePrMetrics {
@@ -288,6 +292,7 @@ impl AggregatePrMetrics {
             p90_needs_review_to_complete,
             p100_needs_review_to_complete,
             iteration_counts,
+            reviewed_by_counts,
         }
     }
 
@@ -322,6 +327,7 @@ pub struct PrMetrics {
     pub first_reviewed: Option<chrono::DateTime<chrono::Utc>>,
     pub first_complete: Option<chrono::DateTime<chrono::Utc>>,
     pub iterations: usize,
+    pub reviewed_by: ReviewedBy,
 }
 
 impl PrMetrics {
@@ -343,6 +349,7 @@ impl PrMetrics {
                     first_needs_review = Some(event.time);
                 }
             } else if event.actor != pr.author {
+                reviewed_by.observe_also(&event.actor, staff);
                 if event.label == "Reviewed" {
                     iterations += 1;
                     if first_reviewed.is_none() {
@@ -365,6 +372,7 @@ impl PrMetrics {
             first_reviewed,
             first_complete,
             iterations,
+            reviewed_by,
         }
     }
 
@@ -385,6 +393,39 @@ impl PrMetrics {
     }
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, PartialOrd, Ord, strum_macros::Display)]
+pub enum ReviewedBy {
+    NoOne,
+    OnlyStaff,
+    OnlyVolunteer,
+    StaffAndVolunteer,
+}
+
+impl ReviewedBy {
+    fn observe_also(&mut self, user: &GithubLogin, staff: &BTreeSet<GithubLogin>) {
+        match self {
+            ReviewedBy::NoOne => {
+                if staff.contains(user) {
+                    *self = ReviewedBy::OnlyStaff;
+                } else {
+                    *self = ReviewedBy::OnlyVolunteer;
+                }
+            }
+            ReviewedBy::OnlyStaff => {
+                if !staff.contains(user) {
+                    *self = ReviewedBy::StaffAndVolunteer;
+                }
+            }
+            ReviewedBy::OnlyVolunteer => {
+                if staff.contains(user) {
+                    *self = ReviewedBy::StaffAndVolunteer;
+                }
+            }
+            ReviewedBy::StaffAndVolunteer => {}
+        }
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Serialize)]
 pub struct LabelAddEvent {
     pub actor: GithubLogin,
@@ -521,6 +562,7 @@ pub(crate) async fn get_review_metrics(
     octocrab: &Octocrab,
     github_org: &str,
     pr: Pr,
+    staff: &BTreeSet<GithubLogin>,
 ) -> Result<PrMetrics, Error> {
     let events = all_pages("timeline events", octocrab, async || {
         octocrab
@@ -561,7 +603,7 @@ pub(crate) async fn get_review_metrics(
         )
         .collect();
     let created_at = pr.created_at;
-    Ok(PrMetrics::new(pr, created_at, label_add_events))
+    Ok(PrMetrics::new(pr, created_at, label_add_events, staff))
 }
 
 // Ideally this would be a more general shared function, but async closures aren't super stable yet.

templates/review-metrics.html

@@ -10,7 +10,7 @@
         }
         .stats-container {
             display: grid;
-            grid-template-columns: repeat(4, 1fr);
+            grid-template-columns: repeat(5, 1fr);
             gap: 40px;
         }
         .reviews-container {
@@ -65,6 +65,14 @@ <h3>Iterations</h3>
                     {% endfor %}
                 </ul>
             </div>
+            <div class="stats-card">
+                <h3>Reviews by</h3>
+                <ul>
+                    {% for (reviewed_by, count) in aggregate_metrics.reviewed_by_counts %}
+                        <li>{{reviewed_by}}: {{count}} PRs</li>
+                    {% endfor %}
+                </ul>
+            </div>
         </div>
 
         {% for module in modules %}
@@ -105,6 +113,15 @@ <h3>Iterations</h3>
                                 {% endfor %}
                             </ul>
                         </div>
+                        <div class="stats-card">
+                            <h3>Reviews by</h3>
+                            <ul>
+                                {% for (reviewed_by, count) in module.aggregate_metrics.reviewed_by_counts %}
+                                    <li>{{reviewed_by}}: {{count}} PRs</li>
+                                {% endfor %}
+                            </ul>
+                        </div>
+
                     </div>
                     <div class="reviews-container">
                         {% for pr in module.metrics %}
@@ -114,6 +131,7 @@ <h3>Iterations</h3>
                                 <div>Needs Review to Complete: {{format_duration(&pr.needs_review_to_complete())}}</div>
                                 <div>{{pr.iterations}} iterations</div>
                                 <div>Time since created: {{format_duration(&Some(pr.time_since_created()))}}</div>
+                                <div>Reviewed by: {{pr.reviewed_by}}</div>
                             </div>
                         {% endfor %}
                     </div>