Remove version checks for ancient GitLab versions (#61)

marge/approvals.py

@@ -7,18 +7,9 @@ class Approvals(gitlab.Resource):
     """Approval info for a MergeRequest."""
 
     def refetch_info(self):
-        gitlab_version = self._api.version()
-        if gitlab_version.release >= (9, 2, 2):
-            approver_url = f'/projects/{self.project_id}/merge_requests/{self.iid}/approvals'
-        else:
-            # GitLab botched the v4 api before 9.2.3
-            approver_url = f'/projects/{self.project_id}/merge_requests/{self.id}/approvals'
-
-        # Approvals are in CE since 13.2
-        if gitlab_version.is_ee or gitlab_version.release >= (13, 2, 0):
-            self._info = self._api.call(GET(approver_url))
-        else:
-            self._info = dict(self._info, approvals_left=0, approved_by=[])
+        approver_url = f'/projects/{self.project_id}/merge_requests/{self.iid}/approvals'
+
+        self._info = self._api.call(GET(approver_url))
 
     @property
     def iid(self):
@@ -60,11 +51,7 @@ def reapprove(self):
 
     def approve(self, obj):
         """Approve an object which can be a merge_request or an approval."""
-        if self._api.version().release >= (9, 2, 2):
-            approve_url = f'/projects/{obj.project_id}/merge_requests/{obj.iid}/approve'
-        else:
-            # GitLab botched the v4 api before 9.2.3
-            approve_url = f'/projects/{obj.project_id}/merge_requests/{obj.id}/approve'
+        approve_url = f'/projects/{obj.project_id}/merge_requests/{obj.iid}/approve'
 
         for uid in self.approver_ids:
             self._api.call(POST(approve_url), sudo=uid)

marge/job.py

@@ -142,18 +142,11 @@ def get_mr_ci_status(self, merge_request, commit_sha=None):
         if commit_sha is None:
             commit_sha = merge_request.sha
 
-        if self._api.version().release >= (10, 5, 0):
-            pipelines = Pipeline.pipelines_by_merge_request(
-                merge_request.target_project_id,
-                merge_request.iid,
-                self._api,
-            )
-        else:
-            pipelines = Pipeline.pipelines_by_branch(
-                merge_request.source_project_id,
-                merge_request.source_branch,
-                self._api,
-            )
+        pipelines = Pipeline.pipelines_by_merge_request(
+            merge_request.target_project_id,
+            merge_request.iid,
+            self._api,
+        )
 
         current_pipeline = next(iter(pipeline for pipeline in pipelines if pipeline.sha == commit_sha), None)
         log.debug('Current pipeline: %s', current_pipeline)

marge/merge_request.py

@@ -176,11 +176,7 @@ def refetch_info(self):
         ))
 
     def comment(self, message):
-        if self._api.version().release >= (9, 2, 2):
-            notes_url = f'/projects/{self.project_id}/merge_requests/{self.iid}/notes'
-        else:
-            # GitLab botched the v4 api before 9.2.2
-            notes_url = f'/projects/{self.project_id}/merge_requests/{self.id}/notes'
+        notes_url = f'/projects/{self.project_id}/merge_requests/{self.iid}/notes'
 
         return self._api.call(POST(notes_url, {'body': message}))
 

marge/project.py

@@ -1,4 +1,3 @@
-import logging as log
 from enum import IntEnum, unique
 from functools import partial
 
@@ -35,35 +34,18 @@ def fetch_all_mine(cls, api):
         # GitLab has an issue where projects may not show appropriate permissions in nested groups. Using
         # `min_access_level` is known to provide the correct projects, so we'll prefer this method
         # if it's available. See #156 for more details.
-        use_min_access_level = api.version().release >= (11, 2)
-        if use_min_access_level:
-            projects_kwargs["min_access_level"] = int(AccessLevel.developer)
+        projects_kwargs["min_access_level"] = int(AccessLevel.developer)
 
         projects_info = api.collect_all_pages(GET(
             '/projects',
             projects_kwargs,
         ))
 
-        def project_seems_ok(project_info):
-            # A bug in at least GitLab 9.3.5 would make GitLab not report permissions after
-            # moving subgroups. See for full story #19.
-            permissions = project_info['permissions']
-            permissions_ok = bool(permissions['project_access'] or permissions['group_access'])
-            if not permissions_ok:
-                project_name = project_info['path_with_namespace']
-                log.warning('Ignoring project %s since GitLab provided no user permissions', project_name)
-
-            return permissions_ok
-
         projects = []
-
         for project_info in projects_info:
-            if use_min_access_level:
-                # We know we fetched projects with at least developer access, so we'll use that as
-                # a fallback if GitLab doesn't correctly report permissions as described above.
-                project_info["permissions"]["marge"] = {"access_level": AccessLevel.developer}
-            elif not project_seems_ok(projects_info):
-                continue
+            # We know we fetched projects with at least developer access, so we'll use that as
+            # a fallback if GitLab doesn't correctly report permissions as described above.
+            project_info["permissions"]["marge"] = {"access_level": AccessLevel.developer}
 
             projects.append(cls(api, project_info))
 

tests/gitlab_api_mock.py

@@ -196,12 +196,9 @@ def add_approvals(self, info, sudo=None, from_state=None, to_state=None):
         path = '/projects/{0.project_id}/merge_requests/{0.iid}/approvals'
         self.add_resource(path, info, sudo, from_state, to_state)
 
-    def add_pipelines(self, project_id, info, sudo=None, from_state=None, to_state=None):
+    def add_pipelines(self, project_id, merge_request_iid, info, sudo=None, from_state=None, to_state=None):
         self.add_transition(
-            GET(
-                f'/projects/{project_id}/pipelines',
-                args={'ref': info['ref'], 'order_by': 'id', 'sort': 'desc'},
-            ),
+            GET(f'/projects/{project_id}/merge_requests/{merge_request_iid}/pipelines'),
             Ok([info]),
             sudo, from_state, to_state,
         )

tests/test_approvals.py

@@ -68,7 +68,7 @@ class TestApprovals:
 
     def setup_method(self, _method):
         self.api = Mock(Api)
-        self.api.version = Mock(return_value=Version.parse('9.2.3-ee'))
+        self.api.version = Mock(return_value=Version.parse('13.3.1-ee'))
         self.approvals = Approvals(api=self.api, info=INFO)
 
     def test_fetch_from_merge_request(self):
@@ -83,19 +83,6 @@ def test_fetch_from_merge_request(self):
         ))
         assert approvals.info == INFO
 
-    def test_fetch_from_merge_request_ce_compat(self):
-        api = self.api
-        api.version = Mock(return_value=Version.parse('9.2.3'))
-        api.call = Mock()
-
-        merge_request = MergeRequest(api, {'id': 74, 'iid': 6, 'project_id': 1234})
-        approvals = merge_request.fetch_approvals()
-
-        api.call.assert_not_called()
-        assert approvals.info == {
-            'id': 74, 'iid': 6, 'project_id': 1234, 'approvals_left': 0, 'approved_by': [],
-        }
-
     def test_properties(self):
         assert self.approvals.project_id == 1
         assert self.approvals.approvals_left == 1

tests/test_job.py

@@ -55,7 +55,7 @@ def test_get_source_project_when_is_fork(self):
 
     @pytest.mark.parametrize(
         'version,use_merge_request_pipelines',
-        [('9.4.0-ee', False), ('10.5.0-ee', True)],
+        [('10.5.0-ee', True)],
     )
     def test_get_mr_ci_status(self, version, use_merge_request_pipelines):
         with patch('marge.job.Pipeline', autospec=True) as pipeline_class:

tests/test_project.py

@@ -63,24 +63,6 @@ def test_fetch_by_path_exists(self):
         api.collect_all_pages.assert_called_once_with(GET('/projects'))
         assert project and project.info == prj2
 
-    def fetch_all_mine_with_permissions(self):
-        prj1, prj2 = INFO, dict(INFO, id=678)
-
-        api = self.api
-        api.collect_all_pages = Mock(return_value=[prj1, prj2])
-        api.version = Mock(return_value=Version.parse("11.0.0-ee"))
-
-        result = Project.fetch_all_mine(api)
-        api.collect_all_pages.assert_called_once_with(GET(
-            '/projects',
-            {
-                'membership': True,
-                'with_merge_requests_enabled': True,
-            },
-        ))
-        assert [prj.info for prj in result] == [prj1, prj2]
-        assert all(prj.access_level == AccessLevel.developer for prj in result)
-
     def fetch_all_mine_with_min_access_level(self):
         prj1, prj2 = dict(INFO, permissions=NONE_ACCESS), dict(INFO, id=678, permissions=NONE_ACCESS)