dependabot-azure-devops icon indicating copy to clipboard operation
dependabot-azure-devops copied to clipboard

Reapproval is not Working when Auto-Approval is set

Open SeMuell opened this issue 1 year ago • 3 comments

Describe the bug After a new branch policy has been introduced in Azure DevOps Service AZD Release Notes August 2023 the vote can remain on Approved, but needs to be re-approved. Unfortunately, the re-approval is not done when the auto-approval feature is used, but would be very beneficial doing when merge conflicts are auto-resolved by a rerun of dependabot.

To Reproduce Steps to reproduce the behavior:

  1. In AZD set branch policy option "Require at least one approval on every iteration" under the "When new changes are pushed".
  2. Let dependabot run with auto approval set which has to create some PRs
  3. Make a change in the target repo which adds a conflict
  4. Complete the PR which throws merge conflicts error
  5. Rerun dependabot
  6. Approval is not enough since reapproval is required

Expected behavior Expected to set a new approval whenever a re-approval is required and the auto-approve feature is used.

Potential solution The new feature does not change the vote enumeration which is the reason that the auto-approval is not done. In the Azure DevOps REST API from version 7.1 on a new flag isReapprove is introduced (Get Pull Request Reviewers API) should be checked as well when auto-approval is on.

Extension (please complete the following information):

  • Host: Azure DevOps
  • Version 1.24 (have to stay on this version due to authentication issues)

SeMuell avatar Apr 05 '24 11:04 SeMuell

I cannot verify currently, but maybe it works:

In azure_helpers.rb change two lines:

      def pull_request_approve(pull_request_id, reviewer_token)
        user_id = get_user_id(reviewer_token)

        # https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-reviewers/create-pull-request-reviewers?view=azure-devops-rest-6.0
        content = {
          # 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected
          vote: 10,
          isReapprove: true
        }

        put_with_token(source.api_endpoint + source.organization + "/" + source.project +
                       "/_apis/git/repositories/" + source.unscoped_repo +
                       "/pullrequests/#{pull_request_id}/reviewers/#{user_id}" \
                       "?api-version=7.2-preview.2", content.to_json, reviewer_token)
      end

SeMuell avatar Apr 05 '24 15:04 SeMuell

According to your followup comment, the change would be:

        def pull_request_approve(pull_request_id, reviewer_token)
          user_id = get_user_id(reviewer_token)

          # https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-reviewers/create-pull-request-reviewers?view=azure-devops-rest-6.0
          content = {
            # 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected
            vote: 10,
+           isReapprove: true
          }

          put_with_token(source.api_endpoint + source.organization + "/" + source.project +
                         "/_apis/git/repositories/" + source.unscoped_repo +
                         "/pullrequests/#{pull_request_id}/reviewers/#{user_id}" \
-                        "?api-version=6.0", content.to_json, reviewer_token)
+                        "?api-version=7.2-preview.2", content.to_json, reviewer_token)
        end

If this is the case, my observations would be:

  1. The API version changes from 6.0 to 7.2-preview.2 which may not be available to some Azure DevOps OnPrem setups.
  2. I would avoid using a preview version just in case it breaks other things for others. We could bump from 6.0 to 7.1 which still has the isReApprove property but observation 1 would come into play.

If we can be sure about the implications of the observations, then I can proceed with:

  • [x] https://github.com/tinglesoftware/dependabot-azure-devops/pull/1101
  • [ ] Add a new input to the task and server for reapproval.

By the way, thanks for your sponsorship @SeMuell

mburumaxwell avatar Apr 23 '24 10:04 mburumaxwell

@mburumaxwell, do you know if there is a way to check the latest supported api-version in Azure DevOps? I did not find any way doing a quick search. However, one way would be introducing a fallback whenever the response is not 200, but this is of course not the nicest solution...

I did test the additional isReapprove flag inside the json content and can confirm it works.

And happy to sponsor, you are saving me quite a lot of time and I definitely appreciate your work!

SeMuell avatar Apr 26 '24 11:04 SeMuell

You can check the versions at https://learn.microsoft.com/en-us/azure/devops/integrate/concepts/rest-api-versioning?view=azure-devops#supported-versions

In #1131, the version has been updated to 7.1 which clears the way for this to be implemented.

mburumaxwell avatar May 23 '24 14:05 mburumaxwell