Skip to content

Commit d8417be

Browse files
committed
Do not perform PR assignment for draft PRs without r?
1 parent f118be8 commit d8417be

File tree

1 file changed

+47
-10
lines changed

1 file changed

+47
-10
lines changed

src/handlers/assign.rs

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,19 @@ Please choose another assignee.";
9090
// Special account that we use to prevent assignment.
9191
const GHOST_ACCOUNT: &str = "ghost";
9292

93+
/// Assignment data stored in the issue/PR body.
9394
#[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
9495
struct AssignData {
9596
user: Option<String>,
9697
}
9798

98-
/// Input for auto-assignment when a PR is created.
99-
pub(super) struct AssignInput {}
99+
/// Input for auto-assignment when a PR is created or converted from draft.
100+
#[derive(Debug)]
101+
pub(super) enum AssignInput {
102+
Opened,
103+
OpenedDraft,
104+
ReadyForReview,
105+
}
100106

101107
/// Prepares the input when a new PR is opened.
102108
pub(super) async fn parse_input(
@@ -108,13 +114,19 @@ pub(super) async fn parse_input(
108114
Some(config) => config,
109115
None => return Ok(None),
110116
};
111-
if config.owners.is_empty()
112-
|| !matches!(event.action, IssuesAction::Opened)
113-
|| !event.issue.is_pr()
114-
{
117+
if config.owners.is_empty() || !event.issue.is_pr() {
115118
return Ok(None);
116119
}
117-
Ok(Some(AssignInput {}))
120+
121+
match event.action {
122+
IssuesAction::Opened => Ok(Some(if event.issue.draft {
123+
AssignInput::OpenedDraft
124+
} else {
125+
AssignInput::Opened
126+
})),
127+
IssuesAction::ReadyForReview => Ok(Some(AssignInput::ReadyForReview)),
128+
_ => Ok(None),
129+
}
118130
}
119131

120132
/// Handles the work of setting an assignment for a new PR and posting a
@@ -123,8 +135,31 @@ pub(super) async fn handle_input(
123135
ctx: &Context,
124136
config: &AssignConfig,
125137
event: &IssuesEvent,
126-
_input: AssignInput,
138+
input: AssignInput,
127139
) -> anyhow::Result<()> {
140+
let assign_command = find_assign_command(ctx, event);
141+
142+
// Perform assignment when:
143+
// - PR was opened normally
144+
// - PR was opened as a draft with an explicit r? (but not r? ghost)
145+
// - PR was converted from a draft and there are no current assignees
146+
let should_assign = match input {
147+
AssignInput::Opened => true,
148+
AssignInput::OpenedDraft => {
149+
// Even if the PR is opened as a draft, we still want to perform assignment if r?
150+
// was used. However, historically, r? ghost was meant to mean "do not
151+
// perform assignment". So in that case, we skip the assignment and only perform it once
152+
// the PR has been marked as being ready for review.
153+
assign_command.is_some() && assign_command.as_deref() != Some(GHOST_ACCOUNT)
154+
}
155+
AssignInput::ReadyForReview => event.issue.assignees.is_empty(),
156+
};
157+
158+
if !should_assign {
159+
log::info!("Skipping PR assignment, input: {input:?}, assign_command: {assign_command:?}");
160+
return Ok(());
161+
}
162+
128163
let Some(diff) = event.issue.diff(&ctx.github).await? else {
129164
bail!(
130165
"expected issue {} to be a PR, but the diff could not be determined",
@@ -134,7 +169,8 @@ pub(super) async fn handle_input(
134169

135170
// Don't auto-assign or welcome if the user manually set the assignee when opening.
136171
if event.issue.assignees.is_empty() {
137-
let (assignee, from_comment) = determine_assignee(ctx, event, config, &diff).await?;
172+
let (assignee, from_comment) =
173+
determine_assignee(ctx, assign_command, event, config, &diff).await?;
138174
if assignee.as_deref() == Some(GHOST_ACCOUNT) {
139175
// "ghost" is GitHub's placeholder account for deleted accounts.
140176
// It is used here as a convenient way to prevent assignment. This
@@ -257,13 +293,14 @@ async fn set_assignee(issue: &Issue, github: &GithubClient, username: &str) {
257293
/// determined from the diff).
258294
async fn determine_assignee(
259295
ctx: &Context,
296+
assign_command: Option<String>,
260297
event: &IssuesEvent,
261298
config: &AssignConfig,
262299
diff: &[FileDiff],
263300
) -> anyhow::Result<(Option<String>, bool)> {
264301
let db_client = ctx.db.get().await;
265302
let teams = crate::team_data::teams(&ctx.github).await?;
266-
if let Some(name) = find_assign_command(ctx, event) {
303+
if let Some(name) = assign_command {
267304
// User included `r?` in the opening PR body.
268305
match find_reviewer_from_names(&db_client, &teams, config, &event.issue, &[name]).await {
269306
Ok(assignee) => return Ok((Some(assignee), true)),

0 commit comments

Comments
 (0)