New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If a user edits an update after Bodhi comments it can be pushed, it won't comment again #1310

Closed
bowlofeggs opened this Issue Feb 23, 2017 · 1 comment

Comments

Projects
None yet
1 participant
@bowlofeggs
Member

bowlofeggs commented Feb 23, 2017

I noticed that my Bodhi update in Bodhi reached 14 days in testing this week, but Bodhi never commented on it to tell me that it had been in testing long enough to be pushed to stable:

https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-cd060eeb3c

After poking around in the staging db for a while to find out what gives, I found that the update's met_testing_requirements() method was returning True because long ago Bodhi had commented to state that the update could be pushed to stable, before I had edited the update and reset its karma and date_testing. The code doesn't take the karma reset event into account:

        min_num_days = self.release.mandatory_days_in_testing
        if min_num_days:
            if not self.meets_testing_requirements:
                return False
        else:
            return True
        for comment in self.comments:
            if comment.user.name == u'bodhi' and \
               comment.text.startswith('This update has reached') and \
               'and can be pushed to stable now if the ' \
               'maintainer wishes' in comment.text:
                return True
        return False

I think we probably want to take this loop from the karma property and make it into a reusable method that just returns comments that should be considered for karma and testing requirements:

        for comment in reversed(self.comments):
            if (comment.user.name == u'bodhi' and
                    ('New build' in comment.text or 'Removed build' in comment.text)):
                # We only want to consider comments since the most recent karma reset, which happens
                # whenever a build is added or removed from an Update. Since we are traversing the
                # comments in reverse order, once we find one of these comments we can simply exit
                # this loop.
                break

This way we can have met_testing_requirements() and karma() both use this new method to get the comments that they use to do what they do.

@bowlofeggs

This comment has been minimized.

Show comment
Hide comment
@bowlofeggs

bowlofeggs Feb 23, 2017

Member

A good way to test this when fixing it is to add a test to this module:

https://github.com/fedora-infra/bodhi/blob/2.4.0/bodhi/tests/server/scripts/test_approve_testing.py

Member

bowlofeggs commented Feb 23, 2017

A good way to test this when fixing it is to add a test to this module:

https://github.com/fedora-infra/bodhi/blob/2.4.0/bodhi/tests/server/scripts/test_approve_testing.py

crungehottman added a commit to crungehottman/bodhi that referenced this issue Mar 31, 2017

crungehottman added a commit to crungehottman/bodhi that referenced this issue Apr 1, 2017

crungehottman added a commit to crungehottman/bodhi that referenced this issue Apr 4, 2017

crungehottman added a commit to crungehottman/bodhi that referenced this issue Apr 4, 2017

crungehottman added a commit to crungehottman/bodhi that referenced this issue Apr 5, 2017

crungehottman added a commit to crungehottman/bodhi that referenced this issue Apr 5, 2017

@bowlofeggs bowlofeggs closed this in ceaf533 Apr 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment