From 78d0175d0db7bb3b6e48fc261545a501512a8f65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Zimmermann?= Date: Thu, 4 Jul 2024 23:07:17 +0200 Subject: [PATCH] Adapt handling of rejected PRs to GitHub projects v2. --- bot-components/GitHub_GraphQL.ml | 27 ++++++++++++++ bot-components/GitHub_mutations.ml | 8 +--- bot-components/GitHub_mutations.mli | 6 ++- bot-components/GitHub_queries.ml | 49 ++++++++++++++++++++++++- bot-components/GitHub_queries.mli | 12 ++++++ bot-components/GitHub_types.mli | 2 +- src/actions.ml | 57 ++++++++++++++++------------- src/actions.mli | 8 ++-- src/bot.ml | 23 ++++++------ 9 files changed, 142 insertions(+), 50 deletions(-) diff --git a/bot-components/GitHub_GraphQL.ml b/bot-components/GitHub_GraphQL.ml index d0a228cb..67ce98f9 100644 --- a/bot-components/GitHub_GraphQL.ml +++ b/bot-components/GitHub_GraphQL.ml @@ -33,6 +33,21 @@ module PullRequest_ID = } |}] +module PullRequest_Milestone = +[%graphql +{| + query prInfo($pr_id: ID!) { + node(id: $pr_id) { + ... on PullRequest { + milestone { + title + description + } + } + } + } +|}] + module PullRequest_ID_and_Milestone = [%graphql {| @@ -49,6 +64,18 @@ module PullRequest_ID_and_Milestone = } |}] +module Milestone_ID = +[%graphql +{| + query milestoneID($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner,name: $repo) { + milestone(number: $number) { + id + } + } + } +|}] + module TeamMembership = [%graphql {| diff --git a/bot-components/GitHub_mutations.ml b/bot-components/GitHub_mutations.ml index b76e42e9..c46a659f 100644 --- a/bot-components/GitHub_mutations.ml +++ b/bot-components/GitHub_mutations.ml @@ -262,21 +262,17 @@ let remove_labels ~bot_info ~labels ~issue = (* TODO: use GraphQL API *) -let update_milestone ~bot_info new_milestone (issue : issue) = +let remove_milestone ~bot_info (issue : issue) = let headers = headers (github_header bot_info) bot_info.github_name in let uri = f "https://api.github.com/repos/%s/%s/issues/%d" issue.owner issue.repo issue.number |> Uri.of_string in - let body = - f {|{"milestone": %s}|} new_milestone |> Cohttp_lwt.Body.of_string - in + let body = {|{"milestone": null}|} |> Cohttp_lwt.Body.of_string in Lwt_io.printf "Sending patch request.\n" >>= fun () -> Client.patch ~headers ~body uri >>= print_response -let remove_milestone = update_milestone "null" - let send_status_check ~bot_info ~repo_full_name ~commit ~state ~url ~context ~description = Lwt_io.printf "Sending status check to %s (commit %s, state %s)\n" diff --git a/bot-components/GitHub_mutations.mli b/bot-components/GitHub_mutations.mli index 082e8b84..56e55614 100644 --- a/bot-components/GitHub_mutations.mli +++ b/bot-components/GitHub_mutations.mli @@ -81,7 +81,11 @@ val remove_labels : -> issue:GitHub_ID.t -> unit Lwt.t -val update_milestone : bot_info:Bot_info.t -> string -> issue -> unit Lwt.t +val update_milestone : + bot_info:Bot_info.t + -> issue:GitHub_ID.t + -> milestone:GitHub_ID.t + -> unit Lwt.t val remove_milestone : bot_info:Bot_info.t -> issue -> unit Lwt.t diff --git a/bot-components/GitHub_queries.ml b/bot-components/GitHub_queries.ml index 5d79db1a..1af574e9 100644 --- a/bot-components/GitHub_queries.ml +++ b/bot-components/GitHub_queries.ml @@ -18,13 +18,17 @@ let extract_backport_info ~(bot_info : Bot_info.t) description : let rec aux description = if string_match ~regexp:backport_info_unit description then let backport_to = Str.matched_group 1 description in - let rejected_milestone = Str.matched_group 2 description in + let rejected_milestone = + Str.matched_group 2 description |> Int.of_string + in Str.matched_group 3 description |> aux |> List.cons {backport_to; rejected_milestone} else if string_match ~regexp:end_regexp description then let backport_to = Str.matched_group 1 description in - let rejected_milestone = Str.matched_group 2 description in + let rejected_milestone = + Str.matched_group 2 description |> Int.of_string + in [{backport_to; rejected_milestone}] else [] in @@ -61,6 +65,28 @@ let get_pull_request_cards ~bot_info ~owner ~repo ~number = | Error err -> Error err +let get_pull_request_milestone ~bot_info ~pr_id = + let open GitHub_GraphQL.PullRequest_Milestone in + let pr_id = GitHub_ID.to_string pr_id in + makeVariables ~pr_id () |> serializeVariables |> variablesToJson + |> send_graphql_query ~bot_info ~query + ~parse:(Fn.compose parse unsafe_fromJson) + >|= Result.bind ~f:(fun result -> + match result.node with + | Some (`PullRequest result) -> ( + match result.milestone with + | Some milestone -> + Ok + ( milestone.description + |> Option.map ~f:(extract_backport_info ~bot_info) + |> Option.value ~default:[] ) + | None -> + Ok [] ) + | Some _ -> + Error (f "Node %s is not a pull request." pr_id) + | None -> + Error (f "Pull request %s does not exist." pr_id) ) + let get_pull_request_id_and_milestone ~bot_info ~owner ~repo ~number = let open GitHub_GraphQL.PullRequest_ID_and_Milestone in makeVariables ~owner ~repo ~number () @@ -107,6 +133,25 @@ let get_pull_request_id ~bot_info ~owner ~repo ~number = | Some pr -> Ok (GitHub_ID.of_string pr.id) ) ) +let get_milestone_id ~bot_info ~owner ~repo ~number = + let open GitHub_GraphQL.Milestone_ID in + makeVariables ~owner ~repo ~number () + |> serializeVariables |> variablesToJson + |> send_graphql_query ~bot_info ~query + ~parse:(Fn.compose parse unsafe_fromJson) + >|= Result.bind ~f:(fun result -> + match result.repository with + | None -> + Error (f "Repository %s/%s does not exist." owner repo) + | Some result -> ( + match result.milestone with + | None -> + Error + (f "Milestone %d does not exist in repository %s/%s." number + owner repo ) + | Some milestone -> + Ok (GitHub_ID.of_string milestone.id) ) ) + let team_membership_of_resp ~org ~team ~user resp = let open GitHub_GraphQL.TeamMembership in match resp.organization with diff --git a/bot-components/GitHub_queries.mli b/bot-components/GitHub_queries.mli index 1fe6f341..b9b569e4 100644 --- a/bot-components/GitHub_queries.mli +++ b/bot-components/GitHub_queries.mli @@ -7,6 +7,11 @@ val get_pull_request_cards : -> number:int -> ((GitHub_ID.t * int) list, string) result Lwt.t +val get_pull_request_milestone : + bot_info:Bot_info.t + -> pr_id:GitHub_ID.t + -> (backport_info list, string) result Lwt.t + val get_pull_request_id_and_milestone : bot_info:Bot_info.t -> owner:string @@ -21,6 +26,13 @@ val get_pull_request_id : -> number:int -> (GitHub_ID.t, string) result Lwt.t +val get_milestone_id : + bot_info:Bot_info.t + -> owner:string + -> repo:string + -> number:int + -> (GitHub_ID.t, string) result Lwt.t + val get_team_membership : bot_info:Bot_info.t -> org:string diff --git a/bot-components/GitHub_types.mli b/bot-components/GitHub_types.mli index 6103074b..c8f37f77 100644 --- a/bot-components/GitHub_types.mli +++ b/bot-components/GitHub_types.mli @@ -7,7 +7,7 @@ type project_column = {id: GitHub_ID.t; databaseId: int option} type merge_method = MERGE | REBASE | SQUASH -type backport_info = {backport_to: string; rejected_milestone: string} +type backport_info = {backport_to: string; rejected_milestone: int} type pull_request_card_info = { pr_id: GitHub_ID.t diff --git a/src/actions.ml b/src/actions.ml index 58b35dc6..4a2bd919 100644 --- a/src/actions.ml +++ b/src/actions.ml @@ -2596,34 +2596,41 @@ let rec adjust_milestone ~bot_info ~issue ~sleep_time = | Error err -> Lwt_io.print (f "Error: %s\n" err) -(* TODO: adapt to rejection through GitHub Project v2 *) -(* -let project_action ~bot_info ~(issue : issue) ~column_id = - GitHub_queries.get_pull_request_id_and_milestone ~bot_info ~owner:"coq" - ~repo:"coq" ~number:issue.number +let project_action ~bot_info ~pr_id ~backport_to () = + GitHub_queries.get_pull_request_milestone ~bot_info ~pr_id >>= function | Error err -> Lwt_io.printf "Error: %s\n" err - | Ok None -> - Lwt_io.printf "Could not find backporting info for PR.\n" - | Ok (Some (id, {backport_info; rejected_milestone})) - when List.exists backport_info ~f:(fun {request_inclusion_column} -> - Int.equal request_inclusion_column column_id ) -> - Lwt_io.printf - "This was a request inclusion column: PR was rejected.\n\ - Change of milestone requested to: %s\n" - rejected_milestone - >>= fun () -> - GitHub_mutations.update_milestone rejected_milestone issue ~bot_info - <&> ( GitHub_mutations.post_comment ~bot_info ~id - ~message: - "This PR was postponed. Please update accordingly the \ - milestone of any issue that this fixes as this cannot be done \ - automatically." - >>= GitHub_mutations.report_on_posting_comment ) - | _ -> - Lwt_io.printf "This was not a request inclusion column: ignoring.\n" -*) + | Ok backport_info -> ( + match + List.find_map backport_info + ~f:(fun {backport_to= backport_to'; rejected_milestone} -> + if String.equal backport_to backport_to' then Some rejected_milestone + else None ) + with + | None -> + Lwt_io.printf + "PR already not in milestone with backporting info for branch %s.\n" + backport_to + | Some rejected_milestone -> ( + Lwt_io.printf + "PR is in milestone for which backporting to %s was rejected.\n\ + Change of milestone requested.\n" + backport_to + >>= fun () -> + GitHub_queries.get_milestone_id ~bot_info ~owner:"coq" ~repo:"coq" + ~number:rejected_milestone + >>= function + | Ok milestone -> + GitHub_mutations.update_milestone ~bot_info ~issue:pr_id ~milestone + <&> ( GitHub_mutations.post_comment ~bot_info ~id:pr_id + ~message: + "This PR was postponed. Please update accordingly the \ + milestone of any issue that this fixes as this cannot \ + be done automatically." + >>= GitHub_mutations.report_on_posting_comment ) + | Error err -> + Lwt_io.printlf "Error while obtaining milestone ID: %s" err ) ) let add_to_column ~bot_info ~backport_to id option = let field = backport_to ^ " status" in diff --git a/src/actions.mli b/src/actions.mli index aaeeddda..7630d62f 100644 --- a/src/actions.mli +++ b/src/actions.mli @@ -73,10 +73,12 @@ val adjust_milestone : -> sleep_time:float -> unit Lwt.t -(* val project_action : - bot_info:Bot_info.t -> issue:GitHub_types.issue -> column_id:int -> unit Lwt.t -*) + bot_info:Bot_info.t + -> pr_id:GitHub_ID.t + -> backport_to:string + -> unit + -> unit Lwt.t val coq_push_action : bot_info:Bot_info.t diff --git a/src/bot.ml b/src/bot.ml index 692cdcd1..95e7ed28 100644 --- a/src/bot.ml +++ b/src/bot.ml @@ -291,21 +291,20 @@ let callback _conn req body = issue.owner issue.repo issue.number ) () | Ok - ( _ - , PullRequestCardEdited {project_number; field; old_value; new_value} - ) - when Int.equal project_number 11 - && String.equal old_value "Request inclusion" - && String.equal new_value "Rejected" - && String.is_suffix ~suffix:" status" field -> + ( Some (1062161 as install_id) (* Coq's installation number *) + , PullRequestCardEdited + { project_number= 11 (* Coq's backporting project number *) + ; pr_id + ; field + ; old_value= "Request inclusion" + ; new_value= "Rejected" } ) + when String.is_suffix ~suffix:" status" field -> let backport_to = String.drop_suffix field 7 in - (* (fun () -> - action_as_github_app ~bot_info ~key ~app_id ~owner:issue.owner - ~repo:issue.repo - (project_action ~issue ~column_id) ) + action_as_github_app_from_install_id ~bot_info ~key ~app_id + ~install_id + (project_action ~pr_id ~backport_to ()) ) |> Lwt.async ; - *) Server.respond_string ~status:`OK ~body: (f