From ce8e298c0f50cc8337718e12ed38b8db0d3db0c4 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 16 Aug 2018 05:32:09 +0200 Subject: [PATCH] [Needs review] Feature: @rfcbot poll [teams] (#219) * add itertools as dependency. * add database model for Poll stuff. * really add itertools as dependency in main.rs. * teams: expose test data, change team names a bit, expose ping and name of teams. * refactor a bunch of logic and implement polling (hopefully...). * update command grammar in README.md. * fix syntax error in up.sql migration. * fix copy error in up.sql migration. * fix typo in up.sql migration. * close poll when everyone has answered it. * fix #212 by filtering in unstarted fcps. (#223) * Update toolchain and lockfile (#232) * update toolchain and lockfile. * add never_type gate. * Add Centril to the lang team (#230) * fix typos (quized -> quizzed). * fix 225, and deal with leading whitespace. * colocate from_invocation_line and from_str_all. * get rid of global state from command parser. * review -> response for polls. * fix comments. * make progress on all 3 in evaluate_nags. * RfcBotCommand::{AskQuestion -> StartPoll}. * gracefully handle poll comment typos. * fix bug. --- Cargo.lock | 16 + Cargo.toml | 1 + README.md | 12 + .../down.sql | 2 + .../up.sql | 19 + src/domain/rfcbot.rs | 46 + src/domain/schema.rs | 27 + src/github/command.rs | 489 +++++++ src/github/mod.rs | 1 + src/github/nag.rs | 1141 ++++++++--------- src/main.rs | 1 + src/teams.rs | 35 +- 12 files changed, 1174 insertions(+), 616 deletions(-) create mode 100644 migrations/2018-06-20-062854_create_poll_table/down.sql create mode 100644 migrations/2018-06-20-062854_create_poll_table/up.sql create mode 100644 src/github/command.rs diff --git a/Cargo.lock b/Cargo.lock index 12998a20..9418bf1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -222,6 +222,11 @@ name = "dtoa" version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "either" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "env_logger" version = "0.4.3" @@ -366,6 +371,14 @@ dependencies = [ "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "itertools" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "either 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "itoa" version = "0.1.1" @@ -711,6 +724,7 @@ dependencies = [ "hex 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "hyper 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)", "hyper-native-tls 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", + "itertools 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "maplit 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1139,6 +1153,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum dotenv 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "400b347fe65ccfbd8f545c9d9a75d04b0caf23fec49aaa838a9a05398f94c019" "checksum dtoa 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0dd841b58510c9618291ffa448da2e4e0f699d984d436122372f446dae62263d" "checksum dtoa 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6d301140eb411af13d3115f9a562c85cc6b541ade9dfa314132244aaee7489dd" +"checksum either 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3be565ca5c557d7f59e7cfcf1844f9e3033650c929c6566f511e8005f205c1d0" "checksum env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "3ddf21e73e016298f5cb37d6ef8e8da8e39f91f9ec8b0df44b7deb16a9f8cd5b" "checksum error 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)" = "a6e606f14042bb87cc02ef6a14db6c90ab92ed6f62d87e69377bc759fd7987cc" "checksum error-chain 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d9435d864e017c3c6afeac1654189b06cdb491cf2ff73dbf0d73b0f292f42ff8" @@ -1155,6 +1170,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum idna 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "38f09e0f0b1fb55fdee1f17470ad800da77af5186a1a76c026b679358b7e844e" "checksum iron 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2440ae846e7a8c7f9b401db8f6e31b4ea5e7d3688b91761337da7e054520c75b" "checksum isatty 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "6c324313540cd4d7ba008d43dc6606a32a5579f13cc17b2804c13096f0a5c522" +"checksum itertools 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)" = "f58856976b776fedd95533137617a02fb25719f40e7d9b01c7043cd65474f450" "checksum itoa 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ae3088ea4baeceb0284ee9eea42f591226e6beaecf65373e41b38d95a1b8e7a1" "checksum itoa 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "5adb58558dcd1d786b5f0bd15f3226ee23486e24b7b58304b60f64dc68e62606" "checksum language-tags 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a91d884b6667cd606bb5a69aa0c99ba811a115fc68915e7056ec08a46e93199a" diff --git a/Cargo.toml b/Cargo.toml index aad468b0..91517f2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ toml = "0.4" url = "1.4" urlencoded = "0.5" maplit = "1.0.1" +itertools = "0.7.8" [dependencies.chrono] features = ["serde"] diff --git a/README.md b/README.md index 8e3433d7..dc8d1078 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,17 @@ cancel ::= "cancel | "canceled" | "canceling" | "cancels" ; review ::= "reviewed" | "review" | "reviewing" | "reviews" ; concern ::= "concern" | "concerned" | "concerning" | "concerns" ; resolve ::= "resolve" | "resolved" | "resolving" | "resolves" ; +poll ::= "ask" | "asked" | "asking" | "asks" | + "poll" | "polled" | "polling" | "polls" | + "query" | "queried" | "querying" | "queries" | + "inquire" | "inquired" | "inquiring" | "inquires" | + "quiz" | "quizzed" | "quizzing" | "quizzes" | + "survey" | "surveyed" | "surveying" | "surveys" ; + +team_label ::= "T-lang" | .. ; +team_label_simple ::= "lang" | .. ; +team_ping ::= "@"? "rust-lang/lang" | ..; +team_target ::= team_label | team_label_simple | team_ping ; line_remainder ::= .+$ ; ws_separated ::= ... ; @@ -59,6 +70,7 @@ ws_separated ::= ... ; subcommand ::= merge | close | postpone | cancel | review | concern line_remainder | resolve line_remainder + | poll [team_target]* line_remainder ; invocation ::= "fcp" subcommand diff --git a/migrations/2018-06-20-062854_create_poll_table/down.sql b/migrations/2018-06-20-062854_create_poll_table/down.sql new file mode 100644 index 00000000..4590c61d --- /dev/null +++ b/migrations/2018-06-20-062854_create_poll_table/down.sql @@ -0,0 +1,2 @@ +DROP TABLE poll; +DROP TABLE poll_response_request; diff --git a/migrations/2018-06-20-062854_create_poll_table/up.sql b/migrations/2018-06-20-062854_create_poll_table/up.sql new file mode 100644 index 00000000..a956357e --- /dev/null +++ b/migrations/2018-06-20-062854_create_poll_table/up.sql @@ -0,0 +1,19 @@ +CREATE TABLE poll ( + id SERIAL PRIMARY KEY, + fk_issue INTEGER UNIQUE NOT NULL REFERENCES issue (id), + fk_initiator INTEGER NOT NULL REFERENCES githubuser (id), + fk_initiating_comment INTEGER NOT NULL REFERENCES issuecomment (id), + fk_bot_tracking_comment INTEGER NOT NULL REFERENCES issuecomment (id), + poll_question VARCHAR NOT NULL, + poll_created_at TIMESTAMP NOT NULL, + poll_closed BOOLEAN NOT NULL, + poll_teams VARCHAR NOT NULL +); + +CREATE TABLE poll_response_request ( + id SERIAL PRIMARY KEY, + fk_poll INTEGER NOT NULL REFERENCES poll (id) ON DELETE CASCADE, + fk_respondent INTEGER NOT NULL REFERENCES githubuser (id), + responded BOOLEAN NOT NULL, + UNIQUE (fk_poll, fk_respondent) +); diff --git a/src/domain/rfcbot.rs b/src/domain/rfcbot.rs index 260d22c9..1751e153 100644 --- a/src/domain/rfcbot.rs +++ b/src/domain/rfcbot.rs @@ -2,6 +2,34 @@ use chrono::NaiveDateTime; use super::schema::*; +#[derive(Clone, Debug, Eq, Ord, Insertable, PartialEq, PartialOrd)] +#[table_name="poll"] +pub struct NewPoll<'a> { + pub fk_issue: i32, + pub fk_initiator: i32, + pub fk_initiating_comment: i32, + pub fk_bot_tracking_comment: i32, + pub poll_question: &'a str, + pub poll_created_at: NaiveDateTime, + pub poll_closed: bool, + pub poll_teams: &'a str, +} + +#[derive(AsChangeset, Clone, Debug, Deserialize, Eq, Ord, + PartialEq, PartialOrd, Queryable, Serialize)] +#[table_name="poll"] +pub struct Poll { + pub id: i32, + pub fk_issue: i32, + pub fk_initiator: i32, + pub fk_initiating_comment: i32, + pub fk_bot_tracking_comment: i32, + pub poll_question: String, + pub poll_created_at: NaiveDateTime, + pub poll_closed: bool, + pub poll_teams: String, +} + #[derive(Clone, Debug, Eq, Ord, Insertable, PartialEq, PartialOrd)] #[table_name="fcp_proposal"] pub struct NewFcpProposal<'a> { @@ -14,6 +42,24 @@ pub struct NewFcpProposal<'a> { pub fcp_closed: bool, } +#[derive(Clone, Debug, Eq, Insertable, Ord, PartialEq, PartialOrd, Serialize)] +#[table_name="poll_response_request"] +pub struct NewPollResponseRequest { + pub fk_poll: i32, + pub fk_respondent: i32, + pub responded: bool, +} + +#[derive(AsChangeset, Clone, Debug, Deserialize, Eq, Ord, + PartialEq, PartialOrd, Queryable, Serialize)] +#[table_name="poll_response_request"] +pub struct PollResponseRequest { + pub id: i32, + pub fk_poll: i32, + pub fk_respondent: i32, + pub responded: bool, +} + #[derive(AsChangeset, Clone, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Queryable, Serialize)] #[table_name="fcp_proposal"] diff --git a/src/domain/schema.rs b/src/domain/schema.rs index 7583840a..181cac7e 100644 --- a/src/domain/schema.rs +++ b/src/domain/schema.rs @@ -129,6 +129,29 @@ table! { } } +table! { + poll (id) { + id -> Int4, + fk_issue -> Int4, + fk_initiator -> Int4, + fk_initiating_comment -> Int4, + fk_bot_tracking_comment -> Int4, + poll_question -> Varchar, + poll_created_at -> Timestamp, + poll_closed -> Bool, + poll_teams -> Varchar, + } +} + +table! { + poll_response_request (id) { + id -> Int4, + fk_poll -> Int4, + fk_respondent -> Int4, + responded -> Bool, + } +} + joinable!(fcp_concern -> githubuser (fk_initiator)); joinable!(fcp_concern -> fcp_proposal (fk_proposal)); joinable!(fcp_proposal -> githubuser (fk_initiator)); @@ -143,3 +166,7 @@ joinable!(pullrequest -> githubuser (fk_assignee)); joinable!(pullrequest -> milestone (fk_milestone)); joinable!(rfc_feedback_request -> issuecomment (fk_feedback_comment)); joinable!(rfc_feedback_request -> issue (fk_issue)); +joinable!(poll -> githubuser (fk_initiator)); +joinable!(poll -> issue (fk_issue)); +joinable!(poll_response_request -> poll (fk_poll)); +joinable!(poll_response_request -> githubuser (fk_respondent)); diff --git a/src/github/command.rs b/src/github/command.rs new file mode 100644 index 00000000..3525e1f5 --- /dev/null +++ b/src/github/command.rs @@ -0,0 +1,489 @@ +use std::collections::BTreeSet; +use std::fmt; + +use error::{DashResult, DashError}; +use config::RFC_BOT_MENTION; +use teams::{TeamLabel, RfcbotConfig}; + +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub enum Label { + FFCP, + PFCP, + FCP, + Postponed, + Closed, + DispositionMerge, + DispositionClose, + DispositionPostpone, +} + +impl Label { + pub fn as_str(self) -> &'static str { + use self::Label::*; + match self { + FFCP => "finished-final-comment-period", + PFCP => "proposed-final-comment-period", + FCP => "final-comment-period", + Postponed => "postponed", + Closed => "closed", + DispositionMerge => "disposition-merge", + DispositionClose => "disposition-close", + DispositionPostpone => "disposition-postpone", + } + } +} + +impl fmt::Display for Label { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str(self.as_str()) + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum FcpDisposition { + Merge, + Close, + Postpone, +} + +const FCP_REPR_MERGE: &'static str = "merge"; +const FCP_REPR_CLOSE: &'static str = "close"; +const FCP_REPR_POSTPONE: &'static str = "postpone"; + +impl FcpDisposition { + pub fn repr(self) -> &'static str { + match self { + FcpDisposition::Merge => FCP_REPR_MERGE, + FcpDisposition::Close => FCP_REPR_CLOSE, + FcpDisposition::Postpone => FCP_REPR_POSTPONE, + } + } + + pub fn from_str(string: &str) -> DashResult { + Ok(match string { + FCP_REPR_MERGE => FcpDisposition::Merge, + FCP_REPR_CLOSE => FcpDisposition::Close, + FCP_REPR_POSTPONE => FcpDisposition::Postpone, + _ => throw!(DashError::Misc(None)), + }) + } + + pub fn label(self) -> Label { + match self { + FcpDisposition::Merge => Label::DispositionMerge, + FcpDisposition::Close => Label::DispositionClose, + FcpDisposition::Postpone => Label::DispositionPostpone, + } + } +} + +/// Parses the text of a subcommand. +fn parse_command_text<'a>(command: &'a str, subcommand: &'a str) -> &'a str { + let name_start = command.find(subcommand).unwrap() + subcommand.len(); + command[name_start..].trim() +} + +fn strip_prefix<'h>(haystack: &'h str, prefix: &str) -> &'h str { + haystack.find(prefix) + .map(|idx| &haystack[idx + prefix.len()..]) + .unwrap_or(haystack) + .trim() +} + +fn match_team_candidate<'a> + (setup: &'a RfcbotConfig, team_candidate: &str) + -> Option<&'a TeamLabel> +{ + setup.teams().find(|&(label, team)| { + strip_prefix(&label.0, "T-") == strip_prefix(team_candidate, "T-") || + team.ping() == strip_prefix(team_candidate, "@") + }).map(|(label, _)| label) +} + +/// Parses all subcommands under the fcp command. +/// If `fcp_context` is set to false, `@rfcbot ` +/// was passed and not `@rfcbot fcp `. +/// +/// @rfcbot accepts roughly the following grammar: +/// +/// merge ::= "merge" | "merged" | "merging" | "merges" ; +/// close ::= "close" | "closed" | "closing" | "closes" ; +/// postpone ::= "postpone" | "postponed" | "postponing" | "postpones" ; +/// cancel ::= "cancel | "canceled" | "canceling" | "cancels" ; +/// review ::= "reviewed" | "review" | "reviewing" | "reviews" ; +/// concern ::= "concern" | "concerned" | "concerning" | "concerns" ; +/// resolve ::= "resolve" | "resolved" | "resolving" | "resolves" ; +/// poll ::= "ask" | "asked" | "asking" | "asks" | +/// "poll" | "polled" | "polling" | "polls" | +/// "query" | "queried" | "querying" | "queries" | +/// "inquire" | "inquired" | "inquiring" | "inquires" | +/// "quiz" | "quizzed" | "quizzing" | "quizzes" | +/// "survey" | "surveyed" | "surveying" | "surveys" ; +/// +/// team_label ::= "T-lang" | .. ; +/// team_label_simple ::= "lang" | .. ; +/// team_ping ::= "@"? "rust-lang/lang" | ..; +/// team_target ::= team_label | team_label_simple | team_ping ; +/// +/// line_remainder ::= .+$ ; +/// ws_separated ::= ... ; +/// +/// subcommand ::= merge | close | postpone | cancel | review +/// | concern line_remainder +/// | resolve line_remainder +/// | poll [team_target]* line_remainder +/// ; +/// +/// invocation ::= "fcp" subcommand +/// | "pr" subcommand +/// | "f?" ws_separated +/// | subcommand +/// ; +/// +/// grammar ::= "@rfcbot" ":"? invocation ; +fn parse_fcp_subcommand<'a>( + setup: &'a RfcbotConfig, + command: &'a str, + subcommand: &'a str, + fcp_context: bool +) -> DashResult> { + Ok(match subcommand { + // Parse a FCP merge command: + "merge" | "merged" | "merging" | "merges" => + RfcBotCommand::FcpPropose(FcpDisposition::Merge), + + // Parse a FCP close command: + "close" | "closed" | "closing" | "closes" => + RfcBotCommand::FcpPropose(FcpDisposition::Close), + + // Parse a FCP postpone command: + "postpone" | "postponed" | "postponing" | "postpones" => + RfcBotCommand::FcpPropose(FcpDisposition::Postpone), + + // Parse a FCP cancel command: + "cancel" | "canceled" | "canceling" | "cancels" => + RfcBotCommand::FcpCancel, + + // Parse a FCP reviewed command: + "reviewed" | "review" | "reviewing" | "reviews" => + RfcBotCommand::Reviewed, + + // Parse a FCP concern command: + "concern" | "concerned" | "concerning" | "concerns" => { + debug!("Parsed command as NewConcern"); + RfcBotCommand::NewConcern(parse_command_text(command, subcommand)) + }, + + // Parse a FCP resolve command: + "resolve" | "resolved" | "resolving" | "resolves" => { + debug!("Parsed command as ResolveConcern"); + RfcBotCommand::ResolveConcern(parse_command_text(command, subcommand)) + }, + + // Parse a StartPoll command: + "ask" | "asked" | "asking" | "asks" | + "poll" | "polled" | "polling" | "polls" | + "query" | "queried" | "querying" | "queries" | + "inquire" | "inquired" | "inquiring" | "inquires" | + "quiz" | "quizzed" | "quizzing" | "quizzes" | + "survey" | "surveyed" | "surveying" | "surveys" => { + debug!("Parsed command as StartPoll"); + + let mut question = parse_command_text(command, subcommand); + let mut teams = BTreeSet::new(); + while let Some(team_candidate) = question.split_whitespace().next() { + if let Some(team) = match_team_candidate(setup, team_candidate) { + question = parse_command_text(question, team_candidate); + teams.insert(&*team.0); + } else { + break; + } + } + RfcBotCommand::StartPoll { teams, question } + }, + + _ => { + throw!(DashError::Misc(if fcp_context { + error!("unrecognized subcommand for fcp: {}", subcommand); + Some(format!("found bad subcommand: {}", subcommand)) + } else { + None + })) + } + }) +} + +fn from_invocation_line<'a> + (setup: &'a RfcbotConfig, command: &'a str) -> DashResult> +{ + let mut tokens = command.trim_left_matches(RFC_BOT_MENTION).trim() + .trim_left_matches(':').trim() + .split_whitespace(); + let invocation = tokens.next().ok_or(DashError::Misc(None))?; + match invocation { + "fcp" | "pr" => { + let subcommand = tokens.next().ok_or(DashError::Misc(None))?; + + debug!("Parsed command as new FCP proposal"); + + parse_fcp_subcommand(setup, command, subcommand, true) + } + "f?" => { + let user = + tokens + .next() + .ok_or_else(|| DashError::Misc(Some("no user specified".to_string())))?; + + if user.is_empty() { + throw!(DashError::Misc(Some("no user specified".to_string()))); + } + + Ok(RfcBotCommand::FeedbackRequest(&user[1..])) + } + _ => parse_fcp_subcommand(setup, command, invocation, false), + } +} + +#[derive(Debug, Eq, PartialEq)] +pub enum RfcBotCommand<'a> { + FcpPropose(FcpDisposition), + FcpCancel, + Reviewed, + NewConcern(&'a str), + ResolveConcern(&'a str), + FeedbackRequest(&'a str), + StartPoll { + teams: BTreeSet<&'a str>, + question: &'a str, + }, +} + +impl<'a> RfcBotCommand<'a> { + pub fn from_str_all(setup: &'a RfcbotConfig, command: &'a str) + -> impl Iterator> + { + // Get the tokens for each command line (starts with a bot mention) + command.lines() + .map(|l| l.trim()) + .filter(|&l| l.starts_with(RFC_BOT_MENTION)) + .map(move |l| from_invocation_line(setup, l)) + .filter_map(Result::ok) + } +} + +#[cfg(test)] +mod test { + use super::*; + use teams::test::TEST_SETUP; + + fn parse_commands(body: &str) -> impl Iterator> { + RfcBotCommand::from_str_all(&TEST_SETUP, body) + } + + #[test] + fn multiple_commands() { +let text = r#" +someothertext +@rfcbot: resolved CONCERN_NAME +somemoretext + +somemoretext + +@rfcbot: fcp cancel +foobar +@rfcbot concern foobar +"#; + + assert_eq!(parse_commands(text).collect::>(), vec![ + RfcBotCommand::ResolveConcern("CONCERN_NAME"), + RfcBotCommand::FcpCancel, + RfcBotCommand::NewConcern("foobar"), + ]); + } + + #[test] + fn accept_leading_whitespace() { +let text = r#" +someothertext + @rfcbot: resolved CONCERN_NAME +somemoretext + +somemoretext + + @rfcbot: fcp cancel +foobar + @rfcbot concern foobar +"#; + + assert_eq!(parse_commands(text).collect::>(), vec![ + RfcBotCommand::ResolveConcern("CONCERN_NAME"), + RfcBotCommand::FcpCancel, + RfcBotCommand::NewConcern("foobar"), + ]); + } + + #[test] + fn fix_issue_225() { +let text = r#" +someothertext + @rfcbot : resolved CONCERN_NAME +somemoretext + +somemoretext + +@rfcbot : fcp cancel +foobar +@rfcbot : concern foobar +"#; + + assert_eq!(parse_commands(text).collect::>(), vec![ + RfcBotCommand::ResolveConcern("CONCERN_NAME"), + RfcBotCommand::FcpCancel, + RfcBotCommand::NewConcern("foobar"), + ]); + } + + fn ensure_take_singleton(mut iter: I) -> I::Item { + let singleton = iter.next().unwrap(); + assert!(iter.next().is_none()); + singleton + } + + macro_rules! justification { + () => { "\n\nSome justification here." }; + } + + macro_rules! some_text { + ($important: expr) => { + concat!(" ", $important, " +someothertext +somemoretext + +somemoretext") + }; + } + + macro_rules! test_from_str { + ($test: ident, [$($cmd: expr),+], $message: expr, $expected: expr) => { + test_from_str!($test, [$(concat!($cmd, $message)),+], $expected); + }; + + ($test: ident, [$($cmd: expr),+], $expected: expr) => { + #[test] + fn $test() { + let expected = $expected; + + $({ + let body = concat!("@rfcbot: ", $cmd); + let body_no_colon = concat!("@rfcbot ", $cmd); + + let with_colon = ensure_take_singleton(parse_commands(body)); + let without_colon = ensure_take_singleton(parse_commands(body_no_colon)); + + assert_eq!(with_colon, without_colon); + assert_eq!(with_colon, expected); + })+ + } + }; + } + + test_from_str!(success_fcp_reviewed, + ["reviewed", "review", "reviewing", "reviews", + "fcp reviewed", "fcp review", "fcp reviewing", + "pr reviewed", "pr review", "pr reviewing"], + RfcBotCommand::Reviewed); + + test_from_str!(success_fcp_merge, + ["merge", "merged", "merging", "merges", + "fcp merge", "fcp merged", "fcp merging", "fcp merges", + "pr merge", "pr merged", "pr merging", "pr merges"], + justification!(), + RfcBotCommand::FcpPropose(FcpDisposition::Merge)); + + test_from_str!(success_fcp_close, + ["close", "closed", "closing", "closes", + "fcp close", "fcp closed", "fcp closing", "fcp closes", + "pr close", "pr closed", "pr closing", "pr closes"], + justification!(), + RfcBotCommand::FcpPropose(FcpDisposition::Close)); + + test_from_str!(success_fcp_postpone, + ["postpone", "postponed", "postponing", "postpones", + "fcp postpone", "fcp postponed", "fcp postponing", "fcp postpones", + "pr postpone", "pr postponed", "pr postponing", "pr postpones"], + justification!(), + RfcBotCommand::FcpPropose(FcpDisposition::Postpone)); + + test_from_str!(success_fcp_cancel, + ["cancel", "canceled", "canceling", "cancels", + "fcp cancel", "fcp canceled", "fcp canceling", "fcp cancels", + "pr cancel", "pr canceled", "pr canceling", "pr cancels"], + justification!(), + RfcBotCommand::FcpCancel); + + test_from_str!(success_concern, + ["concern", "concerned", "concerning", "concerns", + "fcp concern", "fcp concerned", "fcp concerning", "fcp concerns", + "pr concern", "pr concerned", "pr concerning", "pr concerns"], + some_text!("CONCERN_NAME"), + RfcBotCommand::NewConcern("CONCERN_NAME")); + + test_from_str!(success_resolve, + ["resolve", "resolved", "resolving", "resolves", + "fcp resolve", "fcp resolved", "fcp resolving", "fcp resolves", + "pr resolve", "pr resolved", "pr resolving", "pr resolves"], + some_text!("CONCERN_NAME"), + RfcBotCommand::ResolveConcern("CONCERN_NAME")); + + test_from_str!(success_ask_question, + ["ask", "asked", "asking", "asks", + "poll", "polled", "polling", "polls", + "query", "queried", "querying", "queries", + "inquire", "inquired", "inquiring", "inquires", + "quiz", "quizzed", "quizzing", "quizzes", + "survey", "surveyed", "surveying", "surveys", + "fcp ask", "fcp asked", "fcp asking", "fcp asks", + "fcp poll", "fcp polled", "fcp polling", "fcp polls", + "fcp query", "fcp queried", "fcp querying", "fcp queries", + "fcp inquire", "fcp inquired", "fcp inquiring", "fcp inquires", + "fcp quiz", "fcp quizzed", "fcp quizzing", "fcp quizzes", + "fcp survey", "fcp surveyed", "fcp surveying", "fcp surveys", + "pr ask", "pr asked", "pr asking", "pr asks", + "pr poll", "pr polled", "pr polling", "pr polls", + "pr query", "pr queried", "pr querying", "pr queries", + "pr inquire", "pr inquired", "pr inquiring", "pr inquires", + "pr quiz", "pr quizzed", "pr quizzing", "pr quizzes", + "pr survey", "pr surveyed", "pr surveying", "pr surveys"], + some_text!("avengers T-justice-league TO BE OR NOT TO BE?"), + RfcBotCommand::StartPoll { + teams: btreeset! { + "T-avengers", + "justice-league", + }, + question: "TO BE OR NOT TO BE?", + }); + + #[test] + fn success_resolve_mid_body() { + let body = "someothertext +@rfcbot: resolved CONCERN_NAME +somemoretext + +somemoretext"; + let body_no_colon = "someothertext +somemoretext + +@rfcbot resolved CONCERN_NAME + +somemoretext"; + + let with_colon = ensure_take_singleton(parse_commands(body)); + let without_colon = ensure_take_singleton(parse_commands(body_no_colon)); + + assert_eq!(with_colon, without_colon); + assert_eq!(with_colon, RfcBotCommand::ResolveConcern("CONCERN_NAME")); + } + + test_from_str!(success_feedback, ["f?"], some_text!("@bob"), + RfcBotCommand::FeedbackRequest("bob")); +} diff --git a/src/github/mod.rs b/src/github/mod.rs index 1a6023de..5424c28d 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -3,6 +3,7 @@ pub mod client; pub mod models; +mod command; mod nag; pub mod webhooks; diff --git a/src/github/nag.rs b/src/github/nag.rs index 1ae0e238..16561dfa 100644 --- a/src/github/nag.rs +++ b/src/github/nag.rs @@ -1,55 +1,24 @@ use std::collections::BTreeSet; use std::sync::Mutex; -use std::fmt; use chrono::{Duration, Utc}; use diesel::prelude::*; use diesel; -use config::RFC_BOT_MENTION; +use itertools::Itertools; + use DB_POOL; use domain::github::{GitHubUser, Issue, IssueComment}; use domain::rfcbot::{FcpConcern, FcpProposal, FcpReviewRequest, FeedbackRequest, NewFcpProposal, - NewFcpConcern, NewFcpReviewRequest, NewFeedbackRequest}; + NewFcpConcern, NewFcpReviewRequest, NewFeedbackRequest, + NewPoll, Poll, NewPollResponseRequest, PollResponseRequest}; use domain::schema::*; use error::*; use github::models::CommentFromJson; use teams::SETUP; use super::GH; -#[derive(Copy, Clone, PartialEq, Eq, Hash)] -pub enum Label { - FFCP, - PFCP, - FCP, - Postponed, - Closed, - DispositionMerge, - DispositionClose, - DispositionPostpone, -} - -impl Label { - fn as_str(self) -> &'static str { - use self::Label::*; - match self { - FFCP => "finished-final-comment-period", - PFCP => "proposed-final-comment-period", - FCP => "final-comment-period", - Postponed => "postponed", - Closed => "closed", - DispositionMerge => "disposition-merge", - DispositionClose => "disposition-close", - DispositionPostpone => "disposition-postpone", - } - } -} - -impl fmt::Display for Label { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str(self.as_str()) - } -} +use github::command::*; impl Issue { fn remove_label(&self, label: Label) { @@ -87,7 +56,7 @@ pub fn update_nags(comment: &IssueComment) -> DashResult<()> { // Attempt to parse all commands out of the comment let mut any = false; - for command in RfcBotCommand::from_str_all(&comment.body) { + for command in RfcBotCommand::from_str_all(&SETUP, &comment.body) { any = true; // Don't accept bot commands from non-subteam members. @@ -115,8 +84,7 @@ pub fn update_nags(comment: &IssueComment) -> DashResult<()> { comment.id, why)); } - ok_or!(evaluate_nags(), why => - error!("Unable to evaluate outstanding proposals: {:?}", why)); + evaluate_nags(); Ok(()) } @@ -139,31 +107,7 @@ fn update_proposal_review_status(proposal_id: i32) -> DashResult<()> { .first(conn)?; // parse the status comment and mark any new reviews as reviewed - let reviewed = comment - .body - .lines() - .filter_map(|line| if line.starts_with("* [") { - let l = line.trim_left_matches("* ["); - let reviewed = l.starts_with('x'); - let remaining = l.trim_left_matches("x] @").trim_left_matches(" ] @"); - - if let Some(username) = remaining.split_whitespace().next() { - trace!("reviewer parsed as reviewed? {} (line: \"{}\")", - reviewed, - l); - - if reviewed { Some(username) } else { None } - } else { - warn!("An empty usename showed up in comment {} for proposal {}", - comment.id, - proposal.id); - None - } - } else { - None - }); - - for username in reviewed { + for username in parse_ticky_boxes("proposal", proposal.id, &comment) { let user: GitHubUser = githubuser::table .filter(githubuser::login.eq(username)) .first(conn)?; @@ -185,7 +129,156 @@ fn update_proposal_review_status(proposal_id: i32) -> DashResult<()> { Ok(()) } -fn evaluate_nags() -> DashResult<()> { +/// Given a poll, parse out each "responded" status, in the poll's ticky boxes, +// for each user, then update the responded status in the database. +fn update_poll_response_status(poll_id: i32) -> DashResult<()> { + let conn = &*DB_POOL.get()?; + // this is an updated comment from the bot itself + + let survey: Poll = poll::table.find(poll_id).first(conn)?; + + // don't update any statuses if the poll is closed + if survey.poll_closed { + return Ok(()); + } + + let comment: IssueComment = issuecomment::table + .find(survey.fk_bot_tracking_comment) + .first(conn)?; + + // parse the status comment and mark any new responses as responded + for respondent in parse_ticky_boxes("poll", survey.id, &comment) { + let user = githubuser::table + .filter(githubuser::login.eq(respondent)) + .first(conn); + let user: GitHubUser = ok_or_continue!(user, why => + error!("Can't find respondent {} in the database. \ + The poll comment has probably been manually edited. {:?}", + respondent, why) + ); + + { + use domain::schema::poll_response_request::dsl::*; + let mut review_request: PollResponseRequest = poll_response_request + .filter(fk_poll.eq(survey.id)) + .filter(fk_respondent.eq(user.id)) + .first(conn)?; + + review_request.responded = true; + diesel::update(poll_response_request.find(review_request.id)) + .set(&review_request) + .execute(conn)?; + } + } + + Ok(()) +} + +fn parse_ticky_boxes<'a>(what: &'a str, id: i32, comment: &'a IssueComment) + -> impl Iterator +{ + comment.body.lines().filter_map(move |line| if line.starts_with("* [") { + let l = line.trim_left_matches("* ["); + let reviewed = l.starts_with('x'); + let remaining = l.trim_left_matches("x] @").trim_left_matches(" ] @"); + + if let Some(username) = remaining.split_whitespace().next() { + trace!("reviewer parsed as reviewed? {} (line: \"{}\")", + reviewed, l); + + if reviewed { Some(username) } else { None } + } else { + warn!("An empty usename showed up in comment {} for {} {}", + comment.id, what, id); + None + } + } else { + None + }) +} + +fn evaluate_nags() { + ok_or!(evaluate_pendings(), why => + error!("Unable to evaluate outstanding proposals: {:?}", why)); + + ok_or!(evaluate_ffcps(), why => + error!("Unable to evaluate outstanding ffcps: {:?}", why)); + + ok_or!(evaluate_polls(), why => + error!("Unable to evaluate outstanding polls: {:?}", why)); +} + +fn evaluate_polls() -> DashResult<()> { + use domain::schema::poll::dsl::*; + use domain::schema::issuecomment::dsl::*; + use domain::schema::issuecomment::dsl::id as issuecomment_id; + let conn = &*DB_POOL.get()?; + + // first process all "pending" polls (unresponded) + let pending = poll.filter(poll_closed.eq(false)).load::(conn); + let pending = ok_or!(pending, why => { + error!("Unable to retrieve list of pending polls: {:?}", why); + throw!(why) + }); + + for mut survey in pending { + let initiator = githubuser::table.find(survey.fk_initiator) + .first::(conn); + let initiator = ok_or_continue!(initiator, why => + error!("Unable to retrieve poll initiator for poll id {}: {:?}", + survey.id, why)); + + let issue = issue::table.find(survey.fk_issue).first::(conn); + let issue = ok_or_continue!(issue, why => + error!("Unable to retrieve issue for poll {}: {:?}", + survey.id, why)); + + // check to see if any checkboxes were modified before we end up replacing the comment + ok_or_continue!(update_poll_response_status(survey.id), why => + error!("Unable to update response status for poll {}: {:?}", + survey.id, why)); + + // get associated responses + let responses = ok_or_continue!(list_poll_response_requests(survey.id), why => + error!("Unable to retrieve response requests for survey {}: {:?}", + survey.id, why)); + + // If everyone has answered the poll, close it: + if responses.iter().all(|(_, response)| response.responded) { + survey.poll_closed = true; + let update = diesel::update(poll.find(survey.id)) + .set(&survey).execute(conn); + ok_or_continue!(update, why => + error!("Unable to close poll {}: {:?}", survey.id, why)); + } + + // update existing status comment with responses & concerns + let status_comment = RfcBotComment::new(&issue, CommentType::QuestionAsked { + initiator: &initiator, + respondents: &responses, + question: &survey.poll_question, + teams: survey.poll_teams.split(",").collect(), + }); + + let previous_comment: IssueComment = issuecomment + .filter(issuecomment_id.eq(survey.fk_bot_tracking_comment)) + .first(conn)?; + + if previous_comment.body != status_comment.body { + // if the comment body in the database equals the new one we generated, then no change + // is needed from github (this assumes our DB accurately reflects GH's, which should + // be true in most cases by the time this is called) + let post = status_comment.post(Some(survey.fk_bot_tracking_comment)); + ok_or_continue!(post, why => + error!("Unable to update status comment for poll {}: {:?}", + survey.id, why)); + } + } + + Ok(()) +} + +fn evaluate_pendings() -> DashResult<()> { use diesel::prelude::*; use domain::schema::fcp_proposal::dsl::*; use domain::schema::issuecomment::dsl::*; @@ -307,6 +400,14 @@ fn evaluate_nags() -> DashResult<()> { } } + Ok(()) +} + +fn evaluate_ffcps() -> DashResult<()> { + use diesel::prelude::*; + use domain::schema::fcp_proposal::dsl::*; + let conn = &*DB_POOL.get()?; + // look for any FCP proposals that entered FCP a week or more ago but aren't marked as closed let one_business_week_ago = Utc::now().naive_utc() - Duration::days(10); let ffcps = fcp_proposal.filter(fcp_start.le(one_business_week_ago)) @@ -429,6 +530,32 @@ fn list_review_requests(proposal_id: i32) -> DashResult DashResult> +{ + use domain::schema::{poll_response_request, githubuser}; + + let conn = &*DB_POOL.get()?; + + let reviews = poll_response_request::table + .filter(poll_response_request::fk_poll.eq(poll_id)) + .load::(conn)?; + + let mut w_reviewers = Vec::with_capacity(reviews.len()); + + for review in reviews { + let initiator = githubuser::table + .filter(githubuser::id.eq(review.fk_respondent)) + .first::(conn)?; + + w_reviewers.push((initiator, review)); + } + + w_reviewers.sort_by(|a, b| a.0.login.cmp(&b.0.login)); + + Ok(w_reviewers) +} + fn list_concerns_with_authors(proposal_id: i32) -> DashResult> { use domain::schema::{fcp_concern, githubuser}; @@ -477,31 +604,42 @@ fn resolve_applicable_feedback_requests(author: &GitHubUser, Ok(()) } -/// Check if an issue comment is written by a member of one of the subteams labelled on the issue. -fn subteam_members(issue: &Issue) -> DashResult> { +fn resolve_logins_to_users(member_logins: &Vec<&str>) -> DashResult> { use diesel::pg::expression::dsl::any; use domain::schema::githubuser; - let conn = &*DB_POOL.get()?; - // retrieve all of the teams tagged on this issue - // cannot WAIT for by-ref/by-val inference - let member_logins = SETUP.teams() - .filter(|&(ref label, _)| issue.labels.contains(&label.0)) - .flat_map(|(_, team)| team.member_logins()) - .collect::>() - .into_iter() // diesel won't work with btreeset, and dedup has weird lifetime errors - .collect::>(); - // resolve each member into an actual user let users = githubuser::table - .filter(githubuser::login.eq(any(&member_logins))) + .filter(githubuser::login.eq(any(member_logins))) .order(githubuser::login) .load::(conn)?; Ok(users) } +/// Check if an issue comment is written by a member of one of the subteams +/// satisfying the given predicate. +fn specific_subteam_members(included: F) -> DashResult> +where + F: Fn(&String) -> bool +{ + resolve_logins_to_users(&SETUP.teams() + .filter(|&(label, _)| included(&label.0)) + .flat_map(|(_, team)| team.member_logins()) + .collect::>() + .into_iter() // diesel won't work with btreeset, and dedup has weird lifetime errors + .collect::>() + ) +} + +/// Check if an issue comment is written by a member of one of the subteams +/// labelled on the issue. +fn subteam_members(issue: &Issue) -> DashResult> { + // retrieve all of the teams tagged on this issue + specific_subteam_members(|label| issue.labels.contains(label)) +} + fn cancel_fcp(author: &GitHubUser, issue: &Issue, existing: &FcpProposal) -> DashResult<()> { use domain::schema::fcp_proposal::dsl::*; @@ -525,138 +663,36 @@ fn cancel_fcp(author: &GitHubUser, issue: &Issue, existing: &FcpProposal) -> Das Ok(()) } -#[derive(Debug, Eq, PartialEq)] -pub enum RfcBotCommand<'a> { - FcpPropose(FcpDisposition), - FcpCancel, - Reviewed, - NewConcern(&'a str), - ResolveConcern(&'a str), - FeedbackRequest(&'a str), -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum FcpDisposition { - Merge, - Close, - Postpone, +fn existing_proposal(issue: &Issue) -> DashResult> { + use domain::schema::fcp_proposal::dsl::*; + let conn = &*DB_POOL.get()?; + Ok(fcp_proposal + .filter(fk_issue.eq(issue.id)) + .first::(conn) + .optional()?) } -const FCP_REPR_MERGE: &'static str = "merge"; -const FCP_REPR_CLOSE: &'static str = "close"; -const FCP_REPR_POSTPONE: &'static str = "postpone"; - -impl FcpDisposition { - pub fn repr(self) -> &'static str { - match self { - FcpDisposition::Merge => FCP_REPR_MERGE, - FcpDisposition::Close => FCP_REPR_CLOSE, - FcpDisposition::Postpone => FCP_REPR_POSTPONE, - } - } - - pub fn from_str(string: &str) -> DashResult { - Ok(match string { - FCP_REPR_MERGE => FcpDisposition::Merge, - FCP_REPR_CLOSE => FcpDisposition::Close, - FCP_REPR_POSTPONE => FcpDisposition::Postpone, - _ => throw!(DashError::Misc(None)), - }) - } +fn post_insert_comment(issue: &Issue, comment: CommentType) -> DashResult { + use domain::schema::issuecomment; + let conn = &*DB_POOL.get()?; - pub fn label(self) -> Label { - match self { - FcpDisposition::Merge => Label::DispositionMerge, - FcpDisposition::Close => Label::DispositionClose, - FcpDisposition::Postpone => Label::DispositionPostpone, - } + let comment = RfcBotComment::new(issue, comment); + let comment = comment.post(None)?; + info!("Posted base comment to github, no reviewers listed yet"); + + // at this point our new comment doesn't yet exist in the database, so + // we need to insert it + let comment = comment.with_repo(&issue.repository)?; + if let Err(why) = + diesel::insert(&comment) + .into(issuecomment::table) + .execute(conn) + { + warn!("issue inserting new record, maybe received webhook for it: {:?}", + why); } -} - -/// Parses the text of a subcommand. -fn parse_command_text<'a>(command: &'a str, subcommand: &'a str) -> &'a str { - let name_start = command.find(subcommand).unwrap() + subcommand.len(); - command[name_start..].trim() -} - -/// Parses all subcommands under the fcp command. -/// If `fcp_context` is set to false, `@rfcbot ` -/// was passed and not `@rfcbot fcp `. -/// -/// @rfcbot accepts roughly the following grammar: -/// -/// merge ::= "merge" | "merged" | "merging" | "merges" ; -/// close ::= "close" | "closed" | "closing" | "closes" ; -/// postpone ::= "postpone" | "postponed" | "postponing" | "postpones" ; -/// cancel ::= "cancel | "canceled" | "canceling" | "cancels" ; -/// review ::= "reviewed" | "review" | "reviewing" | "reviews" ; -/// concern ::= "concern" | "concerned" | "concerning" | "concerns" ; -/// resolve ::= "resolve" | "resolved" | "resolving" | "resolves" ; -/// -/// line_remainder ::= .+$ ; -/// ws_separated ::= ... ; -/// -/// subcommand ::= merge | close | postpone | cancel | review -/// | concern line_remainder -/// | resolve line_remainder -/// ; -/// -/// invocation ::= "fcp" subcommand -/// | "pr" subcommand -/// | "f?" ws_separated -/// | subcommand -/// ; -/// -/// grammar ::= "@rfcbot" ":"? invocation ; -fn parse_fcp_subcommand<'a>( - command: &'a str, - subcommand: &'a str, - fcp_context: bool -) -> DashResult> { - Ok(match subcommand { - // Parse a FCP merge command: - "merge" | "merged" | "merging" | "merges" => - RfcBotCommand::FcpPropose(FcpDisposition::Merge), - - // Parse a FCP close command: - "close" | "closed" | "closing" | "closes" => - RfcBotCommand::FcpPropose(FcpDisposition::Close), - - // Parse a FCP postpone command: - "postpone" | "postponed" | "postponing" | "postpones" => - RfcBotCommand::FcpPropose(FcpDisposition::Postpone), - - // Parse a FCP cancel command: - "cancel" | "canceled" | "canceling" | "cancels" => - RfcBotCommand::FcpCancel, - - // Parse a FCP reviewed command: - "reviewed" | "review" | "reviewing" | "reviews" => - RfcBotCommand::Reviewed, - - // Parse a FCP concern command: - "concern" | "concerned" | "concerning" | "concerns" => { - debug!("Parsed command as NewConcern"); - let what = parse_command_text(command, subcommand); - RfcBotCommand::NewConcern(what) - }, - - // Parse a FCP resolve command: - "resolve" | "resolved" | "resolving" | "resolves" => { - debug!("Parsed command as ResolveConcern"); - let what = parse_command_text(command, subcommand); - RfcBotCommand::ResolveConcern(what) - }, - _ => { - throw!(DashError::Misc(if fcp_context { - error!("unrecognized subcommand for fcp: {}", subcommand); - Some(format!("found bad subcommand: {}", subcommand)) - } else { - None - })) - } - }) + Ok(comment) } impl<'a> RfcBotCommand<'a> { @@ -664,286 +700,320 @@ impl<'a> RfcBotCommand<'a> { author: &GitHubUser, issue: &Issue, comment: &IssueComment, - issue_subteam_members: &[GitHubUser]) + team_members: &[GitHubUser]) -> DashResult<()> { + use self::RfcBotCommand::*; + match self { + StartPoll { teams, question } => + process_poll(author, issue, comment, question, teams), + FcpPropose(disp) => + process_fcp_propose(author, issue, comment, team_members, disp), + FcpCancel => process_fcp_cancel(author, issue), + Reviewed => process_reviewed(author, issue), + NewConcern(concern_name) => + process_new_concern(author, issue, comment, concern_name), + ResolveConcern(concern_name) => + process_resolve_concern(author, issue, comment, concern_name), + FeedbackRequest(username) => + process_feedback_request(author, issue, username), + } + } +} - let conn = &*DB_POOL.get()?; +fn process_poll + (author: &GitHubUser, issue: &Issue, comment: &IssueComment, + question: &str, teams: BTreeSet<&str>) + -> DashResult<()> +{ + use domain::schema::poll::dsl::*; + use domain::schema::poll_response_request; + let conn = &*DB_POOL.get()?; - // check for existing FCP - let existing_proposal = { - use domain::schema::fcp_proposal::dsl::*; + let teams = if teams.is_empty() { + SETUP.teams() + .filter(|&(label, _)| issue.labels.contains(&label.0)) + .map(|(label, _)| &*label.0) + .collect::>() + } else { + teams + }; + let members = specific_subteam_members(|l| teams.contains(&**l))?; + + info!("adding a new poll to issue."); + + // leave github comment stating that question is asked, ping respondents + let gh_comment = post_insert_comment(issue, CommentType::QuestionAsked { + initiator: author, + teams: teams.clone(), + question, + respondents: &[], + })?; + + let teams_str = teams.iter().cloned().intersperse(",").collect::(); + let new_poll = NewPoll { + fk_issue: issue.id, + fk_initiator: author.id, + fk_initiating_comment: comment.id, + fk_bot_tracking_comment: gh_comment.id, + poll_question: question, + poll_created_at: Utc::now().naive_utc(), + poll_closed: false, + poll_teams: &*teams_str, + }; + let new_poll = diesel::insert(&new_poll).into(poll).get_result::(conn)?; + + debug!("poll inserted into the database"); + + // generate response requests for all relevant subteam members + + let response_requests = members + .iter() + .map(|member| NewPollResponseRequest { + fk_poll: new_poll.id, + fk_respondent: member.id, + // let's assume the initiator has answered it + responded: member.id == author.id, + }) + .collect::>(); - fcp_proposal - .filter(fk_issue.eq(issue.id)) - .first::(conn) - .optional()? - }; + diesel::insert(&response_requests) + .into(poll_response_request::table) + .execute(conn)?; - match self { - RfcBotCommand::FcpPropose(disp) => { - debug!("processing fcp proposal: {:?}", disp); - use domain::schema::fcp_proposal::dsl::*; - use domain::schema::{fcp_review_request, issuecomment}; - - if existing_proposal.is_none() { - // if not exists, create new FCP proposal - info!("proposal is a new FCP, creating..."); - - // leave github comment stating that FCP is proposed, ping reviewers - let gh_comment = - RfcBotComment::new(issue, CommentType::FcpProposed(author, disp, &[], &[])); - - let gh_comment = gh_comment.post(None)?; - info!("Posted base comment to github, no reviewers listed yet"); - - // at this point our new comment doesn't yet exist in the database, so - // we need to insert it - let gh_comment = gh_comment.with_repo(&issue.repository)?; - if let Err(why) = - diesel::insert(&gh_comment) - .into(issuecomment::table) - .execute(conn) - { - warn!("issue inserting new record, maybe received webhook for it: {:?}", - why); - } + // they're in the database, but now we need them paired with githubuser - let proposal = NewFcpProposal { - fk_issue: issue.id, - fk_initiator: author.id, - fk_initiating_comment: comment.id, - disposition: disp.repr(), - fk_bot_tracking_comment: gh_comment.id, - fcp_start: None, - fcp_closed: false, - }; + let response_requests = list_poll_response_requests(new_poll.id)?; - let proposal = diesel::insert(&proposal) - .into(fcp_proposal) - .get_result::(conn)?; + debug!("poll response requests inserted into the database"); - debug!("proposal inserted into the database"); + // we have all of the review requests, generate a new comment and post it + + let new_gh_comment = RfcBotComment::new(issue, CommentType::QuestionAsked { + initiator: author, + teams, + question, + respondents: &*response_requests, + }); + new_gh_comment.post(Some(gh_comment.id))?; - // generate review requests for all relevant subteam members + debug!("github comment updated with poll respondents"); - let review_requests = issue_subteam_members - .iter() - .map(|member| { - // let's assume the initiator has reviewed it - NewFcpReviewRequest { - fk_proposal: proposal.id, - fk_reviewer: member.id, - reviewed: member.id == author.id, - } - }) - .collect::>(); + Ok(()) +} - diesel::insert(&review_requests) - .into(fcp_review_request::table) - .execute(conn)?; +fn process_fcp_propose + (author: &GitHubUser, issue: &Issue, comment: &IssueComment, + team_members: &[GitHubUser], disp: FcpDisposition) + -> DashResult<()> +{ + debug!("processing fcp proposal: {:?}", disp); + use domain::schema::fcp_proposal::dsl::*; + use domain::schema::fcp_review_request; - // they're in the database, but now we need them paired with githubuser + if existing_proposal(issue)?.is_none() { + let conn = &*DB_POOL.get()?; + // if not exists, create new FCP proposal + info!("proposal is a new FCP, creating..."); + + // leave github comment stating that FCP is proposed, ping reviewers + let gh_comment = post_insert_comment(issue, + CommentType::FcpProposed(author, disp, &[], &[]))?; + + let proposal = NewFcpProposal { + fk_issue: issue.id, + fk_initiator: author.id, + fk_initiating_comment: comment.id, + fk_bot_tracking_comment: gh_comment.id, + disposition: disp.repr(), + fcp_start: None, + fcp_closed: false, + }; + let proposal = diesel::insert(&proposal) + .into(fcp_proposal) + .get_result::(conn)?; - let review_requests = list_review_requests(proposal.id)?; + debug!("proposal inserted into the database"); - debug!("review requests inserted into the database"); + // generate review requests for all relevant subteam members - // we have all of the review requests, generate a new comment and post it + let review_requests = team_members + .iter() + .map(|member| NewFcpReviewRequest { + fk_proposal: proposal.id, + fk_reviewer: member.id, + // let's assume the initiator has reviewed it + reviewed: member.id == author.id, + }) + .collect::>(); + + diesel::insert(&review_requests) + .into(fcp_review_request::table) + .execute(conn)?; - let new_gh_comment = - RfcBotComment::new(issue, - CommentType::FcpProposed( - author, disp, &review_requests, &[])); + // they're in the database, but now we need them paired with githubuser - new_gh_comment.post(Some(gh_comment.id))?; + let review_requests = list_review_requests(proposal.id)?; - debug!("github comment updated with reviewers"); - } - } - RfcBotCommand::FcpCancel => { - if let Some(existing) = existing_proposal { - cancel_fcp(author, issue, &existing)?; - } - } - RfcBotCommand::Reviewed => { - // set a reviewed entry for the comment author on this issue + debug!("review requests inserted into the database"); - use domain::schema::fcp_review_request::dsl::*; + // we have all of the review requests, generate a new comment and post it - if let Some(proposal) = existing_proposal { + let new_gh_comment = RfcBotComment::new(issue, + CommentType::FcpProposed(author, disp, &review_requests, &[])); + new_gh_comment.post(Some(gh_comment.id))?; + debug!("github comment updated with reviewers"); + } - let review_request = fcp_review_request - .filter(fk_proposal.eq(proposal.id)) - .filter(fk_reviewer.eq(author.id)) - .first::(conn) - .optional()?; + Ok(()) +} - if let Some(mut review_request) = review_request { - // store an FK to the comment marking for review (not null fk here means - // reviewed) - review_request.reviewed = true; +fn process_fcp_cancel(author: &GitHubUser, issue: &Issue) -> DashResult<()> { + if let Some(existing) = existing_proposal(issue)? { + cancel_fcp(author, issue, &existing)?; + } + Ok(()) +} - diesel::update(fcp_review_request.find(review_request.id)) - .set(&review_request) - .execute(conn)?; - } +fn process_reviewed(author: &GitHubUser, issue: &Issue) -> DashResult<()> { + // set a reviewed entry for the comment author on this issue + if let Some(proposal) = existing_proposal(issue)? { + use domain::schema::fcp_review_request::dsl::*; + let conn = &*DB_POOL.get()?; - } - } - RfcBotCommand::NewConcern(concern_name) => { - - if let Some(mut proposal) = existing_proposal { - // check for existing concern - use domain::schema::fcp_concern::dsl::*; - use domain::schema::fcp_proposal::dsl::*; - - let existing_concern = fcp_concern - .filter(fk_proposal.eq(proposal.id)) - .filter(name.eq(concern_name)) - .first::(conn) - .optional()?; - - if existing_concern.is_none() { - // if not exists, create new concern with this author as creator - - let new_concern = NewFcpConcern { - fk_proposal: proposal.id, - fk_initiator: author.id, - fk_resolved_comment: None, - name: concern_name, - fk_initiating_comment: comment.id, - }; - - diesel::insert(&new_concern) - .into(fcp_concern) - .execute(conn)?; - - // Take us out of FCP and back into PFCP if need be: - if proposal.fcp_start.is_some() { - // Update DB: FCP is not started anymore. - proposal.fcp_start = None; - let update = - diesel::update(fcp_proposal.find(proposal.id)) - .set(&proposal) - .execute(conn); - ok_or!(update, why => { - error!("Unable to mark FCP {} as unstarted: {:?}", proposal.id, why); - return Ok(()); - }); - - // Update labels: - let _ = issue.add_label(Label::PFCP); - issue.remove_label(Label::FCP); - } - } - } - } - RfcBotCommand::ResolveConcern(concern_name) => { + let review_request = fcp_review_request + .filter(fk_proposal.eq(proposal.id)) + .filter(fk_reviewer.eq(author.id)) + .first::(conn) + .optional()?; - debug!("Command is to resolve a concern ({}).", concern_name); + if let Some(mut review_request) = review_request { + // store an FK to the comment marking for review (not null fk here means + // reviewed) + review_request.reviewed = true; + diesel::update(fcp_review_request.find(review_request.id)) + .set(&review_request) + .execute(conn)?; + } + } - if let Some(proposal) = existing_proposal { - // check for existing concern - use domain::schema::fcp_concern::dsl::*; + Ok(()) +} - let existing_concern = fcp_concern - .filter(fk_proposal.eq(proposal.id)) - .filter(fk_initiator.eq(author.id)) - .filter(name.eq(concern_name)) - .first::(conn) - .optional()?; +fn process_new_concern + (author: &GitHubUser, issue: &Issue, comment: &IssueComment, + concern_name: &str) + -> DashResult<()> +{ + if let Some(mut proposal) = existing_proposal(issue)? { + // check for existing concern + use domain::schema::fcp_concern::dsl::*; + use domain::schema::fcp_proposal::dsl::*; + let conn = &*DB_POOL.get()?; - if let Some(mut concern) = existing_concern { + let existing_concern = fcp_concern + .filter(fk_proposal.eq(proposal.id)) + .filter(name.eq(concern_name)) + .first::(conn) + .optional()?; + + if existing_concern.is_none() { + // if not exists, create new concern with this author as creator + let new_concern = NewFcpConcern { + fk_proposal: proposal.id, + fk_initiator: author.id, + fk_resolved_comment: None, + name: concern_name, + fk_initiating_comment: comment.id, + }; + diesel::insert(&new_concern).into(fcp_concern).execute(conn)?; + + // Take us out of FCP and back into PFCP if need be: + if proposal.fcp_start.is_some() { + // Update DB: FCP is not started anymore. + proposal.fcp_start = None; + let update = diesel::update(fcp_proposal.find(proposal.id)) + .set(&proposal) + .execute(conn); + ok_or!(update, why => { + error!("Unable to mark FCP {} as unstarted: {:?}", proposal.id, why); + return Ok(()); + }); + + // Update labels: + let _ = issue.add_label(Label::PFCP); + issue.remove_label(Label::FCP); + } + } + } - debug!("Found a matching concern ({})", concern_name); + Ok(()) +} - // mark concern as resolved by adding resolved_comment - concern.fk_resolved_comment = Some(comment.id); +fn process_resolve_concern + (author: &GitHubUser, issue: &Issue, comment: &IssueComment, concern_name: &str) + -> DashResult<()> +{ + debug!("Command is to resolve a concern ({}).", concern_name); - diesel::update(fcp_concern.find(concern.id)) - .set(&concern) - .execute(conn)?; - } + if let Some(proposal) = existing_proposal(issue)? { + // check for existing concern + use domain::schema::fcp_concern::dsl::*; + let conn = &*DB_POOL.get()?; - } - } - RfcBotCommand::FeedbackRequest(username) => { - - use domain::schema::githubuser; - use domain::schema::rfc_feedback_request::dsl::*; - - // we'll just assume that this user exists...it's very unlikely that someone - // will request feedback from a user who's *never* commented or committed - // on/to a rust-lang* repo - let requested_user = githubuser::table - .filter(githubuser::login.eq(username)) - .first::(conn)?; - - // check for existing feedback request - let existing_request = rfc_feedback_request - .filter(fk_requested.eq(requested_user.id)) - .filter(fk_issue.eq(issue.id)) - .first::(conn) - .optional()?; - - if existing_request.is_none() { - // create feedback request - - let new_request = NewFeedbackRequest { - fk_initiator: author.id, - fk_requested: requested_user.id, - fk_issue: issue.id, - fk_feedback_comment: None, - }; - - diesel::insert(&new_request) - .into(rfc_feedback_request) - .execute(conn)?; - } - } + let existing_concern = fcp_concern + .filter(fk_proposal.eq(proposal.id)) + .filter(fk_initiator.eq(author.id)) + .filter(name.eq(concern_name)) + .first::(conn) + .optional()?; + + if let Some(mut concern) = existing_concern { + // mark concern as resolved by adding resolved_comment + debug!("Found a matching concern ({})", concern_name); + concern.fk_resolved_comment = Some(comment.id); + diesel::update(fcp_concern.find(concern.id)) + .set(&concern) + .execute(conn)?; } - Ok(()) } - pub fn from_str_all(command: &'a str) -> impl Iterator> { - // Get the tokens for each command line (starts with a bot mention) - command.lines() - .filter(|&l| l.starts_with(RFC_BOT_MENTION)) - .map(Self::from_invocation_line) - .filter_map(Result::ok) - } + Ok(()) +} - fn from_invocation_line(command: &'a str) -> DashResult> { - let mut tokens = command.trim_left_matches(RFC_BOT_MENTION) - .trim_left_matches(':') - .trim() - .split_whitespace(); - let invocation = tokens.next().ok_or(DashError::Misc(None))?; - match invocation { - "fcp" | "pr" => { - let subcommand = tokens.next().ok_or(DashError::Misc(None))?; +fn process_feedback_request(author: &GitHubUser, issue: &Issue, username: &str) + -> DashResult<()> +{ + use domain::schema::githubuser; + use domain::schema::rfc_feedback_request::dsl::*; + let conn = &*DB_POOL.get()?; - debug!("Parsed command as new FCP proposal"); + // we'll just assume that this user exists...it's very unlikely that someone + // will request feedback from a user who's *never* commented or committed + // on/to a rust-lang* repo + let requested_user = githubuser::table + .filter(githubuser::login.eq(username)) + .first::(conn)?; - parse_fcp_subcommand(command, subcommand, true) - } - "f?" => { - let user = - tokens - .next() - .ok_or_else(|| DashError::Misc(Some("no user specified".to_string())))?; - - if user.is_empty() { - throw!(DashError::Misc(Some("no user specified".to_string()))); - } + // check for existing feedback request + let existing_request = rfc_feedback_request + .filter(fk_requested.eq(requested_user.id)) + .filter(fk_issue.eq(issue.id)) + .first::(conn) + .optional()?; - Ok(RfcBotCommand::FeedbackRequest(&user[1..])) - } - _ => parse_fcp_subcommand(command, invocation, false), - } + if existing_request.is_none() { + // create feedback request + let new_request = NewFeedbackRequest { + fk_initiator: author.id, + fk_requested: requested_user.id, + fk_issue: issue.id, + fk_feedback_comment: None, + }; + diesel::insert(&new_request).into(rfc_feedback_request).execute(conn)?; } + Ok(()) } struct RfcBotComment<'a> { @@ -952,6 +1022,7 @@ struct RfcBotComment<'a> { comment_type: CommentType<'a>, } +#[derive(Clone)] enum CommentType<'a> { FcpProposed(&'a GitHubUser, FcpDisposition, @@ -969,6 +1040,12 @@ enum CommentType<'a> { added_label: bool, disposition: FcpDisposition }, + QuestionAsked { + initiator: &'a GitHubUser, + respondents: &'a [(GitHubUser, PollResponseRequest)], + question: &'a str, + teams: BTreeSet<&'a str>, + }, } impl<'a> RfcBotComment<'a> { @@ -992,8 +1069,20 @@ impl<'a> RfcBotComment<'a> { } fn format(issue: &Issue, comment_type: &CommentType) -> String { - match *comment_type { + CommentType::QuestionAsked { initiator, respondents, question, ref teams } => { + let mut msg = String::from("Team member @"); + msg.push_str(&initiator.login); + msg.push_str(" has asked teams: "); + msg.extend(teams.iter().cloned().intersperse(", ")); + msg.push_str(", for consensus on: \n > "); + msg.push_str(question); + msg.push_str("\n\n"); + format_ticky_boxes(&mut msg, + respondents.iter().map(|(m, rr)| (m, rr.responded))); + msg + } + CommentType::FcpProposed(initiator, disposition, reviewers, concerns) => { let mut msg = String::from("Team member @"); msg.push_str(&initiator.login); @@ -1002,17 +1091,8 @@ impl<'a> RfcBotComment<'a> { msg.push_str(" this. The next step is review by the rest of the tagged "); msg.push_str("teams:\n\n"); - for &(ref member, ref review_request) in reviewers { - - if review_request.reviewed { - msg.push_str("* [x] @"); - } else { - msg.push_str("* [ ] @"); - } - - msg.push_str(&member.login); - msg.push('\n'); - } + format_ticky_boxes(&mut msg, + reviewers.iter().map(|(m, rr)| (m, rr.reviewed))); if concerns.is_empty() { msg.push_str("\nNo concerns currently listed.\n"); @@ -1021,7 +1101,6 @@ impl<'a> RfcBotComment<'a> { } for &(_, ref concern) in concerns { - if let Some(resolved_comment_id) = concern.fk_resolved_comment { msg.push_str("* ~~"); msg.push_str(&concern.name); @@ -1147,149 +1226,11 @@ impl<'a> RfcBotComment<'a> { } } -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn multiple_commands() { -let text = r#" -someothertext -@rfcbot: resolved CONCERN_NAME -somemoretext - -somemoretext - -@rfcbot: fcp cancel -foobar -@rfcbot concern foobar -"#; - - let cmd = RfcBotCommand::from_str_all(text).collect::>(); - assert_eq!(cmd, vec![ - RfcBotCommand::ResolveConcern("CONCERN_NAME"), - RfcBotCommand::FcpCancel, - RfcBotCommand::NewConcern("foobar"), - ]); - } - - fn ensure_take_singleton(mut iter: I) -> I::Item { - let singleton = iter.next().unwrap(); - assert!(iter.next().is_none()); - singleton - } - - macro_rules! justification { - () => { "\n\nSome justification here." }; - } - - macro_rules! some_text { - ($important: expr) => { - concat!(" ", $important, " -someothertext -somemoretext - -somemoretext") - }; - } - - macro_rules! test_from_str { - ($test: ident, [$($cmd: expr),+], $message: expr, $expected: expr) => { - test_from_str!($test, [$(concat!($cmd, $message)),+], $expected); - }; - - ($test: ident, [$($cmd: expr),+], $expected: expr) => { - #[test] - fn $test() { - let expected = $expected; - - $({ - let body = concat!("@rfcbot: ", $cmd); - let body_no_colon = concat!("@rfcbot ", $cmd); - - let with_colon = - ensure_take_singleton(RfcBotCommand::from_str_all(body)); - - let without_colon = - ensure_take_singleton(RfcBotCommand::from_str_all(body_no_colon)); - - assert_eq!(with_colon, without_colon); - assert_eq!(with_colon, expected); - })+ - } - }; - } - - test_from_str!(success_fcp_reviewed, - ["reviewed", "review", "reviewing", "reviews", - "fcp reviewed", "fcp review", "fcp reviewing", - "pr reviewed", "pr review", "pr reviewing"], - RfcBotCommand::Reviewed); - - test_from_str!(success_fcp_merge, - ["merge", "merged", "merging", "merges", - "fcp merge", "fcp merged", "fcp merging", "fcp merges", - "pr merge", "pr merged", "pr merging", "pr merges"], - justification!(), - RfcBotCommand::FcpPropose(FcpDisposition::Merge)); - - test_from_str!(success_fcp_close, - ["close", "closed", "closing", "closes", - "fcp close", "fcp closed", "fcp closing", "fcp closes", - "pr close", "pr closed", "pr closing", "pr closes"], - justification!(), - RfcBotCommand::FcpPropose(FcpDisposition::Close)); - - test_from_str!(success_fcp_postpone, - ["postpone", "postponed", "postponing", "postpones", - "fcp postpone", "fcp postponed", "fcp postponing", "fcp postpones", - "pr postpone", "pr postponed", "pr postponing", "pr postpones"], - justification!(), - RfcBotCommand::FcpPropose(FcpDisposition::Postpone)); - - test_from_str!(success_fcp_cancel, - ["cancel", "canceled", "canceling", "cancels", - "fcp cancel", "fcp canceled", "fcp canceling", "fcp cancels", - "pr cancel", "pr canceled", "pr canceling", "pr cancels"], - justification!(), - RfcBotCommand::FcpCancel); - - test_from_str!(success_concern, - ["concern", "concerned", "concerning", "concerns", - "fcp concern", "fcp concerned", "fcp concerning", "fcp concerns", - "pr concern", "pr concerned", "pr concerning", "pr concerns"], - some_text!("CONCERN_NAME"), - RfcBotCommand::NewConcern("CONCERN_NAME")); - - test_from_str!(success_resolve, - ["resolve", "resolved", "resolving", "resolves", - "fcp resolve", "fcp resolved", "fcp resolving", "fcp resolves", - "pr resolve", "pr resolved", "pr resolving", "pr resolves"], - some_text!("CONCERN_NAME"), - RfcBotCommand::ResolveConcern("CONCERN_NAME")); - - #[test] - fn success_resolve_mid_body() { - let body = "someothertext -@rfcbot: resolved CONCERN_NAME -somemoretext - -somemoretext"; - let body_no_colon = "someothertext -somemoretext - -@rfcbot resolved CONCERN_NAME - -somemoretext"; - - let with_colon = ensure_take_singleton(RfcBotCommand::from_str_all(body)); - let without_colon = - ensure_take_singleton(RfcBotCommand::from_str_all(body_no_colon)); - - assert_eq!(with_colon, without_colon); - assert_eq!(with_colon, RfcBotCommand::ResolveConcern("CONCERN_NAME")); +fn format_ticky_boxes<'a> + (msg: &mut String, reviewers: impl Iterator) { + for (member, reviewed) in reviewers { + msg.push_str(if reviewed { "* [x] @" } else { "* [ ] @" }); + msg.push_str(&member.login); + msg.push('\n'); } - - test_from_str!(success_feedback, ["f?"], some_text!("@bob"), - RfcBotCommand::FeedbackRequest("bob")); } diff --git a/src/main.rs b/src/main.rs index 8f823f51..b4c50b20 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,6 +34,7 @@ extern crate url; extern crate urlencoded; #[macro_use] extern crate maplit; +extern crate itertools; #[macro_use] mod macros; diff --git a/src/teams.rs b/src/teams.rs index d7f9128b..fe340133 100644 --- a/src/teams.rs +++ b/src/teams.rs @@ -55,17 +55,16 @@ pub struct FcpBehavior { #[derive(Debug, Deserialize)] pub struct Team { - // FIXME(2018-05-16): - // The two following first fields are not used anymore. - // But they could still be useful. Consider what usage they could have. - - //name: String, - //ping: String, - + name: String, + ping: String, members: Vec, } impl Team { + pub fn ping(&self) -> &str { + &self.ping + } + pub fn member_logins(&self) -> impl Iterator { self.members.iter().map(|s| s.as_str()) } @@ -126,12 +125,12 @@ impl Team { //============================================================================== #[cfg(test)] -mod test { +pub mod test { use super::*; - #[test] - fn setup_parser_correct() { -let test = r#" + lazy_static! { + pub static ref TEST_SETUP: RfcbotConfig = + read_rfcbot_cfg_from(r#" [fcp_behaviors] [fcp_behaviors."rust-lang/alpha"] @@ -148,7 +147,7 @@ postpone = false [teams] -[teams.avengers] +[teams.T-avengers] name = "The Avengers" ping = "marvel/avengers" members = [ @@ -170,18 +169,22 @@ members = [ "batman", "theflash" ] -"#; - let cfg = read_rfcbot_cfg_from(test); +"#); + } + + #[test] + fn setup_parser_correct() { + let cfg = &*TEST_SETUP; // Labels are correct: assert_eq!(cfg.team_labels().map(|tl| tl.0.clone()).collect::>(), - vec!["avengers", "justice-league"]); + vec!["T-avengers", "justice-league"]); // Teams are correct: let map: BTreeMap<_, _> = cfg.teams().map(|(k, v)| (k.0.clone(), v.clone())).collect(); - let avengers = map.get("avengers").unwrap(); + let avengers = map.get("T-avengers").unwrap(); //assert_eq!(avengers.name, "The Avengers"); //assert_eq!(avengers.ping, "marvel/avengers"); assert_eq!(avengers.member_logins().collect::>(),