Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reapproval is not Working when Auto-Approval is set #1069

Closed
SeMuell opened this issue Apr 5, 2024 · 4 comments · Fixed by #1132
Closed

Reapproval is not Working when Auto-Approval is set #1069

SeMuell opened this issue Apr 5, 2024 · 4 comments · Fixed by #1132

Comments

@SeMuell
Copy link

SeMuell commented Apr 5, 2024

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
Copy link
Author

SeMuell commented Apr 5, 2024

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

@mburumaxwell
Copy link
Contributor

mburumaxwell commented Apr 23, 2024

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:

By the way, thanks for your sponsorship @SeMuell

@SeMuell
Copy link
Author

SeMuell commented Apr 26, 2024

@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!

@mburumaxwell
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants