Skip to content

Commit

Permalink
Revert "Rename BugKarma to BugFeedback"
Browse files Browse the repository at this point in the history
This reverts commit 5daa473.
  • Loading branch information
mattiaverga committed Nov 15, 2024
1 parent 54249e1 commit f7ec32f
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 133 deletions.

This file was deleted.

64 changes: 20 additions & 44 deletions bodhi-server/bodhi/server/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
UniqueConstraint,
)
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.orm import class_mapper, declarative_base, relationship, synonym, validates
from sqlalchemy.orm import class_mapper, declarative_base, relationship, validates
from sqlalchemy.orm.base import NEVER_SET
from sqlalchemy.orm.exc import NoResultFound
from sqlalchemy.orm.properties import RelationshipProperty
Expand Down Expand Up @@ -3042,20 +3042,14 @@ def get_bugstring(self, show_titles=False):
return val

def get_bug_karma(self, bug):
"""Redirect to get_bug_feedback()."""
warnings.warn("get_bug_karma() is deprecated, use get_bug_feedback(); date=2024-06-12",
DeprecationWarning, stacklevel=2)
return self.get_bug_feedback(bug)

def get_bug_feedback(self, bug):
"""
Return the feedback for this update for the given bug.
Return the karma for this update for the given bug.
Args:
bug (Bug): The bug we want the feedback about.
bug (Bug): The bug we want the karma about.
Returns:
tuple: A 2-tuple of integers. The first represents negative feedback, the second
represents positive feedback.
tuple: A 2-tuple of integers. The first represents negative karma, the second represents
positive karma.
"""
good, bad, seen = 0, 0, set()
for comment in self.comments_since_karma_reset:
Expand All @@ -3064,9 +3058,9 @@ def get_bug_feedback(self, bug):
seen.add(comment.user.name)
for feedback in comment.bug_feedback:
if feedback.bug == bug:
if feedback.feedback > 0:
if feedback.karma > 0:
good += 1
elif feedback.feedback < 0:
elif feedback.karma < 0:
bad += 1
return bad * -1, good

Expand Down Expand Up @@ -3529,8 +3523,8 @@ def update_bugs(self, bug_ids, session):
for bug in to_remove:
self.bugs.remove(bug)
if len(bug.updates) == 0:
# Don't delete the Bug instance if there is any associated BugFeedback
if not session.query(BugFeedback).filter_by(bug_id=bug.bug_id).count():
# Don't delete the Bug instance if there is any associated BugKarma
if not session.query(BugKarma).filter_by(bug_id=bug.bug_id).count():
log.debug("Destroying stray Bugzilla #%d" % bug.bug_id)
session.delete(bug)
session.flush()
Expand Down Expand Up @@ -3567,7 +3561,7 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,

got_feedback = False
for feedback_dict in (bug_feedback + testcase_feedback):
if feedback_dict['feedback'] != 0:
if feedback_dict['karma'] != 0:
got_feedback = True
break

Expand Down Expand Up @@ -3628,7 +3622,7 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
session.flush()

for feedback_dict in bug_feedback:
feedback = BugFeedback(**feedback_dict)
feedback = BugKarma(**feedback_dict)
session.add(feedback)
comment.bug_feedback.append(feedback)

Expand Down Expand Up @@ -4456,37 +4450,20 @@ def __str__(self):
event.listen(Compose.state, 'set', Compose.update_state_date, active_history=True)


def BugKarma(*args, **kwargs):
"""Redirect to BugKarma class."""
warnings.warn("Class BugKarma is deprecated, use BugFeedback; date=2024-06-12",
DeprecationWarning, stacklevel=2)
return BugFeedback(*args, **kwargs)


# Used for one-to-one relationships between a comment and a bug
class BugFeedback(Base):
# Used for one-to-one relationships between karma and a bug
class BugKarma(Base):
"""
Feedback for a bug associated with a comment.
Karma for a bug associated with a comment.
Attributes:
feedback (int): The feedback associated with this bug and comment.
comment (Comment): The comment this BugFeedback is part of.
bug (Bug): The bug this BugFeedback pertains to.
karma (int): The karma associated with this bug and comment.
comment (Comment): The comment this BugKarma is part of.
bug (Bug): The bug this BugKarma pertains to.
"""

__tablename__ = 'comment_bug_assoc'

def __getattribute__(self, item):
"""Deprecate BugFeedback properties warnings."""
if item == 'karma':
warnings.warn("BugFeedback's karma class variable is deprecated, "
"use feedback instead; date=2024-06-12",
DeprecationWarning, stacklevel=2)
return super().__getattribute__(item)

feedback = Column(Integer, default=0)
# DEPRECATED this is only for temporary backwards compatibility
karma = synonym('feedback')
karma = Column(Integer, default=0)

# Many-to-one relationships
comment_id = Column(Integer, ForeignKey('comments.id'))
Expand All @@ -4510,7 +4487,6 @@ class TestCaseKarma(Base):
__tablename__ = 'comment_testcase_assoc'

karma = Column(Integer, default=0)
feedback = synonym('karma')

# Many-to-one relationships
comment_id = Column(Integer, ForeignKey('comments.id'))
Expand Down Expand Up @@ -4545,7 +4521,7 @@ class Comment(Base):
timestamp = Column(TZDateTime, default=partial(datetime.now, tz=timezone.utc))

# One-to-many relationships
bug_feedback = relationship('BugFeedback', back_populates='comment',
bug_feedback = relationship('BugKarma', back_populates='comment',
cascade="all,delete,delete-orphan")

testcase_feedback = relationship('TestCaseKarma', back_populates='comment',
Expand Down Expand Up @@ -4668,7 +4644,7 @@ class Bug(Base):
# Many-to-many relationships
updates = relationship('Update', secondary=update_bug_table, back_populates='bugs')

feedback = relationship('BugFeedback', back_populates='bug')
feedback = relationship('BugKarma', back_populates='bug')

@property
def url(self) -> str:
Expand Down
16 changes: 2 additions & 14 deletions bodhi-server/bodhi/server/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,10 @@ class BugFeedback(colander.MappingSchema):
"""A schema for BugFeedback to be provided via API parameters."""

bug_id = colander.SchemaNode(colander.Integer())
feedback = colander.SchemaNode(
colander.Integer(),
validator=colander.Range(min=-1, max=1),
missing=0,
)
# DEPRECATED this is only for temporary backwards compatibility
karma = colander.SchemaNode(
colander.Integer(),
validator=colander.Range(min=-1, max=1),
missing=None,
missing=0,
)


Expand All @@ -153,16 +147,10 @@ class TestcaseFeedback(colander.MappingSchema):
"""A schema for TestcaseFeedback to be provided via API parameters."""

testcase_name = colander.SchemaNode(colander.String())
feedback = colander.SchemaNode(
colander.Integer(),
validator=colander.Range(min=-1, max=1),
missing=0,
)
# DEPRECATED this is only for temporary backwards compatibility
karma = colander.SchemaNode(
colander.Integer(),
validator=colander.Range(min=-1, max=1),
missing=None,
missing=0,
)


Expand Down
4 changes: 2 additions & 2 deletions bodhi-server/bodhi/server/templates/update.html
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,8 @@ <h4>How to install</h4>
<div class="list-group-item">
<div class="row align-self-center">
<div class="col fw-bold">${self.util.bug_link(bug) | n}</div>
<div class="col-auto ps-3 fw-bold">${self.fragments.karma(update.get_bug_feedback(bug)[0], show_digit=True, zero_thumbsup=False)}</div>
<div class="col-auto ps-3 fw-bold">${self.fragments.karma(update.get_bug_feedback(bug)[1], show_digit=True, zero_thumbsup=True)}</div>
<div class="col-auto ps-3 fw-bold">${self.fragments.karma(update.get_bug_karma(bug)[0], show_digit=True, zero_thumbsup=False)}</div>
<div class="col-auto ps-3 fw-bold">${self.fragments.karma(update.get_bug_karma(bug)[1], show_digit=True, zero_thumbsup=True)}</div>
</div>
</div>
% endfor
Expand Down
5 changes: 0 additions & 5 deletions bodhi-server/bodhi/server/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1055,9 +1055,6 @@ def validate_bug_feedback(request, **kwargs):
bad_bugs.append(bug_id)
else:
item['bug'] = bug
if item['karma'] is not None:
# DEPRECATED this is only for temporary backwards compatibility
item['feedback'] = item.pop('karma')
validated.append(item)

if bad_bugs:
Expand Down Expand Up @@ -1117,8 +1114,6 @@ def validate_testcase_feedback(request, **kwargs):
bad_testcases.append(name)
else:
item['testcase'] = testcase
if item['karma'] is not None:
item['feedback'] = item.pop('karma')
validated.append(item)

if bad_testcases:
Expand Down
11 changes: 3 additions & 8 deletions bodhi-server/tests/services/test_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import copy

from fedora_messaging import api, testing as fml_testing
import pytest
import webtest

from bodhi.messages.schemas import update as update_schemas
Expand Down Expand Up @@ -106,14 +105,10 @@ def test_commenting_with_critpath_feedback(self):
assert res.json_body['comment']['user_id'] == 1
assert res.json_body['comment']['karma_critpath'] == -1

@pytest.mark.parametrize('compat', (True, False))
def test_commenting_with_bug_feedback(self, compat):
def test_commenting_with_bug_feedback(self):
comment = self.make_comment()
comment['bug_feedback.0.bug_id'] = 12345
if compat:
comment['bug_feedback.0.karma'] = 1
else:
comment['bug_feedback.0.feedback'] = 1
comment['bug_feedback.0.karma'] = 1

with fml_testing.mock_sends(update_schemas.UpdateCommentV1):
res = self.app.post_json('/comments/', comment)
Expand Down Expand Up @@ -230,7 +225,7 @@ def test_empty_comment(self):
"""Ensure that a comment without text or feedback is not permitted."""
comment = self.make_comment(text='')
comment['bug_feedback.0.bug_id'] = 12345
comment['bug_feedback.0.feedback'] = 0
comment['bug_feedback.0.karma'] = 0
comment['testcase_feedback.0.testcase_name'] = "Wat"
comment['testcase_feedback.0.karma'] = 0
res = self.app.post_json('/comments/', comment, status=400)
Expand Down
24 changes: 12 additions & 12 deletions bodhi-server/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
LockedUpdateException,
)
from bodhi.server.models import (
BugFeedback,
BugKarma,
PackageManager,
ReleaseState,
TestGatingStatus,
Expand Down Expand Up @@ -2259,20 +2259,20 @@ def test_version_hash(self):
assert update.version_hash == updated_calculated_hash


class TestUpdateGetBugFeedback(BasePyTestCase):
"""Test the get_bug_feedback() method."""
class TestUpdateGetBugKarma(BasePyTestCase):
"""Test the get_bug_karma() method."""

def test_feedback_wrong_bug(self):
"""Feedback for other bugs should be ignored."""
update = model.Update.query.first()
# Let's add a bug feedback to the existing comment on the bug.
bk = model.BugFeedback(feedback=1, comment=update.comments[0], bug=update.bugs[0])
# Let's add a bug karma to the existing comment on the bug.
bk = model.BugKarma(karma=1, comment=update.comments[0], bug=update.bugs[0])
self.db.add(bk)
# Now let's associate a new bug with the update.
bug = model.Bug(bug_id=12345, title='some title')
update.bugs.append(bug)

bad, good = update.get_bug_feedback(bug)
bad, good = update.get_bug_karma(bug)

assert bad == 0
assert good == 0
Expand All @@ -2285,10 +2285,10 @@ def test_mixed_feedback(self):
comment = model.Comment(text='Test comment', karma=karma, user=user)
self.db.add(comment)
update.comments.append(comment)
bug_karma = model.BugFeedback(feedback=karma, comment=comment, bug=update.bugs[0])
bug_karma = model.BugKarma(karma=karma, comment=comment, bug=update.bugs[0])
self.db.add(bug_karma)

bad, good = update.get_bug_feedback(update.bugs[0])
bad, good = update.get_bug_karma(update.bugs[0])

assert bad == -1
assert good == 2
Expand All @@ -2299,7 +2299,7 @@ def test_mixed_feedback(self):
self.db.add(comment)
update.comments.append(comment)

bad, good = update.get_bug_feedback(update.bugs[0])
bad, good = update.get_bug_karma(update.bugs[0])

assert bad == 0
assert good == 0
Expand Down Expand Up @@ -3676,9 +3676,9 @@ def test_update_bugs(self):
assert update.bugs[0].bug_id == 4321
assert self.db.query(model.Bug).filter_by(bug_id=1234).first() is None

# Try removing a bug when it already has BugFeedback
bug_feedback = BugFeedback(bug_id=4321, feedback=1)
self.db.add(bug_feedback)
# Try removing a bug when it already has BugKarma
karma = BugKarma(bug_id=4321, karma=1)
self.db.add(karma)
self.db.flush()
bugs = ['5678']
update.update_bugs(bugs, session)
Expand Down
4 changes: 2 additions & 2 deletions bodhi-server/tests/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ def test_schema_unflattening_for_comments(self):
'text': 'this is an update comment',
'karma': -1,
'karma_critpath': 1,
'bug_feedback': [{'bug_id': 1, 'feedback': 1}],
'bug_feedback': [{'bug_id': 1, 'karma': 1}],
'testcase_feedback': [{'testcase_name': "wat", 'karma': -1}],
}
flat_structure = {
'text': 'this is an update comment',
'karma': -1,
'karma_critpath': 1,
'bug_feedback.0.bug_id': 1,
'bug_feedback.0.feedback': 1,
'bug_feedback.0.karma': 1,
'testcase_feedback.0.testcase_name': 'wat',
'testcase_feedback.0.karma': -1,
}
Expand Down
1 change: 0 additions & 1 deletion news/PR5668.bic

This file was deleted.

1 change: 0 additions & 1 deletion news/summary.migration

This file was deleted.

0 comments on commit f7ec32f

Please sign in to comment.