Editing approval count for existing MR doesn't work

Thanks for reporting, @gjunker! This is intentional, but the feature is a little bit complicated and we might need to explain it better. https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#approvers has the full docs, but I'll copy here too.

Depending on the number of required approvals and the number of approvers set, there are different cases:

If there are more approvers than required approvals, any subset of these users can approve the merge request.

If there are less approvers than required approvals, all the set approvers plus any other user(s) need to approve the merge request before being able to merge it.

If the approvers are equal to the amount of required approvals, all the approvers are required to approve the merge request.

I've put together a list of feature proposals for the approvals feature at https://gitlab.com/gitlab-org/gitlab-ee/issues/580#note_16392433, so if you have a suggestion on how to make this clearer, please create a new issue and add it there.

Hi Sean

One problem I see, is that no matter how many approvers I have, I cannot set the approver count to one. The field is populated with "1" each time I edit the issue, but my guess is that since the field value doesn't change, it doesn't update the DB.

Example: I have 4 approvers on an issue. I decide to remove three of them, and do so, one at a time (clicking the X on the approver capsule). I save the issue, see that there are multiple approvers required, re-edit the issue, see that the approver count says 1. I really do want to reset to 1, but cannot.

Thanks Greg

Thanks @gjunker, I think I understand now. First of all, as the quote above says, there's no particular relationship between the appprovers list and the number of approvals required. So you'll still need three other project members to approve the MR.

We do have a feature that prevents the number of approvals on an MR being lower than the number set on the project. If I understand correctly, there might be a bug in that. Could you tell me what the number of approvals on the project level is set to, and what you see as the default when creating a new MR?

Hi Sean

The project defaults to 1 approver required, and that's what we see when we create new MRs.

Thanks @gjunker, I see the issue now. Steps to reproduce:

Create an MR on a project with approvals enabled.
Increase the number of approvals required for that MR.

This is shown correctly when viewing the MR.

Edit that MR.

Expected: the number of approvals in the edit form matches the number in step 2.
Actual: the number of approvals in the edit form matches the number set on the project.

Reduce the number of approvals back to the project default.

Expected: the number of approvals shown when viewing the MR matches the new number.
Actual: the number of approvals shown when viewing the MR matches the number in step 2.

Added ~481018 ~238305 ~91797 labels

Changed title: Editing approver list for existing issue causes incorrect approver count → Editing approval count for existing MR doesn't work

Mentioned in issue #580

Added ~874214 and removed ~481018 labels

Mentioned in issue #1162 (closed)

Milestone changed to %8.15

@smcgivern Do you want to prioritise this for 8.16?

changed milestone to %8.16

Yes, let's make it Stretch and then Deliverable for 8.17 if it doesn't make it. It's a dumb bug, and if we're making approvals better, bugs like this shouldn't exist.

added Stretch label

mentioned in issue #1501

assigned to @jarka

mentioned in merge request !1082 (merged)

I think the issue is only the number displayed in the edit form. Storing a new value works correctly, it is correctly displayed when showing MR, only shows wrong number in the edit form.

mentioned in issue #1591 (closed)

changed milestone to %8.17

closed via merge request !1082 (merged)

mentioned in commit b8f99c54

After updating to 8.16.1, I see this in an existing MR:

When I click the Rebase button, It gives me the "something went wrong on our end" message. There apparently were merge conflicts with master so I merged master into this branch and pushed, I then can see an accept-merge-request button but the approvals message is still incorrect.

Editing the number of approvers still does not "take" properly -- it's always "1" in the field on the merge-request-edit form, no matter what I enter and save.

reopened

(mistakenly re-opened by accident)

closed

(Also, NM, I see this is supposed to be in 8.17, not 8.16)

@gjunker fix of the issue that displayed wrong number on edit form (along with another related fix) should be included in the 8.16.2 release.

However please remember that you can never set count of approvals on a Merge Request < count of approvals set on project settings.

Thanks, I will look for it in 8.16.2.

Fortunately, our project-level approval count is 1. ;)

Also -- does this 8.16.2 fix also address the "Requires undefined more approvals" issue?

Should I do a new issue for this? With the "undefined" problem, there is no "approve" button...

And "Accept merge request" just says "problem on our end". I see this in the log (gitlab-ctl tail) when I press Accept:

==> /var/log/gitlab/gitlab-rails/production.log <==
Started POST "/<group redacted>/<project redacted>/merge_requests/365/merge" for 10.23.115.23 at 2017-01-26 15:14:17 -0800
Processing by Projects::MergeRequestsController#merge as JS
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"<hash redacted>==", "sha"=>"<sha redacted>", "merge_when_build_succeeds"=>"", "button"=>"", "namespace_id"=>"<group redacted>", "project_id"=>"<project redacted>", "id"=>"365"}
Completed 404 Not Found in 110ms (Views: 4.4ms | ActiveRecord: 13.2ms)

==> /var/log/gitlab/gitlab-workhorse/current <==
2017-01-26_23:14:17.29244 2017/01/26 15:14:17 ErrorPage: serving predefined error page: 404
2017-01-26_23:14:17.29266 <hostname redacted> @ - - [2017-01-26 15:14:17.164683858 -0800 PST] "POST /<group redacted>/<project redacted>/merge_requests/365/merge HTTP/1.1" 404 2680 "https://<hostname redacted>/<group redacted>/<project redacted>/merge_requests/365" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36" 0.127915

==> /var/log/gitlab/nginx/gitlab_access.log <==
10.23.115.23 - - [26/Jan/2017:15:14:17 -0800] "POST /<group redacted>/<project redacted>/merge_requests/365/merge HTTP/2.0" 404 3032 "https://<hostname redacted>/<group redacted>/<project redacted>/merge_requests/365" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36"

reopened

OK this is a huge problem now. There is no approval button on merge requests and now I cannot merge anything in (just get "error on our end").

This doesn't seem to happen in a project that had no branches or merge requests in it previously; I still have no approve button but I can at least merge in the UI.

@gjunker in general, please do open a separate issue for things like this. Issues work more like bug reports and feature requests than discussion threads, so we shouldn't have one issue per general topic, but rather one issue per specific problem.

In this specific case, I think you are having the same issue as https://gitlab.com/gitlab-org/gitlab-ee/issues/1575, which will be fixed by 8.16.2.

OK, will do. In this case, however, while I am having that issue, 8.16.2 (which I installed yesterday at support's suggestion) did not fix it.

closed

Editing approval count for existing MR doesn't work

Summary

Steps to reproduce

Expected behavior

Actual behavior

Results of GitLab application Check

Results of GitLab environment info

Designs

Child items ...

Activity

Admin message

Admin message

Editing approval count for existing MR doesn't work

Summary

Steps to reproduce

Expected behavior

Actual behavior

Results of GitLab application Check

Results of GitLab environment info

Activity