Check blocking_discussions_resolved in gitlab driver

In addition to `merge_status` attribute `blocking_discussions_resolved`
should be checked to know whether it makes sense to attempt merging.
In the project setting it is possible to enable "All discussions must be
resolved" check box what will result in the attribute to be set to false
once there are open discussions. With that (while merge_status is still
can_be_merged) the merge request can not be merged.

Sadly this is another badly documented case.

Change-Id: Iba3c7b424fb8acb3134622776eb1518ffddd5374

tests/base.py

@@ -2230,6 +2230,7 @@ class FakeGitlabMergeRequest(object):
             self.gitlab.server, self.project, self.number)
         self.base_sha = base_sha
         self.approved = False
+        self.blocking_discussions_resolved = True
         self.mr_ref = self._createMRRef(base_sha=base_sha)
         self._addCommitInMR(files=files)
 
@@ -2332,7 +2333,9 @@ class FakeGitlabMergeRequest(object):
                 'iid': self.number,
                 'target_branch': self.branch,
                 'last_commit': {'id': self.sha},
-                'action': action
+                'action': action,
+                'blocking_discussions_resolved':
+                    self.blocking_discussions_resolved
             },
         }
         data['labels'] = [{'title': label} for label in self.labels]

tests/fakegitlab.py

@@ -167,6 +167,8 @@ class GitlabWebServer(object):
                     mr.updated_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
                     'sha': mr.sha,
                     'labels': mr.labels,
+                    'blocking_discussions_resolved':
+                        mr.blocking_discussions_resolved,
                     'merged_at': mr.merged_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ')
                     if mr.merged_at else mr.merged_at,
                     'merge_status': mr.merge_status,

tests/unit/test_gitlab_driver.py

@@ -692,6 +692,28 @@ class TestGitlabDriver(ZuulTestCase):
         self.assertEqual(1, len(self.history))
         self.assertEqual(set(A.labels), {'addme1', 'addme2'})
 
+    @simple_layout('layouts/merging-gitlab.yaml', driver='gitlab')
+    def test_merge_blocking_discussions(self):
+
+        A = self.fake_gitlab.openFakeMergeRequest(
+            'org/project2', 'master', 'A')
+
+        A.blocking_discussions_resolved = False
+        self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        # canMerge is not validated
+        self.assertEqual(0, len(self.history))
+
+        A.blocking_discussions_resolved = True
+        self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        # canMerge is validated
+        self.assertEqual(1, len(self.history))
+
+        self.assertEqual('SUCCESS',
+                         self.getJobFromHistory('project-test').result)
+        self.assertEqual('merged', A.state)
+
     @simple_layout('layouts/merging-gitlab.yaml', driver='gitlab')
     def test_merge_action_in_independent(self):
 

zuul/driver/gitlab/gitlabconnection.py

@@ -732,6 +732,13 @@ class GitlabConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
         change.is_merged = change.mr['state'] == 'merged'
         # Can be "can_be_merged"
         change.merge_status = change.mr['merge_status']
+        # Blocking discussions - not properly documented parameter
+        # It is set to False when project settings enforce "all discussions
+        # must be resolved" and there are unresolved discussions.
+        # Gently try to get it for the case it is not present in some
+        # installations.
+        change.blocking_discussions_resolved = \
+            change.mr.get('blocking_discussions_resolved', True)
         change.approved = change.mr['approved']
         change.message = change.mr.get('description', "")
         change.labels = change.mr['labels']
@@ -759,9 +766,18 @@ class GitlabConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
 
     def canMerge(self, change, allow_needs, event=None):
         log = get_annotated_logger(self.log, event)
-        can_merge = True if change.merge_status == "can_be_merged" else False
-        log.info('Check MR %s#%s mergeability can_merge: %s',
-                 change.project.name, change.number, can_merge)
+        can_merge = False
+        if (
+            change.merge_status == "can_be_merged" and
+            change.blocking_discussions_resolved
+        ):
+            can_merge = True
+
+        log.info('Check MR %s#%s mergeability can_merge: %s'
+                 ' (merge status: %s, blocking discussions resolved: %s)',
+                 change.project.name, change.number, can_merge,
+                 change.merge_status, change.blocking_discussions_resolved)
+
         return can_merge
 
     def getMR(self, project_name, number, event=None):

zuul/driver/gitlab/gitlabmodel.py

@@ -26,6 +26,7 @@ class MergeRequest(Change):
         self.approved = None
         self.labels = None
         self.merge_status = None
+        self.blocking_discussions_resolved = None
         self.mr = None
         self.title = None
 
@@ -56,6 +57,8 @@ class MergeRequest(Change):
             "approved": self.approved,
             "labels": self.labels,
             "merge_status": self.merge_status,
+            "blocking_discussions_resolved":
+                self.blocking_discussions_resolved,
             "mr": self.mr,
             "title": self.title,
         })
@@ -67,6 +70,8 @@ class MergeRequest(Change):
         self.approved = data.get("approved")
         self.labels = data.get("labels")
         self.merge_status = data.get("merge_status")
+        self.blocking_discussions_resolved = data.get(
+            "blocking_discussions_resolved", True)
         self.mr = data.get("mr")
         self.title = data.get("title")