palantir / policy-bot

A GitHub App that enforces approval policies on pull requests
Apache License 2.0
727 stars 104 forks source link

has_successful_status causes review requests while PR has draft status #741

Closed stevoland closed 3 months ago

stevoland commented 3 months ago

If I interpret the docs correctly:

Automatically request reviewers when a Pull Request is opened if this rule is pending, there are no assigned reviewers, and if the Pull Request is not in Draft.

request_review should not request reviews if the PR has draft status.

The behaviour seems correct with this config:

  - name: Approval
    requires:
      count: 2
      teams:
        - "xxx"
    options:
      allow_author: false
      allow_non_author_contributor: true
      invalidate_on_push: true
      ignore_edited_comments: true
      ignore_update_merges: true
      request_review:
        enabled: true
        mode: random-users
        count: 3
      methods:
        comments: []
        github_review: true

but after adding a has_successful_status condition, reviews are requested when the checks pass even if the PR remains in draft:

  - name: Approval
    if:
      has_successful_status:
        - "checks"
    requires:
      count: 2
      teams:
        - "xxx"
    options:
      allow_author: false
      allow_non_author_contributor: true
      invalidate_on_push: true
      ignore_edited_comments: true
      ignore_update_merges: true
      request_review:
        enabled: true
        mode: random-users
        count: 3
      methods:
        comments: []
        github_review: true

(Thanks for a great app btw)

bluekeyes commented 3 months ago

Thanks for the report. This sounds like a bug and I think I found a probable cause, but just to confirm: is the snippet you shared the only rule in your policy that requests reviews or are there other rules involved too?

stevoland commented 3 months ago

@bluekeyes Good question - it's the latter. Apologies, I should have posted the full config:

policy:
  approval:
    - or:
        - Team approval
        - Approval

  disapproval:
    requires:
      teams:
        - yyy
    options:
      methods:
        disapprove:
          comments: []
          github_review: true
        revoke:
          comments: []
          github_review: true

approval_rules:
  - name: Team approval
    requires:
      count: 2
      teams:
        - yyy
    options:
      allow_author: false
      allow_non_author_contributor: true
      invalidate_on_push: false
      ignore_edited_comments: true
      ignore_update_merges: true
      request_review:
        enabled: true
        mode: teams
      methods:
        comments: []
        github_review: true
  - name: Approval
    if:
      has_successful_status:
        - "checks"
    requires:
      count: 2
      teams:
        - xxx
    options:
      allow_author: false
      allow_non_author_contributor: true
      invalidate_on_push: false
      ignore_edited_comments: true
      ignore_update_merges: true
      request_review:
        enabled: true
        mode: random-users
        count: 3
      methods:
        comments: []
        github_review: true

Just in case it's relevant, all members of xxx are in yyy.

bluekeyes commented 3 months ago

Thanks, I don't think those additional rule will impact things. If you're able to run snapshot versions, please try out the palantirtechnologies/policy-bot:snapshot image to see if it fixes this behavior.

stevoland commented 3 months ago

Thank you so much. I'll see if I'm able to try it today

stevoland commented 3 months ago

@bluekeyes

Gave the snapshot a quick test and looks good

Opened draft PR, required checks passed - correctly doesn't request reviews ✅ Moved PR out of draft - correctly requested reviews ✅

Thanks again!