From 75226473fc1cacedada7395abf31d79adb48538f Mon Sep 17 00:00:00 2001
From: Josef Reidinger <jreidinger@suse.cz>
Date: Mon, 26 Jul 2021 22:21:06 +0200
Subject: [PATCH] Avoid creation of duplicate add_role requests (fixes #2609)

---
 check_source.py             | 22 +++++++++++++++++++---
 tests/check_source_tests.py | 23 +++++++++++++++++------
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/check_source.py b/check_source.py
index 157a0bcc..bc296225 100755
--- a/check_source.py
+++ b/check_source.py
@@ -25,6 +25,7 @@ from osclib.core import source_file_load
 from osclib.core import target_archs
 from osclib.core import create_add_role_request
 from osc.core import show_project_meta
+from osc.core import get_request_list
 from urllib.error import HTTPError
 
 import ReviewBot
@@ -171,9 +172,15 @@ class CheckSource(ReviewBot.ReviewBot):
             )
 
             try:
-                add_role_msg = 'Created automatically from request %s' % self.request.reqid
-                add_role_reqid = create_add_role_request(self.apiurl, source_project, self.required_maintainer,
-                                                         'maintainer', message=add_role_msg)
+                add_roles = get_request_list(self.apiurl, source_project,
+                    req_state=['new', 'review'], req_type='add_role')
+                add_roles = list(filter(self.select_add_role, add_roles))
+                if len(add_roles) > 0:
+                    add_role_reqid = add_roles[0].reqid
+                else:
+                    add_role_msg = 'Created automatically from request %s' % self.request.reqid
+                    add_role_reqid = create_add_role_request(self.apiurl, source_project, self.required_maintainer,
+                                                            'maintainer', message=add_role_msg)
                 declined_msg += ' Created the add_role request %s for addressing this problem.' % add_role_reqid
             except HTTPError as e:
                 self.logger.error(
@@ -356,6 +363,15 @@ class CheckSource(ReviewBot.ReviewBot):
 
         return self.required_maintainer in maintainers
 
+    def select_add_role(self, request):
+        action = request.actions[0]
+        user = self.required_maintainer
+        if user.startswith('group:'):
+            group = user.replace('group:', '')
+            return action.group_name == group and action.group_role == 'maintainer'
+        else:
+            return action.person_name == user and action.person_role == 'maintainer'
+
     @staticmethod
     def checkout_package(*args, **kwargs):
         _stdout = sys.stdout
diff --git a/tests/check_source_tests.py b/tests/check_source_tests.py
index bd4919c4..a8bb6b2d 100644
--- a/tests/check_source_tests.py
+++ b/tests/check_source_tests.py
@@ -80,28 +80,39 @@ class TestCheckSource(OBSLocal.TestCase):
         self.assertReview(req_id, by_group=(REVIEW_TEAM, 'new'))
 
     def test_no_source_maintainer(self):
-        """Declines the request when the 'required_maintainer' is not maintainer of the source project"""
+        """Declines the request when the 'required_maintainer' is not maintainer of the source project
+
+          Create also request to add required maintainers to source project unless it is already open
+        """
         self._setup_devel_project()
 
         # Change the required maintainer
         self.wf.create_group(FACTORY_MAINTAINERS.replace('group:', ''))
         self.wf.remote_config_set({ 'required-source-maintainer': FACTORY_MAINTAINERS })
 
-        req_id = self.wf.create_submit_request(SRC_PROJECT, 'blowfish').reqid
+        req = self.wf.create_submit_request(SRC_PROJECT, 'blowfish')
 
-        self.assertReview(req_id, by_user=(self.bot_user, 'new'))
+        self.assertReview(req.reqid, by_user=(self.bot_user, 'new'))
 
-        self.review_bot.set_request_ids([req_id])
+        self.review_bot.set_request_ids([req.reqid])
         self.review_bot.check_requests()
 
-        review = self.assertReview(req_id, by_user=(self.bot_user, 'declined'))
+        review = self.assertReview(req.reqid, by_user=(self.bot_user, 'declined'))
         add_role_req = get_request_list(self.wf.apiurl, SRC_PROJECT, req_state=['new'], req_type='add_role')[0]
 
         self.assertIn('unless %s is a maintainer of %s' % (FACTORY_MAINTAINERS, SRC_PROJECT), review.comment)
         self.assertIn('Created the add_role request %s' % add_role_req.reqid, review.comment)
 
         self.assertEqual(add_role_req.actions[0].tgt_project, SRC_PROJECT)
-        self.assertEqual('Created automatically from request %s' % req_id, add_role_req.description)
+        self.assertEqual('Created automatically from request %s' % req.reqid, add_role_req.description)
+
+        # reopen request and do it again to test that new add_role request won't be created
+        req.change_state('new')
+
+        self.review_bot.check_requests()
+        add_role_reqs = get_request_list(self.wf.apiurl, SRC_PROJECT, req_state=['new'], req_type='add_role')
+
+        self.assertEqual(len(add_role_reqs), 1)
 
     def test_source_maintainer(self):
         """Accepts the request when the 'required_maintainer' is a group and is a maintainer for the project"""