diff --git a/.sqlx/query-323b623291bbf288bc65c6ce430a887809bf4c7d429ac242b8a3f21c91415499.json b/.sqlx/query-323b623291bbf288bc65c6ce430a887809bf4c7d429ac242b8a3f21c91415499.json new file mode 100644 index 00000000..69549429 --- /dev/null +++ b/.sqlx/query-323b623291bbf288bc65c6ce430a887809bf4c7d429ac242b8a3f21c91415499.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE pull_request\n SET approved_by = NULL,\n approved_sha = NULL,\n auto_build_id = NULL\n WHERE\n id = $1 AND\n (approved_by IS NULL OR approved_sha != $2)\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int4", + "Text" + ] + }, + "nullable": [] + }, + "hash": "323b623291bbf288bc65c6ce430a887809bf4c7d429ac242b8a3f21c91415499" +} diff --git a/src/bors/command/mod.rs b/src/bors/command/mod.rs index cf24bff3..cd2cc172 100644 --- a/src/bors/command/mod.rs +++ b/src/bors/command/mod.rs @@ -174,5 +174,20 @@ pub enum BorsCommand { /// Cancel an auto build currently running on a given PR (without removing it from the queue). Cancel, /// Squash all commits of a pull request into a single commit. - Squash { commit_message: SquashCommitMessage }, + Squash { + /// Squash message for the commit + commit_message: SquashCommitMessage, + }, + SquashApprove { + /// Squash message for the commit + commit_message: SquashCommitMessage, + /// Who is approving the commit. + approver: Approver, + /// Priority of the commit. + priority: Option, + /// Rollup status of the commit. + rollup: Option, + /// Optional note attached at the end of the command. + note: Option, + }, } diff --git a/src/bors/command/parser.rs b/src/bors/command/parser.rs index 003bb69e..933effc5 100644 --- a/src/bors/command/parser.rs +++ b/src/bors/command/parser.rs @@ -23,7 +23,7 @@ pub enum CommandParseError { } /// Part of a command, either a bare string like `try` or a key value like `parent=`. -#[derive(PartialEq, Copy, Clone)] +#[derive(Debug, PartialEq, Copy, Clone)] enum CommandPart<'a> { Bare(&'a str), KeyValue { key: &'a str, value: &'a str }, @@ -267,6 +267,12 @@ fn parse_parts(input: &str) -> Result>, CommandParseError> { /// - "@bors r+ [p=] [rollup=] [note=]" /// - "@bors r= [p=] [rollup=] [note=]" fn parser_approval(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResult { + if let CommandPart::Bare("r+") = command + && !parts.is_empty() + && parts.contains(&CommandPart::Bare("squash")) + { + return parser_squash_approve(command, parts); + } let approver = match command { CommandPart::Bare("r+") => Approver::Myself, CommandPart::KeyValue { key: "r", value } => { @@ -670,6 +676,53 @@ fn parser_squash(command: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseR } } +/// Parses `@bors r+ squash` command. +/// Supports specifying a commit message via `@bors r+ squash [msg|message]="message"`. +fn parser_squash_approve(_: &CommandPart<'_>, parts: &[CommandPart<'_>]) -> ParseResult { + match parts { + &[CommandPart::Bare("squash")] => Some(Ok(BorsCommand::SquashApprove { + commit_message: SquashCommitMessage::AutoGenerate, + approver: Approver::Myself, + priority: None, + rollup: None, + note: None, + })), + &[ + CommandPart::Bare("squash"), + CommandPart::KeyValue { + key: "msg" | "message", + value, + }, + .., + ] => { + if value == "description" { + Some(Ok(BorsCommand::SquashApprove { + commit_message: SquashCommitMessage::PullRequestDescription, + approver: Approver::Myself, + priority: None, + rollup: None, + note: None, + })) + } else { + Some(Ok(BorsCommand::SquashApprove { + commit_message: SquashCommitMessage::Explicit(value.to_owned()), + approver: Approver::Myself, + priority: None, + rollup: None, + note: None, + })) + } + } + [part, ..] => Some(Err(CommandParseError::UnknownArg { + arg: part.as_key().to_owned(), + did_you_mean: "r+ squash [msg|message=\"\"|description]".to_string(), + })), + part => Some(Err(CommandParseError::MissingArgValue { + arg: format!("{:#?}", part), + })), + } +} + #[cfg(test)] mod tests { use crate::bors::command::BorsCommand; @@ -2099,6 +2152,137 @@ for the crater", "#); } + #[test] + fn parse_squash_approve() { + let cmds = parse_commands("@bors r+ squash"); + insta::assert_debug_snapshot!(cmds, @" + [ + Ok( + SquashApprove { + commit_message: AutoGenerate, + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "); + } + + #[test] + fn parse_squash_approve_msg() { + let cmds = parse_commands("@bors r+ squash msg=foo"); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Ok( + SquashApprove { + commit_message: Explicit( + "foo", + ), + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "#); + } + + #[test] + fn parse_squash_approve_message() { + let cmds = parse_commands("@bors r+ squash message=foo"); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Ok( + SquashApprove { + commit_message: Explicit( + "foo", + ), + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "#); + } + + #[test] + fn parse_squash_approve_message_quoted() { + let cmds = parse_commands(r#"@bors r+ squash message="foo bar baz""#); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Ok( + SquashApprove { + commit_message: Explicit( + "foo bar baz", + ), + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "#); + } + + #[test] + fn parse_squash_approve_msg_description() { + let cmds = parse_commands("@bors r+ squash msg=description"); + insta::assert_debug_snapshot!(cmds, @" + [ + Ok( + SquashApprove { + commit_message: PullRequestDescription, + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "); + } + + #[test] + fn parse_squash_approve_unknown_arg() { + let cmds = parse_commands("@bors r+ squash commit=foo"); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Err( + UnknownArg { + arg: "squash", + did_you_mean: "r+ squash [msg|message=\"\"|description]", + }, + ), + ] + "#); + } + + #[test] + fn parse_squash_approve_extra_args() { + let cmds = parse_commands("@bors r+ squash message=foo baz"); + insta::assert_debug_snapshot!(cmds, @r#" + [ + Ok( + SquashApprove { + commit_message: Explicit( + "foo", + ), + approver: Myself, + priority: None, + rollup: None, + note: None, + }, + ), + ] + "#); + } + #[test] fn parse_in_html_command() { let cmds = parse_commands( diff --git a/src/bors/handlers/help.rs b/src/bors/handlers/help.rs index 4460b191..04452832 100644 --- a/src/bors/handlers/help.rs +++ b/src/bors/handlers/help.rs @@ -65,6 +65,9 @@ mod tests { - `squash [msg|message=|description]`: Squash the commits of a PR into a single commit. - Optionally, you can specify a `` for the created commit. If not specified, the commit messages of all squashed commits will be combined. - If you specify `msg=description`, then the PR body will be used as the squashed commit message. + - `r+ squash [msg|message=|description]`: Squash the commits of a PR into a single commit, then approve on your behalf. + - Optionally, you can specify a `` for the created commit. If not specified, the commit messages of all squashed commits will be combined. + - If you specify `msg=description`, then the PR body will be used as the squashed commit message. - `info`: Get information about the current PR ## Repository management diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs index a7819ee9..92a73cc6 100644 --- a/src/bors/handlers/mod.rs +++ b/src/bors/handlers/mod.rs @@ -15,6 +15,7 @@ use crate::bors::handlers::refresh::{ use crate::bors::handlers::review::{ TreeCloseArguments, command_approve, command_close_tree, command_open_tree, command_unapprove, }; +use crate::bors::handlers::squash::AfterSquashCallback; use crate::bors::handlers::trybuild::{command_try_build, command_try_cancel}; use crate::bors::handlers::workflow::{ AutoBuildCancelReason, handle_workflow_completed, handle_workflow_job_completed, @@ -487,6 +488,7 @@ async fn handle_comment( rollup, note, senders.merge_queue(), + pr.github.head.sha.clone(), ) .instrument(span) .await @@ -650,9 +652,11 @@ async fn handle_comment( commit_message, ctx.parser.prefix(), senders.gitops_queue(), + None, ) .instrument(span) - .await + .await?; + Ok(()) } else { repo.client .post_comment( @@ -668,6 +672,78 @@ async fn handle_comment( Ok(()) } } + BorsCommand::SquashApprove { + commit_message, + approver, + priority, + rollup, + note, + } => { + let span = tracing::info_span!("SquashAndApprove"); + + if ctx.local_git_available() { + let ctx2 = ctx.clone(); + let repo2 = repo.clone(); + let db2 = database.clone(); + let pr_github = pr_github.clone(); + let comment_author = comment.author.clone(); + let merge_queue_tx = senders.merge_queue().clone(); + let callback: AfterSquashCallback = Box::new(move |sha: CommitSha| { + Box::pin(async move { + let pr_db = db2 + .get_pull_request(repo2.repository(), pr_github.number) + .await? + .expect("TODO"); + let pr2 = PullRequestData { + github: &pr_github, + db: &pr_db, + }; + + command_approve( + ctx2, + repo2, + db2, + pr2, + &comment_author, + &approver, + priority, + rollup, + note, + &merge_queue_tx, + sha, + ) + .await + }) + }); + squash::command_squash( + repo, + database, + pr, + &comment.author, + commit_message, + ctx.parser.prefix(), + senders.gitops_queue(), + Some(callback), + ) + .instrument(span) + .await?; + } else { + repo.client + .post_comment( + pr_number, + Comment::new( + "`@bors squash` is not enabled in this bors instance.\ + Cancelling command, to just approve use the `r+`\ + command instead." + .to_string(), + ), + &ctx.db, + ) + .instrument(span) + .await?; + } + Ok(()) + } }; if result.is_err() { return result.context("Cannot execute Bors command"); @@ -832,7 +908,9 @@ pub enum InvalidationReason { /// A new commit was pushed to the pull request. /// If it was approved, it will be unapproved. /// If it was contained in any rollups, they will be closed. - CommitShaChanged, + CommitShaChanged { sha: CommitSha }, + /// The base branch of the PR has changed. + BaseBranchChanged, /// The pull request was closed. /// If it was approved, it will be unapproved. /// If it was contained in any rollups, they will be closed. @@ -862,6 +940,20 @@ pub async fn unapprove_pr( Ok(()) } +/// Unapprove the given pull request if `new_sha` already wasn't the approved commit. +/// Returns true if the PR was actually unapproved. +pub async fn unapprove_pr_if_sha_changed( + repo_state: &RepositoryState, + db: &PgDbClient, + pr_db: &PullRequestModel, + pr_gh: &PullRequestInfo, + new_sha: &CommitSha, +) -> anyhow::Result { + let unapproved = db.unapprove_if_sha_changed(pr_db, new_sha).await?; + handle_label_trigger(repo_state, pr_gh, LabelTrigger::Unapproved).await?; + Ok(unapproved) +} + pub struct InvalidationComment { /// Start of the invalidation comment base_text: String, @@ -919,17 +1011,38 @@ pub async fn invalidate_pr( comment: Option, ) -> anyhow::Result { // Step 1: unapprove the pull request if it was approved - // This happens everytime the PR is invalidated, if it was approved before + // This happens everytime the PR is invalidated, if it was approved before with a different + // commit let pr_unapproved = if pr_db.is_approved() { - unapprove_pr(repo_state, db, pr_db, &pr_gh.clone().into()).await?; - true + // This is handling a potential race condition coming from `@bors r+ squash`. + // When we squash, we push a new commit to the PR, and then we approve it. + // GitHub will then send us a webhook about the push, which will call this function. + // If we then unapproved the PR, then we would essentially cancel the previous `r+ squash`. + // So instead, if the pushed commit SHA is the same as the one that is already approved in + // the DB, we do not unapprove it. We figure this out atomically, to avoid further race + // conditions in the DB. + match &info.reason { + InvalidationReason::CommitShaChanged { sha } => { + unapprove_pr_if_sha_changed(repo_state, db, pr_db, &pr_gh.clone().into(), sha) + .await? + } + InvalidationReason::Close + | InvalidationReason::BaseBranchChanged + | InvalidationReason::Unapproval { .. } + | InvalidationReason::RollupMemberInvalidated { .. } => { + unapprove_pr(repo_state, db, pr_db, &pr_gh.clone().into()).await?; + true + } + } } else { false }; fn get_cancel_reason(reason: &InvalidationReason) -> AutoBuildCancelReason { match reason { - InvalidationReason::CommitShaChanged => AutoBuildCancelReason::PushToPR, + InvalidationReason::CommitShaChanged { .. } | InvalidationReason::BaseBranchChanged => { + AutoBuildCancelReason::PushToPR + } InvalidationReason::Close => AutoBuildCancelReason::Close, InvalidationReason::Unapproval { .. } => AutoBuildCancelReason::Unapproval, InvalidationReason::RollupMemberInvalidated { reason, .. } => get_cancel_reason(reason), @@ -949,7 +1062,9 @@ pub async fn invalidate_pr( // Note that we don't do this on `InvalidationReason::Close` itself, because that happens after // the PR has been closed already. let pr_closed = if let InvalidationReason::RollupMemberInvalidated { reason, .. } = &info.reason - && let InvalidationReason::Close | InvalidationReason::CommitShaChanged = &**reason + && let InvalidationReason::Close + | InvalidationReason::CommitShaChanged { .. } + | InvalidationReason::BaseBranchChanged = &**reason && matches!( pr_gh.status, PullRequestStatus::Open | PullRequestStatus::Draft @@ -964,7 +1079,8 @@ pub async fn invalidate_pr( // Step 4: recursively invalidate all open rollups containing this PR let invalidate_rollups = match info.reason { - InvalidationReason::CommitShaChanged + InvalidationReason::CommitShaChanged { .. } + | InvalidationReason::BaseBranchChanged | InvalidationReason::Close | InvalidationReason::Unapproval { .. } => true, // We do not assume that rollups contain other rollups @@ -1092,7 +1208,12 @@ pub fn invalidation_comment( }; let action = match &**reason { - InvalidationReason::CommitShaChanged => format!("{} its commit SHA", wrap("changed")), + InvalidationReason::CommitShaChanged { sha } => { + format!("{} its commit SHA to {sha}", wrap("changed")) + } + InvalidationReason::BaseBranchChanged => { + format!("{} its base branch", wrap("changed")) + } InvalidationReason::Close => format!("was {}", wrap("closed")), InvalidationReason::Unapproval { .. } => format!("was {}", wrap("unapproved")), InvalidationReason::RollupMemberInvalidated { .. } => { diff --git a/src/bors/handlers/pr_events.rs b/src/bors/handlers/pr_events.rs index f85d2742..0f2906c0 100644 --- a/src/bors/handlers/pr_events.rs +++ b/src/bors/handlers/pr_events.rs @@ -41,7 +41,7 @@ pub(super) async fn handle_pull_request_edited( &db, &pr_model, pr, - InvalidationInfo::new(InvalidationReason::CommitShaChanged), + InvalidationInfo::new(InvalidationReason::BaseBranchChanged), Some(InvalidationComment::new(format!( ":warning: The base branch changed to `{base_name}`.", base_name = payload.pull_request.base.name @@ -71,7 +71,9 @@ pub(super) async fn handle_push_to_pull_request( &db, &pr_model, pr, - InvalidationInfo::new(InvalidationReason::CommitShaChanged), + InvalidationInfo::new(InvalidationReason::CommitShaChanged { + sha: pr.head.sha.clone(), + }), Some(InvalidationComment::new(format!( ":warning: A new commit `{}` was pushed.", pr.head.sha diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 6575429c..3829e2f7 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -35,6 +35,7 @@ pub(super) async fn command_approve( rollup_mode: Option, note: Option, merge_queue_tx: &MergeQueueSender, + sha: CommitSha, ) -> anyhow::Result<()> { tracing::info!("Approving PR {}", pr.number()); if !has_permission(&repo_state, author, pr, PermissionType::Review).await? { @@ -79,7 +80,7 @@ pub(super) async fn command_approve( let approval_info = ApprovalInfo { approver: approver.clone(), - sha: pr.github.head.sha.to_string(), + sha: sha.to_string(), }; db.approve(pr.db, approval_info, priority, rollup_mode, note) @@ -125,7 +126,7 @@ pub(super) async fn command_approve( approved_comment( ctx.get_web_url(), repo_state.repository(), - &pr.github.head.sha, + &sha, &approver, unknown_reviewers, tree_state, diff --git a/src/bors/handlers/squash.rs b/src/bors/handlers/squash.rs index 83aa1da4..7d1a71f5 100644 --- a/src/bors/handlers/squash.rs +++ b/src/bors/handlers/squash.rs @@ -14,16 +14,21 @@ use crate::bors::{ use crate::database::BuildStatus; use crate::github::api::CommitAuthor; use crate::github::api::operations::Commit; -use crate::github::{GithubRepoName, GithubUser}; +use crate::github::{CommitSha, GithubRepoName, GithubUser}; use crate::permissions::PermissionType; use std::collections::HashSet; use std::fmt::Write; +use std::pin::Pin; use std::sync::Arc; const CO_AUTHORED_BY_TRAILER: &str = "Co-authored-by"; +pub(super) type AfterSquashCallback = + Box Pin> + Send>> + Send>; + /// Entry point for the squash command. /// This function validates the command and enqueues the actual work to the gitops queue. +#[allow(clippy::too_many_arguments)] pub(super) async fn command_squash( repo_state: Arc, db: Arc, @@ -32,6 +37,7 @@ pub(super) async fn command_squash( commit_message: SquashCommitMessage, bot_prefix: &CommandPrefix, gitops_queue: &GitOpsQueueSender, + after_squash_callback: Option, ) -> anyhow::Result<()> { let send_comment = async |text: String| { let comment = repo_state @@ -227,8 +233,10 @@ pub(super) async fn command_squash( &db, &pr_model, &pr_github, - InvalidationInfo::new(InvalidationReason::CommitShaChanged) - .with_comment_url(notify_comment.html_url.to_string()), + InvalidationInfo::new(InvalidationReason::CommitShaChanged { + sha: commit.clone(), + }) + .with_comment_url(notify_comment.html_url.to_string()), Some( InvalidationComment::new(format!( ":hammer: {} commits were squashed into {commit}.", @@ -240,6 +248,11 @@ pub(super) async fn command_squash( .await?; // Hide previous "squash started" comments. hide_tagged_comments(&repo_state, &db, &pr_model, CommentTag::SquashStarted).await?; + + if let Some(cb) = after_squash_callback { + cb(commit).await?; + } + Ok(()) }) }); @@ -259,7 +272,7 @@ pub(super) async fn command_squash( source_repo: repo_state.repository().clone(), target_repo: fork_repository, target_branch, - commit, + commit: commit.clone(), token, on_finish, }); @@ -881,6 +894,36 @@ also include this pls .await; } + #[sqlx::test(migrator = "crate::MIGRATOR")] + async fn squash_approve_push_webhook(pool: sqlx::PgPool) { + run_test((pool, squash_state()), async |ctx: &mut BorsTester| { + ctx.modify_pr_in_gh((), |pr| { + pr.title = "Foobar".to_string(); + pr.reset_to_single_commit(Commit::from_sha("sha1")); + pr.add_commits(vec![Commit::from_sha("sha2")]); + }); + ctx.post_comment("@bors r+ squash").await?; + ctx.expect_comments((), 1).await; + ctx.run_gitop_queue().await?; + insta::assert_snapshot!( + ctx.get_next_comment_text(()).await?, + @":hammer: 2 commits were squashed into sha2-reauthored-to-git-user." + ); + insta::assert_snapshot!( + ctx.get_next_comment_text(()).await?, + @":hammer: 2 commits were squashed into sha2-reauthored-to-git-user." + ); + let branch = ctx.pr(()).await.get_gh_pr().head_branch_copy(); + + // Check that this won't unapprove the PR + ctx.push_to_pr((), branch.get_commit().clone()).await?; + + ctx.pr(()).await.expect_approved_by("default-user"); + Ok(()) + }) + .await; + } + fn squash_state() -> GitHub { let gh = GitHub::default(); let pr_author = User::default_pr_author(); diff --git a/src/bors/merge_queue.rs b/src/bors/merge_queue.rs index 850c1b3b..5c73514c 100644 --- a/src/bors/merge_queue.rs +++ b/src/bors/merge_queue.rs @@ -465,14 +465,14 @@ async fn handle_start_auto_build( &ctx.db, pr, &gh_pr, - InvalidationInfo::new(InvalidationReason::CommitShaChanged), + InvalidationInfo::new(InvalidationReason::CommitShaChanged { + sha: actual.clone(), + }), Some( InvalidationComment::new(format!( r#"Commit SHA did not match the approved SHA during a merge attempt. -Approved commit SHA: {expected_sha} -Actual head SHA: {actual_sha}"#, - expected_sha = pr.approved_sha().unwrap_or("").to_owned(), - actual_sha = gh_pr.head.sha +Approved commit SHA: {approved} +Actual head SHA: {actual}"#, )) .post_always(), ), diff --git a/src/bors/mod.rs b/src/bors/mod.rs index 96ccf6ef..4f0139ab 100644 --- a/src/bors/mod.rs +++ b/src/bors/mod.rs @@ -106,6 +106,7 @@ pub fn format_help() -> &'static str { BorsCommand::Retry => {} BorsCommand::Cancel => {} BorsCommand::Squash { .. } => {} + BorsCommand::SquashApprove { .. } => {} } r#" diff --git a/src/database/client.rs b/src/database/client.rs index 43076c0e..37e37a56 100644 --- a/src/database/client.rs +++ b/src/database/client.rs @@ -8,8 +8,9 @@ use super::operations::{ insert_repo_if_not_exists, is_rollup, record_tagged_bot_comment, set_pr_assignees, set_pr_mergeability_state, set_pr_priority, set_pr_rollup_mode, set_pr_status, set_rollup_member_unrolled_state, set_rollup_members_unrolled_state, - set_stale_mergeability_status_by_base_branch, unapprove_pull_request, undelegate_pull_request, - update_build, update_pr_try_build_id, update_pr_unrolled_build_id, update_workflow_status, + set_stale_mergeability_status_by_base_branch, unapprove_pull_request, + unapprove_pull_request_if_sha_changed, undelegate_pull_request, update_build, + update_pr_try_build_id, update_pr_unrolled_build_id, update_workflow_status, upsert_pull_request, upsert_repository, }; use super::{ @@ -114,6 +115,17 @@ impl PgDbClient { unapprove_pull_request(&self.pool, pr.id).await } + /// Unapprove a pull request and remove its auto build status, if there is any attached. + /// Only do it if `sha` wasn't already approved. + /// Returns true if the PR was actually unapproved. + pub async fn unapprove_if_sha_changed( + &self, + pr: &PullRequestModel, + sha: &CommitSha, + ) -> anyhow::Result { + unapprove_pull_request_if_sha_changed(&self.pool, pr.id, sha).await + } + pub async fn clear_auto_build(&self, pr: &PullRequestModel) -> anyhow::Result<()> { clear_auto_build(&self.pool, pr.id).await } diff --git a/src/database/operations.rs b/src/database/operations.rs index 21623973..8710c7b7 100644 --- a/src/database/operations.rs +++ b/src/database/operations.rs @@ -527,6 +527,32 @@ pub(crate) async fn unapprove_pull_request( .await } +pub(crate) async fn unapprove_pull_request_if_sha_changed( + executor: impl PgExecutor<'_>, + pr_id: i32, + sha: &CommitSha, +) -> anyhow::Result { + measure_db_query("unapprove_pull_request_if_sha_changed", || async { + let result = sqlx::query!( + r#" + UPDATE pull_request + SET approved_by = NULL, + approved_sha = NULL, + auto_build_id = NULL + WHERE + id = $1 AND + (approved_by IS NULL OR approved_sha != $2) + "#, + pr_id, + sha.0 + ) + .execute(executor) + .await?; + Ok(result.rows_affected() > 0) + }) + .await +} + pub(crate) async fn delegate_pull_request( executor: impl PgExecutor<'_>, pr_id: i32, diff --git a/src/github/rollup.rs b/src/github/rollup.rs index e9478ce5..ecf4e09b 100644 --- a/src/github/rollup.rs +++ b/src/github/rollup.rs @@ -1111,7 +1111,7 @@ also include this pls" This PR was contained in a rollup (#4), which was closed. "); insta::assert_snapshot!(ctx.get_next_comment_text(4).await?, @" - PR #3, which is a member of this rollup, changed its commit SHA. + PR #3, which is a member of this rollup, changed its commit SHA to foo. This rollup was thus unapproved due to being closed. "); @@ -1192,7 +1192,7 @@ also include this pls" "); insta::assert_snapshot!(ctx.get_next_comment_text(3).await?, @":hourglass: Testing commit pr-3-sha with merge merge-0-pr-3-d7d45f1f-reauthored-to-bors..."); insta::assert_snapshot!(ctx.get_next_comment_text(4).await?, @" - PR #2, which is a member of this rollup, changed its commit SHA. + PR #2, which is a member of this rollup, changed its commit SHA to foobar. This rollup was closed. ");