Add deprecation warnings to karma_critpath

Signed-off-by: Mattia Verga <[email protected]>

bodhi-server/bodhi/server/models.py

@@ -3545,7 +3545,7 @@ def update_bugs(self, bug_ids, session):
         session.flush()
         return new
 
-    def comment(self, session, text, karma=0, author=None, karma_critpath=0,
+    def comment(self, session, text, karma=0, author=None, karma_critpath=None,
                 bug_feedback=None, testcase_feedback=None, email_notification=True):
         """Add a comment to this update.
 
@@ -3555,6 +3555,12 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
         if not author:
             raise ValueError('You must provide a comment author')
 
+        if karma_critpath:
+            warnings.warn(
+                "karma_critpath is not used anymore and should not be passed in comment() call; "
+                "date=2024-11-16", DeprecationWarning, stacklevel=2
+            )
+
         # Listify these
         bug_feedback = bug_feedback or []
         testcase_feedback = testcase_feedback or []
@@ -3565,7 +3571,7 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
                 got_feedback = True
                 break
 
-        if (not text and not karma and not karma_critpath and not got_feedback):
+        if (not text and not karma and not got_feedback):
             raise ValueError('You must provide either some text or feedback')
 
         caveats = []
@@ -3582,8 +3588,7 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
             user = User(name=author)
             session.add(user)
 
-        comment = Comment(text=text, karma=karma, karma_critpath=karma_critpath,
-                          update=self, user=user)
+        comment = Comment(text=text, karma=karma, update=self, user=user)
         session.add(comment)
 
         if karma != 0:
@@ -4503,7 +4508,7 @@ class Comment(Base):
     Attributes:
         karma (int): The karma associated with this comment. Defaults to 0.
         karma_critpath (int): The critpath karma associated with this comment. Defaults to 0.
-            **DEPRECATED** no longer used in the UI
+            **DEPRECATED** no longer used in the UI, maintained in db only for historic reason
         text (str): The text of the comment.
         timestamp (datetime.datetime): The time the comment was created. Defaults to
             the return value of datetime.now(timezone.utc).

bodhi-server/bodhi/server/schemas.py

@@ -178,6 +178,7 @@ def deserialize(self, cstruct):
         validator=colander.Range(min=-1, max=1),
         missing=0,
     )
+    # DEPRECATED not used anymore; date=2024-06-13
     karma_critpath = colander.SchemaNode(
         colander.Integer(),
         validator=colander.Range(min=-1, max=1),

bodhi-server/bodhi/server/services/comments.py

@@ -18,6 +18,7 @@
 """Define the service endpoints that handle Comments."""
 
 import math
+import warnings
 
 from cornice import Service
 from cornice.validators import colander_body_validator, colander_querystring_validator
@@ -223,6 +224,13 @@ def new_comment(request):
         request.errors.status = HTTPForbidden.code
         return
 
+    if data.get('karma_critpath', None):
+        data.pop('karma_critpath')
+        warnings.warn(
+            "karma_critpath is not used anymore and should not be passed in new comments; "
+            "date=2024-11-16", DeprecationWarning, stacklevel=2
+        )
+
     try:
         comment, caveats = update.comment(session=request.db, author=author, **data)
     except ValueError as e:

bodhi-server/tests/services/test_comments.py

@@ -92,7 +92,9 @@ def test_invalid_karma(self):
                                  status=400)
         assert '2 is greater than maximum value 1' in res
 
-    def test_commenting_with_critpath_feedback(self):
+    @mock.patch('bodhi.server.services.comments.warnings.warn')
+    def test_commenting_with_critpath_feedback(self, warn):
+        """karma_critpath is not used anymore and should be ignored with a warning."""
         comment = self.make_comment()
         comment['karma_critpath'] = -1  # roll out the trucks
 
@@ -103,7 +105,12 @@ def test_commenting_with_critpath_feedback(self):
         assert 'comment' in res.json_body
         assert res.json_body['comment']['text'] == 'Test'
         assert res.json_body['comment']['user_id'] == 1
-        assert res.json_body['comment']['karma_critpath'] == -1
+        assert res.json_body['comment']['karma_critpath'] == 0
+        warn.assert_called_once_with(
+            ("karma_critpath is not used anymore and should not be passed in new comments; "
+             "date=2024-11-16"),
+            DeprecationWarning, stacklevel=2
+        )
 
     def test_commenting_with_bug_feedback(self):
         comment = self.make_comment()

bodhi-server/tests/test_models.py

@@ -4804,3 +4804,17 @@ def test_override_with_inheritance(self, get_session):
         calls = [mock.call(release_f17.override_tag, build.nvr, strict=True),
                  mock.call(build.release.override_tag, build.nvr, strict=True)]
         get_session.return_value.untagBuild.assert_has_calls(calls)
+
+
+class TestDeprecatedObjects(BasePyTestCase):
+    """Test deprecated objects."""
+    @mock.patch('bodhi.server.models.warnings.warn')
+    def test_deprecated_BugKarma(self, warn):
+        """Trying to define a BugKarma should return a BugFeedback object."""
+        update = model.Update.query.first()
+        update.comment(self.db, 'testing', author='enemy', karma=-1, karma_critpath=-1)
+        warn.assert_called_once_with(
+            "karma_critpath is not used anymore and should not be passed in comment() call; "
+            "date=2024-11-16",
+            DeprecationWarning, stacklevel=2
+        )

news/PR5796.dev

@@ -0,0 +1 @@
+A deprecation warning is now emitted when a value for `karma_critpath` is set in a comment