Skip to content

feat(merge_request_approvals): add support for deleting MR approval rules #1888

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 13, 2022
Merged

feat(merge_request_approvals): add support for deleting MR approval rules #1888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69bc390
test(unit): clean up MR approvals fixtures
nejch Feb 10, 2022
25a4518
feat(merge_request_approvals): add support for deleting MR approval r…
nejch Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/gl_objects/merge_request_approvals.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ List MR-level MR approval rules::

mr.approval_rules.list()

Delete MR-level MR approval rule::

rules = mr.approval_rules.list()
rules[0].delete()

# or
mr.approval_rules.delete(approval_id)

Change MR-level MR approval rule::

mr_approvalrule.user_ids = [105]
Expand Down
4 changes: 2 additions & 2 deletions gitlab/v4/objects/merge_request_approvals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def set_approvers(
return approval_rules.create(data=data)


class ProjectMergeRequestApprovalRule(SaveMixin, RESTObject):
class ProjectMergeRequestApprovalRule(SaveMixin, ObjectDeleteMixin, RESTObject):
_id_attr = "approval_rule_id"
_short_print_attr = "approval_rule"
id: int
Expand Down Expand Up @@ -192,7 +192,7 @@ def save(self, **kwargs: Any) -> None:


class ProjectMergeRequestApprovalRuleManager(
ListMixin, UpdateMixin, CreateMixin, RESTManager
ListMixin, UpdateMixin, CreateMixin, DeleteMixin, RESTManager
):
_path = "/projects/{project_id}/merge_requests/{mr_iid}/approval_rules"
_obj_cls = ProjectMergeRequestApprovalRule
Expand Down
155 changes: 32 additions & 123 deletions tests/unit/objects/test_project_merge_request_approvals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,102 +24,7 @@


@pytest.fixture
def resp_snippet():
merge_request_content = [
{
"id": 1,
"iid": 1,
"project_id": 1,
"title": "test1",
"description": "fixed login page css paddings",
"state": "merged",
"merged_by": {
"id": 87854,
"name": "Douwe Maan",
"username": "DouweM",
"state": "active",
"avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
"web_url": "https://gitlab.com/DouweM",
},
"merged_at": "2018-09-07T11:16:17.520Z",
"closed_by": None,
"closed_at": None,
"created_at": "2017-04-29T08:46:00Z",
"updated_at": "2017-04-29T08:46:00Z",
"target_branch": "main",
"source_branch": "test1",
"upvotes": 0,
"downvotes": 0,
"author": {
"id": 1,
"name": "Administrator",
"username": "admin",
"state": "active",
"avatar_url": None,
"web_url": "https://gitlab.example.com/admin",
},
"assignee": {
"id": 1,
"name": "Administrator",
"username": "admin",
"state": "active",
"avatar_url": None,
"web_url": "https://gitlab.example.com/admin",
},
"assignees": [
{
"name": "Miss Monserrate Beier",
"username": "axel.block",
"id": 12,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon",
"web_url": "https://gitlab.example.com/axel.block",
}
],
"source_project_id": 2,
"target_project_id": 3,
"labels": ["Community contribution", "Manage"],
"work_in_progress": None,
"milestone": {
"id": 5,
"iid": 1,
"project_id": 3,
"title": "v2.0",
"description": "Assumenda aut placeat expedita exercitationem labore sunt enim earum.",
"state": "closed",
"created_at": "2015-02-02T19:49:26.013Z",
"updated_at": "2015-02-02T19:49:26.013Z",
"due_date": "2018-09-22",
"start_date": "2018-08-08",
"web_url": "https://gitlab.example.com/my-group/my-project/milestones/1",
},
"merge_when_pipeline_succeeds": None,
"merge_status": "can_be_merged",
"sha": "8888888888888888888888888888888888888888",
"merge_commit_sha": None,
"squash_commit_sha": None,
"user_notes_count": 1,
"discussion_locked": None,
"should_remove_source_branch": True,
"force_remove_source_branch": False,
"allow_collaboration": False,
"allow_maintainer_to_push": False,
"web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1",
"references": {
"short": "!1",
"relative": "my-group/my-project!1",
"full": "my-group/my-project!1",
},
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
"human_time_estimate": None,
"human_total_time_spent": None,
},
"squash": False,
"task_completion_status": {"count": 0, "completed_count": 0},
}
]
def resp_mr_approval_rules():
mr_ars_content = [
{
"id": approval_rule_id,
Expand Down Expand Up @@ -188,20 +93,6 @@ def resp_snippet():
}

with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps:
rsps.add(
method=responses.GET,
url="http://localhost/api/v4/projects/1/merge_requests",
json=merge_request_content,
content_type="application/json",
status=200,
)
rsps.add(
method=responses.GET,
url="http://localhost/api/v4/projects/1/merge_requests/1",
json=merge_request_content[0],
content_type="application/json",
status=200,
)
rsps.add(
method=responses.GET,
url="http://localhost/api/v4/projects/1/merge_requests/1/approval_rules",
Expand Down Expand Up @@ -248,7 +139,20 @@ def resp_snippet():
yield rsps


def test_project_approval_manager_update_uses_post(project, resp_snippet):
@pytest.fixture
def resp_delete_mr_approval_rule(no_content):
with responses.RequestsMock() as rsps:
rsps.add(
method=responses.DELETE,
url="http://localhost/api/v4/projects/1/merge_requests/1/approval_rules/1",
json=no_content,
content_type="application/json",
status=204,
)
yield rsps


def test_project_approval_manager_update_uses_post(project):
"""Ensure the
gitlab.v4.objects.merge_request_approvals.ProjectApprovalManager object has
_update_uses_post set to True"""
Expand All @@ -259,15 +163,20 @@ def test_project_approval_manager_update_uses_post(project, resp_snippet):
assert approvals._update_uses_post is True


def test_list_merge_request_approval_rules(project, resp_snippet):
approval_rules = project.mergerequests.get(1).approval_rules.list()
def test_list_merge_request_approval_rules(project, resp_mr_approval_rules):
approval_rules = project.mergerequests.get(1, lazy=True).approval_rules.list()
assert len(approval_rules) == 1
assert approval_rules[0].name == approval_rule_name
assert approval_rules[0].id == approval_rule_id


def test_update_merge_request_approvals_set_approvers(project, resp_snippet):
approvals = project.mergerequests.get(1).approvals
def test_delete_merge_request_approval_rule(project, resp_delete_mr_approval_rule):
merge_request = project.mergerequests.get(1, lazy=True)
merge_request.approval_rules.delete(approval_rule_id)


def test_update_merge_request_approvals_set_approvers(project, resp_mr_approval_rules):
approvals = project.mergerequests.get(1, lazy=True).approvals
assert isinstance(
approvals,
gitlab.v4.objects.merge_request_approvals.ProjectMergeRequestApprovalManager,
Expand All @@ -286,8 +195,8 @@ def test_update_merge_request_approvals_set_approvers(project, resp_snippet):
assert response.name == approval_rule_name


def test_create_merge_request_approvals_set_approvers(project, resp_snippet):
approvals = project.mergerequests.get(1).approvals
def test_create_merge_request_approvals_set_approvers(project, resp_mr_approval_rules):
approvals = project.mergerequests.get(1, lazy=True).approvals
assert isinstance(
approvals,
gitlab.v4.objects.merge_request_approvals.ProjectMergeRequestApprovalManager,
Expand All @@ -305,8 +214,8 @@ def test_create_merge_request_approvals_set_approvers(project, resp_snippet):
assert response.name == new_approval_rule_name


def test_create_merge_request_approval_rule(project, resp_snippet):
approval_rules = project.mergerequests.get(1).approval_rules
def test_create_merge_request_approval_rule(project, resp_mr_approval_rules):
approval_rules = project.mergerequests.get(1, lazy=True).approval_rules
data = {
"name": new_approval_rule_name,
"approvals_required": new_approval_rule_approvals_required,
Expand All @@ -321,8 +230,8 @@ def test_create_merge_request_approval_rule(project, resp_snippet):
assert response.name == new_approval_rule_name


def test_update_merge_request_approval_rule(project, resp_snippet):
approval_rules = project.mergerequests.get(1).approval_rules
def test_update_merge_request_approval_rule(project, resp_mr_approval_rules):
approval_rules = project.mergerequests.get(1, lazy=True).approval_rules
ar_1 = approval_rules.list()[0]
ar_1.user_ids = updated_approval_rule_user_ids
ar_1.approvals_required = updated_approval_rule_approvals_required
Expand All @@ -333,8 +242,8 @@ def test_update_merge_request_approval_rule(project, resp_snippet):
assert ar_1.eligible_approvers[0]["id"] == updated_approval_rule_user_ids[0]


def test_get_merge_request_approval_state(project, resp_snippet):
merge_request = project.mergerequests.get(1)
def test_get_merge_request_approval_state(project, resp_mr_approval_rules):
merge_request = project.mergerequests.get(1, lazy=True)
approval_state = merge_request.approval_state.get()
assert isinstance(
approval_state,
Expand Down