Skip to content

Add review_submitted handler which changes labels after a review #1379

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

Merged
merged 3 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ impl<'a> Action for Step<'a> {
let mut context = Context::new();
let mut results = HashMap::new();

for Query { repos, queries} in &self.actions {

for Query { repos, queries } in &self.actions {
for repo in repos {
let repository = Repository {
full_name: repo.to_string(),
Expand Down Expand Up @@ -118,7 +117,6 @@ impl<'a> Action for Step<'a> {

match count {
Ok(count) => {

let result = if let Some(value) = context.get(*name) {
value.as_u64().unwrap() + count as u64
} else {
Expand Down
8 changes: 8 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub(crate) struct Config {
pub(crate) autolabel: Option<AutolabelConfig>,
pub(crate) notify_zulip: Option<NotifyZulipConfig>,
pub(crate) github_releases: Option<GitHubReleasesConfig>,
pub(crate) review_submitted: Option<ReviewSubmittedConfig>,
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
Expand Down Expand Up @@ -142,6 +143,12 @@ pub(crate) struct GlacierConfig {}
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct CloseConfig {}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct ReviewSubmittedConfig {
pub(crate) review_labels: Vec<String>,
pub(crate) reviewed_label: String,
}

pub(crate) async fn get(gh: &GithubClient, repo: &str) -> Result<Arc<Config>, ConfigurationError> {
if let Some(config) = get_cached_config(repo) {
log::trace!("returning config for {} from cache", repo);
Expand Down Expand Up @@ -290,6 +297,7 @@ mod tests {
autolabel: None,
notify_zulip: None,
github_releases: None,
review_submitted: None,
}
);
}
Expand Down
12 changes: 12 additions & 0 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ pub struct Comment {
pub user: User,
#[serde(alias = "submitted_at")] // for pull request reviews
pub updated_at: chrono::DateTime<Utc>,
#[serde(rename = "state")]
pub pr_review_state: PullRequestReviewState,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this may have broken deserialization of issue comments -- I'm filing a fix now.

}

#[derive(Debug, serde::Deserialize, Eq, PartialEq)]
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
pub enum PullRequestReviewState {
Approved,
ChangesRequested,
Commented,
Dismissed,
Pending,
}

fn opt_string<'de, D>(deserializer: D) -> Result<String, D::Error>
Expand Down
19 changes: 17 additions & 2 deletions src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ mod notify_zulip;
mod ping;
mod prioritize;
mod relabel;
mod review_submitted;
mod rustc_commits;

pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
Expand Down Expand Up @@ -74,6 +75,20 @@ pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
);
}

if let Some(config) = config
.as_ref()
.ok()
.and_then(|c| c.review_submitted.as_ref())
{
if let Err(e) = review_submitted::handle(ctx, event, config).await {
log::error!(
"failed to process event {:?} with review_submitted handler: {:?}",
event,
e
)
}
}

if let Some(ghr_config) = config
.as_ref()
.ok()
Expand Down Expand Up @@ -120,9 +135,9 @@ macro_rules! issue_handlers {
}
}

// Handle events that happend on issues
// Handle events that happened on issues
//
// This is for events that happends only on issues (e.g. label changes).
// This is for events that happen only on issues (e.g. label changes).
// Each module in the list must contain the functions `parse_input` and `handle_input`.
issue_handlers! {
autolabel,
Expand Down
46 changes: 46 additions & 0 deletions src/handlers/review_submitted.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use crate::github::{Issue, IssueCommentAction, IssueCommentEvent, Label, PullRequestReviewState};
use crate::{config::ReviewSubmittedConfig, github::Event, handlers::Context};

pub(crate) async fn handle(
ctx: &Context,
event: &Event,
config: &ReviewSubmittedConfig,
) -> anyhow::Result<()> {
if let Event::IssueComment(
event
@
IssueCommentEvent {
action: IssueCommentAction::Created,
issue: Issue {
pull_request: Some(_),
..
},
..
},
) = event
{
if event.comment.pr_review_state != PullRequestReviewState::ChangesRequested {
return Ok(());
}

if event.issue.assignees.contains(&event.comment.user) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only do this for "Request Changes". I'm not sure how to interpret a simple "comment" review that has no "inclination".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, changed it

let labels = event
.issue
.labels()
.iter()
// Remove review related labels
.filter(|label| !config.review_labels.contains(&label.name))
.cloned()
// Add waiting on author label
.chain(std::iter::once(Label {
name: config.reviewed_label.clone(),
}));
event
.issue
.set_labels(&ctx.github, labels.collect())
.await?;
}
}

Ok(())
}