diff --git a/Cargo.lock b/Cargo.lock index 1036afd4..e1555b3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -310,6 +310,16 @@ dependencies = [ "typenum", ] +[[package]] +name = "ctor" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdffe87e1d521a10f9696f833fe502293ea446d7f256c06128293a4119bdf4cb" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "cynic" version = "0.14.1" @@ -384,6 +394,12 @@ dependencies = [ "syn", ] +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "digest" version = "0.8.1" @@ -457,6 +473,26 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5320ae4c3782150d900b79807611a59a99fc9a1d61d686faafc24b93fc8d7ca" +[[package]] +name = "factori" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ff6b50917609e530c145de1c6aa8df9c38c40562375e8aa5eeaaf6c737a0b31" +dependencies = [ + "factori-impl", +] + +[[package]] +name = "factori-impl" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d6344ded92b0a4a1d90a816632f7ff2a12e01401d325d6295810dacca1dbdd6" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "fake-simd" version = "0.1.2" @@ -1182,6 +1218,15 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "output_vt100" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "628223faebab4e3e40667ee0b2336d34a5b960ff60ea743ddfdbcf7770bcfb66" +dependencies = [ + "winapi", +] + [[package]] name = "parking_lot" version = "0.11.2" @@ -1212,8 +1257,10 @@ name = "parser" version = "0.1.0" dependencies = [ "log", + "postgres-types", "pulldown-cmark", "regex", + "serde", ] [[package]] @@ -1323,9 +1370,9 @@ checksum = "1df8c4ec4b0627e53bdf214615ad287367e482558cf84b109250b37464dc03ae" [[package]] name = "postgres-derive" -version = "0.4.2" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0c2c18e40b92144b05e6f3ae9d1ee931f0d1afa9410ac8b97486c6eaaf91201" +checksum = "9e76c801e97c9cf696097369e517785b98056e98b21149384c812febfc5912f2" dependencies = [ "proc-macro2", "quote", @@ -1385,13 +1432,25 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eb9f9e6e233e5c4a35559a617bf40a4ec447db2e84c20b55a6f83167b7e57872" +[[package]] +name = "pretty_assertions" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c89f989ac94207d048d92db058e4f6ec7342b0971fc58d1271ca148b799b3563" +dependencies = [ + "ansi_term", + "ctor", + "diff", + "output_vt100", +] + [[package]] name = "proc-macro2" -version = "1.0.37" +version = "1.0.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec757218438d5fda206afc041538b2f6d889286160d649a86a24d37e1235afd1" +checksum = "0a2ca2c61bc9f3d74d2886294ab7b9853abd9c1ad903a3ac7815c58989bb7bab" dependencies = [ - "unicode-xid", + "unicode-ident", ] [[package]] @@ -1408,9 +1467,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.18" +version = "1.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1feb54ed693b93a84e14094943b84b7c4eae204c512b7ccb95ab0c66d278ad1" +checksum = "bbe448f377a7d6961e30f5955f9b8d106c3f5e449d493ee1b125c1d43c2b5179" dependencies = [ "proc-macro2", ] @@ -1781,13 +1840,13 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" [[package]] name = "syn" -version = "1.0.91" +version = "1.0.99" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b683b2b825c8eef438b77c36a06dc262294da3d5a5813fac20da149241dcd44d" +checksum = "58dbef6ec655055e20b86b15a8cc6d439cca19b667537ac6a1369572d151ab13" dependencies = [ "proc-macro2", "quote", - "unicode-xid", + "unicode-ident", ] [[package]] @@ -2085,6 +2144,7 @@ dependencies = [ "cron", "cynic", "dotenv", + "factori", "futures", "github-graphql", "glob", @@ -2100,6 +2160,7 @@ dependencies = [ "parser", "postgres-native-tls", "postgres-types", + "pretty_assertions", "rand", "regex", "reqwest", @@ -2224,6 +2285,12 @@ version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1a01404663e3db436ed2746d9fefef640d868edae3cceb81c3b8d5732fda678f" +[[package]] +name = "unicode-ident" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4f5b37a154999a8f3f98cc23a628d850e154479cd94decf3414696e12e31aaf" + [[package]] name = "unicode-normalization" version = "0.1.19" @@ -2239,12 +2306,6 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3ed742d4ea2bd1176e236172c8429aaf54486e7ac098db29ffe6529e0ce50973" -[[package]] -name = "unicode-xid" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" - [[package]] name = "unicode_categories" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index 204fb06a..510f716d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,8 @@ rand = "0.8.5" ignore = "0.4.18" postgres-types = { version = "0.2.4", features = ["derive"] } cron = { version = "0.12.0" } +pretty_assertions = "1.2" +factori = "1.1.0" [dependencies.serde] version = "1" diff --git a/parser/Cargo.toml b/parser/Cargo.toml index 22bf0aaf..d73d4aa9 100644 --- a/parser/Cargo.toml +++ b/parser/Cargo.toml @@ -8,3 +8,8 @@ edition = "2021" pulldown-cmark = "0.7.0" log = "0.4" regex = "1.6.0" +postgres-types = { version = "0.2.4", features = ["derive"] } + +[dependencies.serde] +version = "1" +features = ["derive"] diff --git a/parser/src/command.rs b/parser/src/command.rs index 7e40949d..671ea41f 100644 --- a/parser/src/command.rs +++ b/parser/src/command.rs @@ -5,6 +5,7 @@ use regex::Regex; pub mod assign; pub mod close; +pub mod decision; pub mod glacier; pub mod nominate; pub mod note; @@ -26,6 +27,7 @@ pub enum Command<'a> { Shortcut(Result>), Close(Result>), Note(Result>), + Decision(Result>), } #[derive(Debug)] @@ -132,6 +134,11 @@ impl<'a> Input<'a> { Command::Close, &original_tokenizer, )); + success.extend(parse_single_command( + decision::DecisionCommand::parse, + Command::Decision, + &original_tokenizer, + )); if success.len() > 1 { panic!( @@ -207,6 +214,7 @@ impl<'a> Command<'a> { Command::Shortcut(r) => r.is_ok(), Command::Close(r) => r.is_ok(), Command::Note(r) => r.is_ok(), + Command::Decision(r) => r.is_ok(), } } diff --git a/parser/src/command/decision.rs b/parser/src/command/decision.rs new file mode 100644 index 00000000..289c78e4 --- /dev/null +++ b/parser/src/command/decision.rs @@ -0,0 +1,214 @@ +//! The decision process command parser. +//! +//! This can parse arbitrary input, giving the user to be assigned. +//! +//! The grammar is as follows: +//! +//! ```text +//! Command: `@bot merge`, `@bot hold`, `@bot restart`, `@bot dissent`, `@bot stabilize` or `@bot close`. +//! ``` + +use std::fmt; + +use crate::error::Error; +use crate::token::{Token, Tokenizer}; +use postgres_types::{FromSql, ToSql}; +use serde::{Deserialize, Serialize}; + +/// A command as parsed and received from calling the bot with some arguments, +/// like `@rustbot merge` +#[derive(Debug, Eq, PartialEq)] +pub struct DecisionCommand { + pub resolution: Resolution, + pub reversibility: Reversibility, + pub team: Option, +} + +impl DecisionCommand { + pub fn parse<'a>(input: &mut Tokenizer<'a>) -> Result, Error<'a>> { + let mut toks = input.clone(); + + match toks.peek_token()? { + Some(Token::Word("merge")) => { + toks.next_token()?; + + let team: Option = get_team(&mut toks)?; + + command_or_error( + input, + &mut toks, + Self { + resolution: Resolution::Merge, + reversibility: Reversibility::Reversible, + team, + }, + ) + } + Some(Token::Word("hold")) => { + toks.next_token()?; + + let team: Option = get_team(&mut toks)?; + + command_or_error( + input, + &mut toks, + Self { + resolution: Resolution::Hold, + reversibility: Reversibility::Reversible, + team, + }, + ) + } + _ => Ok(None), + } + } +} + +fn get_team<'a>(toks: &mut Tokenizer<'a>) -> Result, Error<'a>> { + match toks.peek_token()? { + Some(Token::Word(team)) => { + toks.next_token()?; + + Ok(Some(team.to_string())) + } + _ => Ok(None), + } +} + +fn command_or_error<'a>( + input: &mut Tokenizer<'a>, + toks: &mut Tokenizer<'a>, + command: DecisionCommand, +) -> Result, Error<'a>> { + if let Some(Token::Dot) | Some(Token::EndOfLine) = toks.peek_token()? { + *input = toks.clone(); + Ok(Some(command)) + } else { + Err(toks.error(ParseError::ExpectedEnd)) + } +} + +#[derive(PartialEq, Eq, Debug)] +pub enum ParseError { + ExpectedEnd, +} + +impl std::error::Error for ParseError {} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ParseError::ExpectedEnd => write!(f, "expected end of command"), + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, Copy, ToSql, FromSql, Eq, PartialEq)] +#[postgres(name = "reversibility")] +pub enum Reversibility { + #[postgres(name = "reversible")] + Reversible, + #[postgres(name = "irreversible")] + Irreversible, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Copy, ToSql, FromSql, Eq, PartialEq)] +#[postgres(name = "resolution")] +pub enum Resolution { + #[postgres(name = "merge")] + Merge, + #[postgres(name = "hold")] + Hold, +} + +impl fmt::Display for Resolution { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Resolution::Merge => write!(f, "merge"), + Resolution::Hold => write!(f, "hold"), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse<'a>(input: &'a str) -> Result, Error<'a>> { + let mut toks = Tokenizer::new(input); + Ok(DecisionCommand::parse(&mut toks)?) + } + + #[test] + fn test_correct_merge() { + assert_eq!( + parse("merge"), + Ok(Some(DecisionCommand { + resolution: Resolution::Merge, + reversibility: Reversibility::Reversible, + team: None + })), + ); + } + + #[test] + fn test_correct_merge_final_dot() { + assert_eq!( + parse("merge."), + Ok(Some(DecisionCommand { + resolution: Resolution::Merge, + reversibility: Reversibility::Reversible, + team: None + })), + ); + } + + #[test] + fn test_correct_hold() { + assert_eq!( + parse("hold"), + Ok(Some(DecisionCommand { + resolution: Resolution::Hold, + reversibility: Reversibility::Reversible, + team: None + })), + ); + } + + #[test] + fn test_expected_end() { + use std::error::Error; + assert_eq!( + parse("hold lang beer") + .unwrap_err() + .source() + .unwrap() + .downcast_ref(), + Some(&ParseError::ExpectedEnd), + ); + } + + #[test] + fn test_correct_merge_with_team() { + assert_eq!( + parse("merge lang"), + Ok(Some(DecisionCommand { + resolution: Resolution::Merge, + reversibility: Reversibility::Reversible, + team: Some("lang".to_string()) + })), + ); + } + + #[test] + fn test_correct_hold_with_team() { + assert_eq!( + parse("hold lang"), + Ok(Some(DecisionCommand { + resolution: Resolution::Hold, + reversibility: Reversibility::Reversible, + team: Some("lang".to_string()) + })), + ); + } +} diff --git a/src/config.rs b/src/config.rs index 053aa682..203d2366 100644 --- a/src/config.rs +++ b/src/config.rs @@ -34,6 +34,7 @@ pub(crate) struct Config { pub(crate) note: Option, pub(crate) mentions: Option, pub(crate) no_merges: Option, + pub(crate) decision: Option, } #[derive(PartialEq, Eq, Debug, serde::Deserialize)] @@ -257,6 +258,12 @@ pub(crate) struct GitHubReleasesConfig { pub(crate) changelog_branch: String, } +#[derive(PartialEq, Eq, Debug, serde::Deserialize)] +pub(crate) struct DecisionConfig { + #[serde(default)] + _empty: (), +} + fn get_cached_config(repo: &str) -> Option, ConfigurationError>> { let cache = CONFIG_CACHE.read().unwrap(); cache.get(repo).and_then(|(config, fetch_time)| { @@ -343,6 +350,8 @@ mod tests { infra = "T-infra" [shortcut] + + [decision-process] "#; let config = toml::from_str::(&config).unwrap(); let mut ping_teams = HashMap::new(); @@ -395,6 +404,7 @@ mod tests { review_submitted: None, mentions: None, no_merges: None, + decision: None } ); } diff --git a/src/db.rs b/src/db.rs index a696be99..bd3de5cc 100644 --- a/src/db.rs +++ b/src/db.rs @@ -9,6 +9,7 @@ use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use tokio_postgres::Client as DbClient; pub mod issue_data; +pub mod issue_decision_state; pub mod jobs; pub mod notifications; pub mod rustc_commits; @@ -273,4 +274,20 @@ CREATE UNIQUE INDEX jobs_name_scheduled_at_unique_index name, scheduled_at ); ", + " +CREATE TYPE reversibility AS ENUM ('reversible', 'irreversible'); +", + " +CREATE TYPE resolution AS ENUM ('hold', 'merge'); +", + "CREATE TABLE issue_decision_state ( + issue_id BIGINT PRIMARY KEY, + initiator TEXT NOT NULL, + start_date TIMESTAMP WITH TIME ZONE NOT NULL, + end_date TIMESTAMP WITH TIME ZONE NOT NULL, + current JSONB NOT NULL, + history JSONB, + reversibility reversibility NOT NULL DEFAULT 'reversible', + resolution resolution NOT NULL DEFAULT 'merge' +);", ]; diff --git a/src/db/issue_decision_state.rs b/src/db/issue_decision_state.rs new file mode 100644 index 00000000..af673c3d --- /dev/null +++ b/src/db/issue_decision_state.rs @@ -0,0 +1,117 @@ +//! The issue decision state table provides a way to store +//! the decision process state of each issue + +use anyhow::{Context as _, Result}; +use chrono::{DateTime, Utc}; +use parser::command::decision::{Resolution, Reversibility}; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use tokio_postgres::Client as DbClient; + +#[derive(Debug, Serialize, Deserialize)] +pub struct IssueDecisionState { + pub issue_id: i64, + pub initiator: String, + pub start_date: DateTime, + pub end_date: DateTime, + pub current: BTreeMap>, + pub history: BTreeMap>, + pub reversibility: Reversibility, + pub resolution: Resolution, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct UserStatus { + pub comment_id: String, + pub text: String, + pub reversibility: Reversibility, + pub resolution: Resolution, +} + +pub async fn insert_issue_decision_state( + db: &DbClient, + issue_number: &u64, + initiator: &String, + start_date: &DateTime, + end_date: &DateTime, + current: &BTreeMap>, + history: &BTreeMap>, + reversibility: &Reversibility, + resolution: &Resolution, +) -> Result<()> { + tracing::trace!("insert_issue_decision_state(issue_id={})", issue_number); + let issue_id = *issue_number as i64; + + db.execute( + "INSERT INTO issue_decision_state (issue_id, initiator, start_date, end_date, current, history, reversibility, resolution) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + ON CONFLICT DO NOTHING", + &[&issue_id, &initiator, &start_date, &end_date, &serde_json::to_value(current).unwrap(), &serde_json::to_value(history).unwrap(), &reversibility, &resolution], + ) + .await + .context("Inserting decision state")?; + + Ok(()) +} + +pub async fn update_issue_decision_state( + db: &DbClient, + issue_number: &u64, + end_date: &DateTime, + current: &BTreeMap, + history: &BTreeMap>, + reversibility: &Reversibility, + resolution: &Resolution, +) -> Result<()> { + tracing::trace!("update_issue_decision_state(issue_id={})", issue_number); + let issue_id = *issue_number as i64; + + db.execute("UPDATE issue_decision_state SET end_date = $2, current = $3, history = $4, reversibility = $5, resolution = $6 WHERE issue_id = $1", + &[&issue_id, &end_date, &serde_json::to_value(current).unwrap(), &serde_json::to_value(history).unwrap(), &reversibility, &resolution] + ) + .await + .context("Updating decision state")?; + + Ok(()) +} + +pub async fn get_issue_decision_state( + db: &DbClient, + issue_number: &u64, +) -> Result { + tracing::trace!("get_issue_decision_state(issue_id={})", issue_number); + let issue_id = *issue_number as i64; + + let state = db + .query_one( + "SELECT * FROM issue_decision_state WHERE issue_id = $1", + &[&issue_id], + ) + .await + .context("Getting decision state data")?; + + deserialize_issue_decision_state(&state) +} + +fn deserialize_issue_decision_state(row: &tokio_postgres::row::Row) -> Result { + let issue_id: i64 = row.try_get(0)?; + let initiator: String = row.try_get(1)?; + let start_date: DateTime = row.try_get(2)?; + let end_date: DateTime = row.try_get(3)?; + let current: BTreeMap> = + serde_json::from_value(row.try_get(4).unwrap())?; + let history: BTreeMap> = + serde_json::from_value(row.try_get(5).unwrap())?; + let reversibility: Reversibility = row.try_get(6)?; + let resolution: Resolution = row.try_get(7)?; + + Ok(IssueDecisionState { + issue_id, + initiator, + start_date, + end_date, + current, + history, + reversibility, + resolution, + }) +} diff --git a/src/github.rs b/src/github.rs index 8b0cc29f..cdb587db 100644 --- a/src/github.rs +++ b/src/github.rs @@ -402,7 +402,7 @@ impl fmt::Display for IssueRepository { } impl IssueRepository { - fn url(&self) -> String { + pub fn url(&self) -> String { format!( "https://api.github.com/repos/{}/{}", self.organization, self.repository @@ -779,6 +779,27 @@ impl Issue { Ok(()) } + pub async fn merge(&self, client: &GithubClient) -> anyhow::Result<()> { + let merge_url = format!("{}/pulls/{}/merge", self.repository().url(), self.number); + + // change defaults by reading from somewhere, maybe in .toml? + #[derive(serde::Serialize)] + struct MergeIssue<'a> { + commit_title: &'a str, + merge_method: &'a str, + } + + client + ._send_req(client.put(&merge_url).json(&MergeIssue { + commit_title: "Merged by the bot!", + merge_method: "merge", + })) + .await + .context("failed to merge issue")?; + + Ok(()) + } + /// Returns the diff in this event, for Open and Synchronize events for now. pub async fn diff(&self, client: &GithubClient) -> anyhow::Result> { let (before, after) = if let (Some(base), Some(head)) = (&self.base, &self.head) { @@ -1837,7 +1858,7 @@ impl GithubClient { response.text().await.context("raw gist from url") } - fn get(&self, url: &str) -> RequestBuilder { + pub fn get(&self, url: &str) -> RequestBuilder { log::trace!("get {:?}", url); self.client.get(url).configure(self) } diff --git a/src/handlers.rs b/src/handlers.rs index da39943b..c0cf9dd1 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -26,6 +26,7 @@ impl fmt::Display for HandlerError { mod assign; mod autolabel; mod close; +mod decision; pub mod docs_update; mod github_releases; mod glacier; @@ -275,6 +276,7 @@ command_handlers! { shortcut: Shortcut, close: Close, note: Note, + decision: Decision, } pub struct Context { diff --git a/src/handlers/decision.rs b/src/handlers/decision.rs new file mode 100644 index 00000000..8818cdc2 --- /dev/null +++ b/src/handlers/decision.rs @@ -0,0 +1,340 @@ +use crate::github; +use crate::{ + config::DecisionConfig, db::issue_decision_state::*, github::Event, handlers::Context, + interactions::ErrorComment, +}; +use anyhow::bail; +use anyhow::Context as Ctx; +use chrono::{DateTime, Duration, Utc}; +use parser::command::decision::Resolution; +use parser::command::decision::*; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; + +pub const DECISION_PROCESS_JOB_NAME: &str = "decision_process_action"; + +#[derive(Serialize, Deserialize)] +pub struct DecisionProcessActionMetadata { + pub message: String, + pub get_issue_url: String, + pub status: Resolution, +} + +pub(super) async fn handle_command( + ctx: &Context, + _config: &DecisionConfig, + event: &Event, + cmd: DecisionCommand, +) -> anyhow::Result<()> { + let db = ctx.db.get().await; + + let DecisionCommand { + resolution, + reversibility, + team: team_name, + } = cmd; + + let issue = event.issue().unwrap(); + let user = event.user(); + + let is_team_member = user.is_team_member(&ctx.github).await.unwrap_or(false); + + if !is_team_member { + let cmnt = ErrorComment::new( + &issue, + "Only team members can be part of the decision process.", + ); + cmnt.post(&ctx.github).await?; + return Ok(()); + } + + match get_issue_decision_state(&db, &issue.number).await { + Ok(_state) => { + // TO DO + let cmnt = ErrorComment::new( + &issue, + "We don't support having more than one vote yet. Coming soon :)", + ); + cmnt.post(&ctx.github).await?; + + Ok(()) + } + _ => { + match team_name { + None => { + let cmnt = ErrorComment::new( + &issue, + "In the first vote, is necessary to specify the team name that will be involved in the decision process.", + ); + cmnt.post(&ctx.github).await?; + + Ok(()) + } + Some(team_name) => { + match github::get_team(&ctx.github, &team_name).await { + Ok(Some(team)) => { + let start_date: DateTime = chrono::Utc::now().into(); + let end_date: DateTime = + start_date.checked_add_signed(Duration::days(10)).unwrap(); + + let mut current: BTreeMap> = BTreeMap::new(); + let mut history: BTreeMap> = BTreeMap::new(); + + // Add team members to current and history + for member in team.members { + current.insert(member.github.clone(), None); + history.insert(member.github.clone(), Vec::new()); + } + + // Add issue user to current and history + current.insert( + user.login.clone(), + Some(UserStatus { + comment_id: event.html_url().unwrap().to_string(), + text: event.comment_body().unwrap().to_string(), + reversibility: reversibility, + resolution: resolution, + }), + ); + history.insert(user.login.clone(), Vec::new()); + + // Initialize issue decision state + insert_issue_decision_state( + &db, + &issue.number, + &user.login, + &start_date, + &end_date, + ¤t, + &history, + &reversibility, + &resolution, + ) + .await?; + + // TO DO -- Do not insert this job until we support more votes + // let metadata = serde_json::value::to_value(DecisionProcessActionMetadata { + // message: "some message".to_string(), + // get_issue_url: format!("{}/issues/{}", issue.repository().url(), issue.number), + // status: resolution, + // }) + // .unwrap(); + // insert_job( + // &db, + // &DECISION_PROCESS_JOB_NAME.to_string(), + // &end_date, + // &metadata, + // ) + // .await?; + + let comment = build_status_comment(&history, ¤t)?; + issue + .post_comment(&ctx.github, &comment) + .await + .context("merge vote comment")?; + + Ok(()) + } + _ => { + let cmnt = + ErrorComment::new(&issue, "Failed to resolve to a known team."); + cmnt.post(&ctx.github).await?; + + Ok(()) + } + } + } + } + } + } +} + +fn build_status_comment( + history: &BTreeMap>, + current: &BTreeMap>, +) -> anyhow::Result { + let mut comment = "| Team member | State |\n|-------------|-------|".to_owned(); + for (user, status) in current { + let mut user_statuses = format!("\n| {} |", user); + + // previous stasuses + match history.get(user) { + Some(statuses) => { + for status in statuses { + let status_item = format!(" ~~{}~~ ", status.resolution); + user_statuses.push_str(&status_item); + } + } + None => bail!("user {} not present in history statuses list", user), + } + + // current status + let user_resolution = match status { + Some(status) => format!("**{}**", status.resolution), + _ => "".to_string(), + }; + + let status_item = format!(" {} |", user_resolution); + user_statuses.push_str(&status_item); + + comment.push_str(&user_statuses); + } + + Ok(comment) +} + +#[cfg(test)] +mod tests { + use super::*; + use factori::{create, factori}; + + factori!(UserStatus, { + default { + comment_id = "the-comment-id".to_string(), + text = "this is my argument for making this decision".to_string(), + reversibility = Reversibility::Reversible, + resolution = Resolution::Merge + } + + mixin hold { + resolution = Resolution::Hold + } + }); + + #[test] + fn test_successfuly_build_comment() { + let mut history: BTreeMap> = BTreeMap::new(); + let mut current_statuses: BTreeMap> = BTreeMap::new(); + + // user 1 + let mut user_1_statuses: Vec = Vec::new(); + user_1_statuses.push(create!(UserStatus)); + user_1_statuses.push(create!(UserStatus, :hold)); + + history.insert("Niklaus".to_string(), user_1_statuses); + + current_statuses.insert("Niklaus".to_string(), Some(create!(UserStatus))); + + // user 2 + let mut user_2_statuses: Vec = Vec::new(); + user_2_statuses.push(create!(UserStatus, :hold)); + user_2_statuses.push(create!(UserStatus)); + + history.insert("Barbara".to_string(), user_2_statuses); + + current_statuses.insert("Barbara".to_string(), Some(create!(UserStatus))); + + let build_result = build_status_comment(&history, ¤t_statuses) + .expect("it shouldn't fail building the message"); + let expected_comment = "| Team member | State |\n\ + |-------------|-------|\n\ + | Barbara | ~~hold~~ ~~merge~~ **merge** |\n\ + | Niklaus | ~~merge~~ ~~hold~~ **merge** |" + .to_string(); + + assert_eq!(build_result, expected_comment); + } + + #[test] + fn test_successfuly_build_comment_user_no_votes() { + let mut history: BTreeMap> = BTreeMap::new(); + let mut current_statuses: BTreeMap> = BTreeMap::new(); + + // user 1 + let mut user_1_statuses: Vec = Vec::new(); + user_1_statuses.push(create!(UserStatus)); + user_1_statuses.push(create!(UserStatus, :hold)); + + history.insert("Niklaus".to_string(), user_1_statuses); + + current_statuses.insert("Niklaus".to_string(), Some(create!(UserStatus))); + + // user 2 + let mut user_2_statuses: Vec = Vec::new(); + user_2_statuses.push(create!(UserStatus, :hold)); + user_2_statuses.push(create!(UserStatus)); + + history.insert("Barbara".to_string(), user_2_statuses); + + current_statuses.insert("Barbara".to_string(), Some(create!(UserStatus))); + + // user 3 + history.insert("Tom".to_string(), Vec::new()); + + current_statuses.insert("Tom".to_string(), None); + + let build_result = build_status_comment(&history, ¤t_statuses) + .expect("it shouldn't fail building the message"); + let expected_comment = "| Team member | State |\n\ + |-------------|-------|\n\ + | Barbara | ~~hold~~ ~~merge~~ **merge** |\n\ + | Niklaus | ~~merge~~ ~~hold~~ **merge** |\n\ + | Tom | |" + .to_string(); + + assert_eq!(build_result, expected_comment); + } + + #[test] + fn test_build_comment_inconsistent_users() { + let mut history: BTreeMap> = BTreeMap::new(); + let mut current_statuses: BTreeMap> = BTreeMap::new(); + + // user 1 + let mut user_1_statuses: Vec = Vec::new(); + user_1_statuses.push(create!(UserStatus)); + user_1_statuses.push(create!(UserStatus, :hold)); + + history.insert("Niklaus".to_string(), user_1_statuses); + + current_statuses.insert("Niklaus".to_string(), Some(create!(UserStatus))); + + // user 2 + let mut user_2_statuses: Vec = Vec::new(); + user_2_statuses.push(create!(UserStatus, :hold)); + user_2_statuses.push(create!(UserStatus)); + + history.insert("Barbara".to_string(), user_2_statuses); + + current_statuses.insert("Martin".to_string(), Some(create!(UserStatus))); + + let build_result = build_status_comment(&history, ¤t_statuses); + assert_eq!( + format!("{}", build_result.unwrap_err()), + "user Martin not present in history statuses list" + ); + } + + #[test] + fn test_successfuly_build_comment_no_history() { + let mut history: BTreeMap> = BTreeMap::new(); + let mut current_statuses: BTreeMap> = BTreeMap::new(); + + // user 1 + let mut user_1_statuses: Vec = Vec::new(); + user_1_statuses.push(create!(UserStatus)); + user_1_statuses.push(create!(UserStatus, :hold)); + + current_statuses.insert("Niklaus".to_string(), Some(create!(UserStatus))); + history.insert("Niklaus".to_string(), Vec::new()); + + // user 2 + let mut user_2_statuses: Vec = Vec::new(); + user_2_statuses.push(create!(UserStatus, :hold)); + user_2_statuses.push(create!(UserStatus)); + + current_statuses.insert("Barbara".to_string(), Some(create!(UserStatus))); + history.insert("Barbara".to_string(), Vec::new()); + + let build_result = build_status_comment(&history, ¤t_statuses) + .expect("it shouldn't fail building the message"); + let expected_comment = "| Team member | State |\n\ + |-------------|-------|\n\ + | Barbara | **merge** |\n\ + | Niklaus | **merge** |\ + " + .to_string(); + + assert_eq!(build_result, expected_comment); + } +} diff --git a/src/handlers/jobs.rs b/src/handlers/jobs.rs index adb05e4a..8c3ac3c2 100644 --- a/src/handlers/jobs.rs +++ b/src/handlers/jobs.rs @@ -5,6 +5,11 @@ // Further info could be find in src/jobs.rs use super::Context; +use crate::github::*; +use crate::handlers::decision::{DecisionProcessActionMetadata, DECISION_PROCESS_JOB_NAME}; +use parser::command::decision::Resolution::{Hold, Merge}; +use reqwest::Client; +use tracing as log; pub async fn handle_job( ctx: &Context, @@ -17,6 +22,7 @@ pub async fn handle_job( super::rustc_commits::synchronize_commits_inner(ctx, None).await; Ok(()) } + DECISION_PROCESS_JOB_NAME => decision_process_handler(&metadata).await, _ => default(&name, &metadata), } } @@ -30,3 +36,28 @@ fn default(name: &String, metadata: &serde_json::Value) -> anyhow::Result<()> { Ok(()) } + +async fn decision_process_handler(metadata: &serde_json::Value) -> anyhow::Result<()> { + tracing::trace!( + "handle_job fell into decision process case: (metadata={:?})", + metadata + ); + + let metadata: DecisionProcessActionMetadata = serde_json::from_value(metadata.clone())?; + let gh_client = GithubClient::new_with_default_token(Client::new().clone()); + let request = gh_client.get(&metadata.get_issue_url); + + match gh_client.json::(request).await { + Ok(issue) => match metadata.status { + Merge => issue.merge(&gh_client).await?, + Hold => issue.close(&gh_client).await?, + }, + Err(e) => log::error!( + "Failed to get issue {}, error: {}", + metadata.get_issue_url, + e + ), + } + + Ok(()) +} diff --git a/test/decision/01_merging_proposal__1.md b/test/decision/01_merging_proposal__1.md new file mode 100644 index 00000000..f77ea4e1 --- /dev/null +++ b/test/decision/01_merging_proposal__1.md @@ -0,0 +1,8 @@ +Hello! @Alan has proposed to merge this. This is a **reversible** decision, which means that it will be affirmed once the "final comment period" of 10 days have passed, unless a team member places a "hold" on the decision (or cancels it). + +| Team member | State | +|-------------|-------| +| @Alan | [merge](http://example.com/issue/1#comment=1) | +| @Barbara | | +| @Grace | | +| @Niklaus | | diff --git a/test/decision/01_merging_proposal__2.md b/test/decision/01_merging_proposal__2.md new file mode 100644 index 00000000..ae2225f0 --- /dev/null +++ b/test/decision/01_merging_proposal__2.md @@ -0,0 +1,8 @@ +Hello! @Alan has proposed to merge this. This is a **reversible** decision, which means that it will be affirmed once the "final comment period" of 10 days have passed, unless a team member places a "hold" on the decision (or cancels it). + +| Team member | State | +|-------------|-------| +| @Alan | [merge](http://example.com/issue/1#comment=1) | +| @Barbara | [hold](http://example.com/issue/1#comment=2) | +| @Grace | | +| @Niklaus | | diff --git a/test/decision/01_merging_proposal__3.md b/test/decision/01_merging_proposal__3.md new file mode 100644 index 00000000..dd8676c9 --- /dev/null +++ b/test/decision/01_merging_proposal__3.md @@ -0,0 +1,8 @@ +Hello! @Alan has proposed to merge this. This is a **reversible** decision, which means that it will be affirmed once the "final comment period" of 10 days have passed, unless a team member places a "hold" on the decision (or cancels it). + +| Team member | History | State | +|-------------|---------|-------| +| @Alan | | [merge](http://example.com/issue/1#comment=1) | +| @Barbara | [hold](http://example.com/issue/1#comment=2) | [merge](http://example.com/issue/1#comment=3) | +| @Grace | | | +| @Niklaus | | |