From 507cc27ee4e5c12f8fd8b815698fdfa86eb612c4 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 5 Apr 2025 12:04:50 +0200 Subject: [PATCH] Also show warnings when PRs are updated --- src/handlers/check_commits.rs | 79 ++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/src/handlers/check_commits.rs b/src/handlers/check_commits.rs index ef1c6f095..719214626 100644 --- a/src/handlers/check_commits.rs +++ b/src/handlers/check_commits.rs @@ -3,18 +3,35 @@ use anyhow::bail; use super::Context; use crate::{ config::Config, - github::{Event, IssuesAction}, + db::issue_data::IssueData, + github::{Event, IssuesAction, IssuesEvent, ReportedContentClassifiers}, }; mod modified_submodule; mod non_default_branch; +/// Key for the state in the database +const CHECK_COMMITS_WARNINGS_KEY: &str = "check-commits-warnings"; + +/// State stored in the database +#[derive(Debug, Default, serde::Deserialize, serde::Serialize)] +struct CheckCommitsWarningsState { + /// List of the last warnings in the most recent comment. + last_warnings: Vec, + /// ID of the most recent warning comment. + last_warned_comment: Option, +} + pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> anyhow::Result<()> { let Event::Issue(event) = event else { return Ok(()); }; - if !matches!(event.action, IssuesAction::Opened) || !event.issue.is_pr() { + if !matches!( + event.action, + IssuesAction::Opened | IssuesAction::Synchronize + ) || !event.issue.is_pr() + { return Ok(()); } @@ -27,6 +44,7 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any let mut warnings = Vec::new(); + // Compute the warnings if let Some(assign_config) = &config.assign { // For legacy reasons the non-default-branch and modifies-submodule warnings // are behind the `[assign]` config. @@ -40,14 +58,65 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any warnings.extend(modified_submodule::modifies_submodule(diff)); } - if !warnings.is_empty() { + handle_warnings(ctx, event, warnings).await +} + +// Add, hide or hide&add a comment with the warnings. +async fn handle_warnings( + ctx: &Context, + event: &IssuesEvent, + warnings: Vec, +) -> anyhow::Result<()> { + // Get the state of the warnings for this PR in the database. + let mut db = ctx.db.get().await; + let mut state: IssueData<'_, CheckCommitsWarningsState> = + IssueData::load(&mut db, &event.issue, CHECK_COMMITS_WARNINGS_KEY).await?; + + // We only post a new comment when we haven't posted one with the same warnings before. + if !warnings.is_empty() && state.data.last_warnings != warnings { + // New set of warnings, let's post them. + + // Hide a previous warnings comment if there was one before printing the new ones. + if let Some(last_warned_comment_id) = state.data.last_warned_comment { + event + .issue + .hide_comment( + &ctx.github, + &last_warned_comment_id, + ReportedContentClassifiers::Resolved, + ) + .await?; + } + + // Format the warnings for user consumption on Github let warnings: Vec<_> = warnings .iter() .map(|warning| format!("* {warning}")) .collect(); let warning = format!(":warning: **Warning** :warning:\n\n{}", warnings.join("\n")); - event.issue.post_comment(&ctx.github, &warning).await?; - }; + let comment = event.issue.post_comment(&ctx.github, &warning).await?; + + // Save new state in the database + state.data.last_warnings = warnings; + state.data.last_warned_comment = Some(comment.node_id); + state.save().await?; + } else if warnings.is_empty() { + // No warnings to be shown, let's resolve a previous warnings comment, if there was one. + if let Some(last_warned_comment_id) = state.data.last_warned_comment { + event + .issue + .hide_comment( + &ctx.github, + &last_warned_comment_id, + ReportedContentClassifiers::Resolved, + ) + .await?; + + state.data.last_warnings = Vec::new(); + state.data.last_warned_comment = None; + state.save().await?; + } + } Ok(()) }