Skip to content

Commit 729c27d

Browse files
committed
fix: first attempt at making review plugin time bound. Needs to be tested
Signed-off-by: Nina 'Nino' Agrawal <ninaagrawal@mitre.org>
1 parent 1428d72 commit 729c27d

File tree

8 files changed

+71
-11
lines changed

8 files changed

+71
-11
lines changed

plugins/github/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ graphql_client = "0.14.0"
1313
hipcheck-sdk = { version = "0.5.0", path = "../../sdk/rust", features = [
1414
"macros",
1515
] }
16+
chrono = "0.4.40"
1617
tracing = "0.1"
1718
# Exactly matching the version of rustls used by ureq
1819
# Get rid of default features since we don't use the AWS backed crypto

plugins/github/src/gh_query.graphql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ query Reviews($owner:String!, $repo:String!, $cursor:String) {
1212
nodes {
1313
databaseId
1414
}
15-
}
15+
},
16+
mergedAt
1617
}
1718
}
1819
}

plugins/github/src/gh_schema.graphql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ type PullRequest {
9494
number: Int!
9595

9696
url: URI!
97+
98+
"""
99+
Identifies the date the pull request was merged
100+
"""
101+
mergedAt: String!
102+
97103
"""
98104
A list of reviews associated with the pull request.
99105
"""

plugins/github/src/graphql.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// SPDX-License-Identifier: Apache-2.0
22

3+
use chrono::prelude::*;
34
use std::convert::TryInto;
45

56
use self::reviews::{ResponseData, ReviewsRepositoryPullRequestsNodes as RawPull, Variables};
@@ -152,6 +153,14 @@ fn process_pr(pr: RawPull) -> GitHubPullRequest {
152153
}
153154
.try_into()
154155
.unwrap();
155-
156-
GitHubPullRequest { number, reviews }
156+
let date_merged: DateTime<Utc> = pr
157+
.merged_at
158+
.parse::<DateTime<Utc>>()
159+
.expect("Error, could not get timestamp");
160+
161+
GitHubPullRequest {
162+
number,
163+
reviews,
164+
date_merged,
165+
}
157166
}

plugins/github/src/main.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ mod data;
55
mod graphql;
66
mod types;
77
mod util;
8-
98
use crate::data::GitHub;
9+
use chrono::prelude::*;
1010
use clap::Parser;
1111
use hipcheck_sdk::{
1212
prelude::*,
@@ -54,6 +54,7 @@ static CONFIG: OnceLock<Config> = OnceLock::new();
5454
pub struct PullRequest {
5555
pub id: u64,
5656
pub reviews: u64,
57+
pub date_merged: DateTime<Utc>,
5758
}
5859

5960
fn get_github_agent<'a>(owner: &'a str, repo: &'a str) -> Result<GitHub<'a>> {
@@ -91,6 +92,7 @@ async fn pr_reviews(_engine: &mut PluginEngine, key: KnownRemote) -> Result<Vec<
9192
.map(|pr| PullRequest {
9293
id: pr.number,
9394
reviews: pr.reviews,
95+
date_merged: pr.date_merged,
9496
})
9597
.collect();
9698

plugins/github/src/types.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
// SPDX-License-Identifier: Apache-2.0
22

3+
use chrono::{DateTime, Utc};
34
use serde::Deserialize;
45

56
#[derive(Debug, Deserialize)]
67
pub struct GitHubPullRequest {
78
pub number: u64,
89
pub reviews: u64,
10+
pub date_merged: DateTime<Utc>,
911
}

plugins/review/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ clap = { version = "4.5.32", features = ["derive"] }
1212
hipcheck-sdk = { version = "0.5.0", path = "../../sdk/rust", features = [
1313
"macros",
1414
] }
15+
chrono = "0.4.40"
1516
tracing = "0.1"
1617
schemars = { version = "0.8.22", features = ["url"] }
1718
serde = { version = "1.0.219", features = ["derive", "rc"] }

plugins/review/src/main.rs

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//! Plugin for querying what percentage of pull requests were merged without review
44
55
use anyhow::Context as _;
6+
use chrono::prelude::*;
67
use clap::Parser;
78
use hipcheck_sdk::{prelude::*, types::Target, LogLevel};
89
use schemars::JsonSchema;
@@ -21,6 +22,7 @@ static CONFIG: OnceLock<Config> = OnceLock::new();
2122
pub struct PullRequest {
2223
pub id: u64,
2324
pub reviews: u64,
25+
pub date_merged: DateTime<Utc>,
2426
}
2527

2628
#[derive(Debug, Clone, Eq, PartialEq)]
@@ -44,6 +46,17 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
4446
tracing::error!("target repository is not a GitHub repository or else is missing GitHub repo information");
4547
return Err(Error::UnexpectedPluginQueryInputFormat);
4648
};
49+
let latest_commit_date = engine
50+
.query("mitre/git/last_commit_date", value.local)
51+
.await
52+
.context("failed to get the last commit date from repo")?;
53+
54+
let latest_commit_date_as_string: String =
55+
serde_json::from_value(latest_commit_date).map_err(Error::InvalidJsonInQueryOutput)?;
56+
57+
let latest_commit_date_as_timestamp = latest_commit_date_as_string
58+
.parse::<DateTime<Utc>>()
59+
.unwrap();
4760

4861
// Get a list of all pull requests to the repo, with their corresponding number of reviews
4962
let value = engine
@@ -59,7 +72,11 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
5972
// Create a Vec big enough to hold every single pull request
6073
let mut pull_reviews = Vec::with_capacity(pull_requests.len());
6174

62-
pull_reviews.extend(pull_requests.into_iter().map(|pr| pr.reviews > 0));
75+
pull_reviews.extend(
76+
pull_requests
77+
.into_iter()
78+
.map(|pr| pr.reviews > 0 && pr.date_merged <= latest_commit_date_as_timestamp),
79+
);
6380

6481
tracing::info!("completed review query");
6582
Ok(pull_reviews)
@@ -129,7 +146,6 @@ async fn main() -> Result<()> {
129146
#[cfg(test)]
130147
mod test {
131148
use super::*;
132-
133149
use hipcheck_sdk::types::{KnownRemote, LocalGitRepo, RemoteGitRepo};
134150
use std::result::Result as StdResult;
135151
use url::Url;
@@ -144,15 +160,37 @@ mod test {
144160
fn mock_responses() -> StdResult<MockResponses, Error> {
145161
let known_remote = known_remote();
146162

147-
let pr1 = PullRequest { id: 1, reviews: 1 };
148-
let pr2 = PullRequest { id: 2, reviews: 3 };
149-
let pr3 = PullRequest { id: 3, reviews: 0 };
150-
let pr4 = PullRequest { id: 4, reviews: 1 };
163+
let pr1 = PullRequest {
164+
id: 1,
165+
reviews: 1,
166+
date_merged: Utc::now(),
167+
};
168+
let pr2 = PullRequest {
169+
id: 2,
170+
reviews: 3,
171+
date_merged: Utc::now(),
172+
};
173+
let pr3 = PullRequest {
174+
id: 3,
175+
reviews: 0,
176+
date_merged: Utc::now(),
177+
};
178+
let pr4 = PullRequest {
179+
id: 4,
180+
reviews: 1,
181+
date_merged: Utc::now(),
182+
};
151183
let prs = vec![pr1, pr2, pr3, pr4];
152184

153185
// when calling into query, the input known_remote gets passed to `pr_reviews`, lets assume it returns the vec of PullRequests `prs`
154186
let mut mock_responses = MockResponses::new();
155-
mock_responses.insert("mitre/github/pr_reviews", known_remote, Ok(prs))?;
187+
mock_responses.insert("mitre/github/pr_reviews", &known_remote, Ok(prs))?;
188+
// TODO: figure out how to call multiple mock responses since review calls both github/pr_reviews and git/last_commit_date
189+
mock_responses.insert(
190+
"mitre/git/last_commit_date",
191+
&known_remote,
192+
Ok("2023-10-01T15:30:45Z"),
193+
)?;
156194
Ok(mock_responses)
157195
}
158196

0 commit comments

Comments
 (0)