Fix openSUSE/open-build-service#8994

osclib/stagingapi.py

@@ -518,7 +518,9 @@ def superseded_request(self, request, target_requests=None):
                     # Supersedes request is from the same project
                     if request_new.find('./action/source').get('project') == request_old.find('./action/source').get('project'):
                         message = 'sr#{} has newer source and is from the same project'.format(request_new.get('id'))
-                        self.set_review(int(stage_info['rq_id']), stage_info['prj'], state='declined', msg=message)
+
+                        self.rm_from_prj(stage_info['prj'], request_id=stage_info['rq_id'])
+                        self.do_change_review_state(stage_info['rq_id'], 'declined', by_group=self.cstaging_group, message=message)
                         return stage_info, None
                     # Ingore the new request pending manual review.
                     IgnoreCommand(self).perform([str(request_id)], message)
@@ -547,9 +549,7 @@ def update_superseded_request(self, request, target_requests=None):
         stage_info, code = self.superseded_request(request, target_requests)
         if stage_info and (code is None or code == 'unstage'):
             # Remove the old request
-            self.rm_from_prj(stage_info['prj'],
-                             request_id=stage_info['rq_id'],
-                             msg='Replaced by sr#{}'.format(request_id))
+            self.rm_from_prj(stage_info['prj'], request_id=stage_info['rq_id'])
             if code is None:
                 # Add the new request that should be replacing the old one.
                 self.rq_to_prj(request_id, stage_info['prj'])
@@ -662,13 +662,11 @@ def get_package_for_request_id(self, project, request_id):
                 return x.get('package')
         return None
 
-    def rm_from_prj(self, project, package=None, request_id=None,
-                    msg=None):
+    def rm_from_prj(self, project, package=None, request_id=None):
         """
         Delete request from the project
         :param project: project to remove from
         :param request_id: request we want to remove
-        :param msg: message for the log
         :param review: review state for the review, defautl accepted
         """
 
@@ -1121,30 +1119,6 @@ def add_review(self, request_id, by_project=None, by_group=None, msg=None):
         url = self.makeurl(['request', str(request_id)], query)
         http_POST(url, data=msg)
 
-    def set_review(self, request_id, project, state='accepted', msg=None):
-        """
-        Sets review for request done by project
-        :param request_id: request to change review for
-        :param project: project to do the review
-        """
-        req = get_request(self.apiurl, str(request_id))
-        if not req:
-            raise oscerr.WrongArgs('Request {} not found'.format(request_id))
-        # don't try to change reviews if the request is dead
-        if req.state.name not in ('new', 'review'):
-            return
-        cont = False
-        for i in req.reviews:
-            if i.by_project == project and i.state == 'new':
-                cont = True
-        if not cont:
-            return
-        if not msg:
-            msg = 'Reviewed by staging project "{}" with result: "{}"'
-            msg = msg.format(project, state)
-        self.do_change_review_state(request_id, state, by_project=project,
-                                    message=msg)
-
     def get_flag_in_prj(self, project, flag='build', repository=None, arch=None):
         """Return the flag value in a project."""
         url = self.makeurl(['source', project, '_meta'])

osclib/supersede_command.py

@@ -26,4 +26,4 @@ def perform(self, requests=None):
                 Fore.CYAN + target_package + Fore.RESET,
                 verbage,
                 stage_info['rq_id'],
-                Fore.YELLOW + stage_info['prj']))
+                Fore.YELLOW + stage_info['prj'] + Fore.RESET))

osclib/unselect_command.py

@@ -53,7 +53,7 @@ def perform(self, packages, cleanup, message):
             staging_project = request_project['staging']
             affected_projects.add(staging_project)
             print('Unselecting "{}" from "{}"'.format(request, staging_project))
-            self.api.rm_from_prj(staging_project, request_id=request, msg='Removing from {}, re-evaluation needed'.format(staging_project))
+            self.api.rm_from_prj(staging_project, request_id=request)
 
             req = get_request(self.api.apiurl, str(request))
             if message:

tests/OBSLocal.py

@@ -227,6 +227,7 @@ def setup_remote_config(self):
 
         config = {
             'overridden-by-local': 'remote-nope',
+            'staging-group': 'factory-staging',
             'remote-only': 'remote-indeed',
         }
         self.remote_config_set(config, replace_all=True)
@@ -282,7 +283,8 @@ def create_user(self, name):
 
     def create_target(self):
         if self.projects.get('target'): return
-        self.create_group('factory-staging')
+        self.create_user('staging-bot')
+        self.create_group('factory-staging', users=['staging-bot'])
         self.projects['target'] = Project(name=self.project, reviewer={'groups': ['factory-staging']})
         url = osc.core.makeurl(APIURL, ['staging', self.project, 'workflow'])
         data = "<workflow managers='factory-staging'/>"
@@ -353,7 +355,7 @@ def create_staging(self, suffix, freeze=False, rings=None):
             project_links.append(self.project + ":Rings:0-Bootstrap")
         if rings == 1 or rings == 0:
             project_links.append(self.project + ":Rings:1-MinimalX")
-        staging.update_meta(project_links=project_links)
+        staging.update_meta(project_links=project_links, maintainer={'groups': ['factory-staging']})
 
         if freeze:
             FreezeCommand(self.api).perform(staging.name)

tests/api_tests.py

@@ -168,20 +168,6 @@ def test_review_handling(self):
         self.wf.api.add_review(num, by_project='openSUSE:Factory:Staging:B')
         self.assertEqual(request.reviews(), reviews)
 
-        # Accept review
-        self.wf.api.set_review(num, 'openSUSE:Factory:Staging:B')
-        reviews[1]['state'] = 'accepted'
-        self.assertEqual(request.reviews(), reviews)
-
-        # Try to accept it again should do anything
-        self.wf.api.set_review(num, 'openSUSE:Factory:Staging:B')
-        self.assertEqual(request.reviews(), reviews)
-
-        # But we should be able to reopen it
-        self.wf.api.add_review(num, by_project='openSUSE:Factory:Staging:B')
-        reviews.append({'by_project': 'openSUSE:Factory:Staging:B', 'state': 'new'})
-        self.assertEqual(request.reviews(), reviews)
-
     def test_prj_from_letter(self):
 
         # Verify it works

tests/select_tests.py

@@ -9,6 +9,7 @@
 from osclib.core import package_list
 from osclib.select_command import SelectCommand
 from osclib.unselect_command import UnselectCommand
+from osclib.supersede_command import SupersedeCommand
 from osclib.stagingapi import StagingAPI
 from osclib.memoize import memoize_session_reset
 from osclib.core import source_file_load
@@ -17,15 +18,16 @@
 from mock import MagicMock
 from . import OBSLocal
 
-class TestSelect(unittest.TestCase):
+class TestSelect(OBSLocal.TestCase):
 
     def setUp(self):
+        super().setUp()
         super(TestSelect, self).setUp()
         self.wf = OBSLocal.StagingWorkflow()
 
     def tearDown(self):
-        del self.wf
         super(TestSelect, self).tearDown()
+        del self.wf
 
     def test_old_frozen(self):
         self.wf.api.prj_frozen_enough = MagicMock(return_value=False)
@@ -99,3 +101,21 @@ def test_select_multibuild_package(self):
 
         # no stale links
         self.assertEqual([], package_list(self.wf.apiurl, staging.name))
+
+    def test_supersede(self):
+        self.wf.setup_rings()
+        staging = self.wf.create_staging('A', freeze=True)
+
+        rq1 = self.wf.create_submit_request('devel:wine', 'wine')
+        ret = SelectCommand(self.wf.api, staging.name).perform(['wine'])
+        rq2 = self.wf.create_submit_request('devel:wine', 'wine', text='Something new')
+        self.wf.api._packages_staged = None
+
+        self.osc_user('staging-bot')
+        Config.get(self.wf.apiurl, self.wf.project)
+
+        SupersedeCommand(self.wf.api).perform()
+
+        self.assertEqual(rq1.reviews(), [{'state': 'accepted', 'by_group': 'factory-staging'}, {'state': 'accepted', 'by_project': 'openSUSE:Factory:Staging:A'},
+                                    {'state': 'declined', 'by_group': 'factory-staging'}])
+        self.assertEqual(rq2.reviews(), [{'state': 'accepted', 'by_group': 'factory-staging'}, {'state': 'new', 'by_project': 'openSUSE:Factory:Staging:A'}])