Merge branch 'develop' of github.com:fedora-infra/bodhi into develop

bodhi/models/models.py

@@ -1386,41 +1386,48 @@ def comment(self, session, text, karma=0, author=None, anonymous=False,
                 notice = 'You may not give karma to your own updates.'
                 caveats.append({'name': 'karma', 'description': notice})
 
-        if not anonymous and karma != 0 and \
-           not filter(lambda c: c.user.name == author and c.karma == karma,
-                      self.comments):
-            mycomments = [
-                c.karma for c in self.comments if c.user.name == author]
-            if karma == 1 and -1 in mycomments:
-                self.karma += 2
-                caveats.append({
-                    'name': 'karma',
-                    'description': 'Your karma standing was reversed.',
-                })
-            elif karma == -1 and 1 in mycomments:
-                self.karma -= 2
-                caveats.append({
-                    'name': 'karma',
-                    'description': 'Your karma standing was reversed.',
-                })
-            else:
-                self.karma += karma
-
-            log.info("Updated %s karma to %d" % (self.title, self.karma))
-
-            if check_karma and author not in config.get('system_users').split():
-                try:
-                    self.check_karma_thresholds(session, 'bodhi')
-                except LockedUpdateException:
-                    pass
-                except BodhiException as e:
-                    # This gets thrown if the karma is pushed over the
-                    # threshold, but it is a critpath update that is not
-                    # critpath_approved. ... among other cases.
-                    log.exception('Problem checking the karma threshold.')
+        if not anonymous and karma != 0:
+            # Take all comments since the previous karma reset
+            reset_index = 0
+            for i, comment in enumerate(self.comments):
+                if (comment.user.name == u'bodhi' and ('New build' in
+                    comment.text or 'Removed build' in comment.text)):
+                    reset_index = i
+
+            mycomments = [c.karma for c in self.comments if c.user.name == author][reset_index:]
+            if karma not in mycomments:
+                if karma == 1 and -1 in mycomments:
+                    self.karma += 2
                     caveats.append({
-                        'name': 'karma', 'description': str(e),
+                        'name': 'karma',
+                        'description': 'Your karma standing was reversed.',
                     })
+                elif karma == -1 and 1 in mycomments:
+                    self.karma -= 2
+                    caveats.append({
+                        'name': 'karma',
+                        'description': 'Your karma standing was reversed.',
+                    })
+                else:
+                    self.karma += karma
+
+                log.info("Updated %s karma to %d" % (self.title, self.karma))
+
+                if check_karma and author not in config.get('system_users').split():
+                    try:
+                        self.check_karma_thresholds(session, 'bodhi')
+                    except LockedUpdateException:
+                        pass
+                    except BodhiException as e:
+                        # This gets thrown if the karma is pushed over the
+                        # threshold, but it is a critpath update that is not
+                        # critpath_approved. ... among other cases.
+                        log.exception('Problem checking the karma threshold.')
+                        caveats.append({
+                            'name': 'karma', 'description': str(e),
+                        })
+            else:
+                log.debug('Ignoring duplicate %d karma from %s on %s' % (karma, author, self.title))
 
         comment = Comment(
             text=text, anonymous=anonymous,

bodhi/tests/functional/test_updates.py

@@ -2034,3 +2034,64 @@ def test_edit_testing_update_reset_karma(self, publish, *args):
         self.assertEquals(up['title'], u'bodhi-2.0.0-3.fc17')
         # This is what we really want to test here.
         self.assertEquals(up['karma'], 0)
+
+    @mock.patch(**mock_valid_requirements)
+    @mock.patch('bodhi.notifications.publish')
+    def test_edit_testing_update_reset_karma_with_same_tester(self, publish, *args):
+        """
+        Ensure that someone who gave an update karma can do it again after a reset.
+        https://github.com/fedora-infra/bodhi/issues/659
+        """
+        user = User(name=u'bob')
+        self.db.add(user)
+        self.db.flush()
+
+        nvr = u'bodhi-2.0.0-2.fc17'
+        args = self.get_update(nvr)
+        r = self.app.post_json('/updates/', args)
+        publish.assert_called_with(topic='update.request.testing', msg=ANY)
+
+        # Mark it as testing
+        upd = Update.get(nvr, self.db)
+        upd.status = UpdateStatus.testing
+        upd.request = None
+        self.db.flush()
+
+        # Have bob +1 it
+        upd.comment(self.db, u'LGTM', author=u'bob', karma=1)
+        upd = Update.get(nvr, self.db)
+        self.assertEquals(upd.karma, 1)
+
+        # Then.. edit it and change the builds!
+        new_nvr = u'bodhi-2.0.0-3.fc17'
+        args['edited'] = args['builds']
+        args['builds'] = new_nvr
+        r = self.app.post_json('/updates/', args)
+        up = r.json_body
+        self.assertEquals(up['title'], new_nvr)
+        # This is what we really want to test here.
+        self.assertEquals(up['karma'], 0)
+
+        # Have bob +1 it again
+        upd = Update.get(new_nvr, self.db)
+        upd.comment(self.db, u'Ship it!', author=u'bob', karma=1)
+
+        # Bob should be able to give karma again since the reset
+        self.assertEquals(upd.karma, 1)
+
+        # Then.. edit it and change the builds!
+        newer_nvr = u'bodhi-2.0.0-4.fc17'
+        args['edited'] = args['builds']
+        args['builds'] = newer_nvr
+        r = self.app.post_json('/updates/', args)
+        up = r.json_body
+        self.assertEquals(up['title'], newer_nvr)
+        # This is what we really want to test here.
+        self.assertEquals(up['karma'], 0)
+
+        # Have bob +1 it again
+        upd = Update.get(newer_nvr, self.db)
+        upd.comment(self.db, u'Ship it!', author=u'bob', karma=1)
+
+        # Bob should be able to give karma again since the reset
+        self.assertEquals(upd.karma, 1)