diff --git a/marge/job.py b/marge/job.py index 82f1bd84..1f7c38ba 100644 --- a/marge/job.py +++ b/marge/job.py @@ -5,6 +5,9 @@ from collections import namedtuple from datetime import datetime, timedelta +from marge.member import Member +from marge.protected_branch import ProtectedBranch + from . import git, gitlab from .branch import Branch from .interval import IntervalUnion @@ -44,6 +47,24 @@ def ensure_mergeable_mr(self, merge_request): log.info('Ensuring MR !%s is mergeable', merge_request.iid) log.debug('Ensuring MR %r is mergeable', merge_request) + # Zero means that the MR wasn't fetched using `fetch_all_open_for_user`, + # in this case we have no straighforward way to check the user who assigned us + # or we created the MR ourselfes + if merge_request.assigned_author != 0: + try: + assigned_author_member = Member.fetch_by_id(self.project.id, merge_request.assigned_author, self._api) + except gitlab.NotFound as ex: + raise CannotMerge("User that assigned me is not a project member!") + + try: + protected_branch = ProtectedBranch.fetch_by_name(self.project.id, merge_request.target_branch, self._api) + merge_min_access_level = protected_branch.merge_min_access_level + if merge_min_access_level > assigned_author_member.access_level: + raise CannotMerge("User that assigned me is not allowed to merge on the target branch!") + except gitlab.NotFound as ex: + # branch isn't protected + pass + if merge_request.work_in_progress: raise CannotMerge("Sorry, I can't merge requests marked as Work-In-Progress!") diff --git a/marge/member.py b/marge/member.py new file mode 100644 index 00000000..e6f2caf0 --- /dev/null +++ b/marge/member.py @@ -0,0 +1,16 @@ +from . import gitlab + + +GET = gitlab.GET + + +class Member(gitlab.Resource): + + @classmethod + def fetch_by_id(cls, project_id, member_id, api): + info = api.call(GET('/projects/%s/members/all/%s' % (project_id, member_id))) + return cls(api, info) + + @property + def access_level(self): + return self.info['access_level'] diff --git a/marge/merge_request.py b/marge/merge_request.py index 030b79b8..13c06c40 100644 --- a/marge/merge_request.py +++ b/marge/merge_request.py @@ -11,6 +11,9 @@ class MergeRequest(gitlab.Resource): + def __init__(self, api, info, assigned_author): + super().__init__(api=api, info=info) + self._assigned_author = assigned_author @classmethod def create(cls, api, project_id, params): @@ -18,7 +21,7 @@ def create(cls, api, project_id, params): '/projects/{project_id}/merge_requests'.format(project_id=project_id), params, )) - merge_request = cls(api, merge_request_info) + merge_request = cls(api, merge_request_info, 0) return merge_request @classmethod @@ -27,16 +30,17 @@ def search(cls, api, project_id, params): '/projects/{project_id}/merge_requests'.format(project_id=project_id), params, )) - return [cls(api, merge_request) for merge_request in merge_requests] + return [cls(api, merge_request, 0) for merge_request in merge_requests] @classmethod def fetch_by_iid(cls, project_id, merge_request_iid, api): - merge_request = cls(api, {'iid': merge_request_iid, 'project_id': project_id}) + merge_request = cls(api, {'iid': merge_request_iid, 'project_id': project_id}, 0) merge_request.refetch_info() return merge_request @classmethod - def fetch_assigned_at(cls, user, api, merge_request): + def fetch_assigned_author_and_at(cls, user, api, merge_request): + assigned_author_id = 0 assigned_at = 0 all_discussions = api.collect_all_pages( GET('/projects/{project_id}/merge_requests/{merge_requests_id}/discussions'.format( @@ -56,7 +60,8 @@ def fetch_assigned_at(cls, user, api, merge_request): assigned = datetime.datetime.strptime(date_string, date_format).timestamp() if assigned > assigned_at: assigned_at = assigned - return assigned_at + assigned_author_id = note['author']['id'] + return [assigned_author_id, assigned_at] @classmethod def fetch_all_open_for_user(cls, project_id, user, api, merge_order): @@ -72,10 +77,15 @@ def fetch_all_open_for_user(cls, project_id, user, api, merge_order): (user.id in [assignee.get('id') for assignee in (mri.get('assignees', []) or [])]) ] + for mri in my_merge_request_infos: + assigned_author_and_at = cls.fetch_assigned_author_and_at(user, api, mri) + mri['assigned_author'] = assigned_author_and_at[0] + mri['assigned_at'] = assigned_author_and_at[1] + if merge_order == 'assigned_at': - my_merge_request_infos.sort(key=lambda mri: cls.fetch_assigned_at(user, api, mri)) + my_merge_request_infos.sort(key=lambda mri: mri['assigned_at']) - return [cls(api, merge_request_info) for merge_request_info in my_merge_request_infos] + return [cls(api, merge_request_info, merge_request_info['assigned_author']) for merge_request_info in my_merge_request_infos] @property def project_id(self): @@ -159,6 +169,10 @@ def blocking_discussions_resolved(self): def force_remove_source_branch(self): return self.info['force_remove_source_branch'] + @property + def assigned_author(self): + return self._assigned_author + def update_sha(self, sha): """record the updated sha. We don't use refetch_info instead as it may hit cache.""" self._info['sha'] = sha diff --git a/marge/protected_branch.py b/marge/protected_branch.py new file mode 100644 index 00000000..3594b13e --- /dev/null +++ b/marge/protected_branch.py @@ -0,0 +1,21 @@ +import sys +from . import gitlab + + +GET = gitlab.GET + + +class ProtectedBranch(gitlab.Resource): + + @classmethod + def fetch_by_name(cls, project_id, branch_name, api): + info = api.call(GET('/projects/%s/protected_branches/%s' % (project_id, branch_name))) + return cls(api, info) + + @property + def merge_min_access_level(self): + min_access_level = 60 # instance admin + for level in self.info['merge_access_levels']: + if level['access_level'] < min_access_level: + min_access_level = level['access_level'] + return min_access_level diff --git a/tests/test_merge_request.py b/tests/test_merge_request.py index 7259ea72..d039330b 100644 --- a/tests/test_merge_request.py +++ b/tests/test_merge_request.py @@ -30,8 +30,8 @@ DISCUSSION = { 'id': 'aabbcc0044', 'notes': [ - {'id': 12, "body": "assigned to @john_smith", "created_at": "2020-08-04T06:56:11.854Z"}, - {'id': 13, "body": "assigned to @john_smith", "created_at": "2020-08-18T06:52:58.093Z"} + {'id': 12, 'author': {'id': 42}, "body": "assigned to @john_smith", "created_at": "2020-08-04T06:56:11.854Z"}, + {'id': 13, 'author': {'id': 69}, "body": "assigned to @john_smith", "created_at": "2020-08-18T06:52:58.093Z"} ], } @@ -214,19 +214,19 @@ def test_fetch_all_opened_for_me(self): )) assert [mr.info for mr in result] == [mr1, mr2] - def test_fetch_assigned_at(self): + def fetch_assigned_author_and_at(self): api = self.api dis1, dis2 = DISCUSSION, dict(DISCUSSION, id=679) mr1 = INFO user = marge.user.User(api=None, info=dict(USER_INFO, id=_MARGE_ID)) api.collect_all_pages = Mock(return_value=[dis1, dis2]) - result = MergeRequest.fetch_assigned_at( + result = MergeRequest.fetch_assigned_author_and_at( user=user, api=api, merge_request=mr1 ) api.collect_all_pages.assert_called_once_with(GET( '/projects/1234/merge_requests/54/discussions', )) - assert result == 1597733578.093 + assert result == [69, 1597733578.093] def _load(self, json): old_mock = self.api.call