Update's meets_requirements_why doesn't enforce test case and bug feedback requirements

[harshith1118]

In bodhi/server/models.py, line 4011, there is a FIXME comment in the meets_requirements_why method that
states:

FIXME: we should probably wire up the test case and bug feedback requirements
that are shown in the UI but not currently enforced.

Problem:
The meets_requirements_why function currently only checks for:

1. Test gating status
2. Karma threshold (min_karma)
3. Mandatory days in testing (mandatory_days_in_testing)

But it does NOT check the require_testcases and require_bugs fields that are part of the update model. These
boolean fields allow users to specify that:

• require_testcases: All associated test cases must have positive feedback before the update can be pushed
  to stable
• require_bugs: All associated bugs must have positive feedback before the update can be pushed to stable

Expected Behavior:
When require_testcases is True, the function should verify that all test cases associated with the update
have positive feedback (no negative karma).
When require_bugs is True, the function should verify that all bugs associated with the update have
positive feedback (no negative karma).

Impact:
Without this check, updates with require_testcases=True or require_bugs=True may be pushed to stable even
when the required positive feedback has not been received, which violates the intended policy.

Files to Modify:

• bodhi/server/models.py - Update the meets_requirements_why method around line 4000

The function should return (False, "reason") when requirements are not met, similar to how other checks
are implemented in the same function.

[varruunnn]

@harshith1118 @gridhead
I'd like to take on this issue if the solution sounds good to the team. The fix involves adding checks for require_bugs and require_testcases within the Update.meets_requirements_why method, as indicated by the FIXME comment.
Since the policy is "positive feedback (no negative karma)," the check should ensure that if the requirement is enabled, no associated bug or test case has received negative karma.
To achieve this, I will implement two new helper properties on the Update model and integrate them into meets_requirements_why.
New Helper Properties (models.py)
I will add these properties to the Update class, leveraging the existing get_bug_karma and get_testcase_karma methods
Modification to meets_requirements_why
The meets_requirements_why method will be modified to include these checks early, ensuring policy failure is caught before evaluating minimum karma or days in testing:

@property
def meets_requirements_why(self):
    req_days = self.mandatory_days_in_testing
    req_karma = self.min_karma
    if self.require_bugs and not self.bugs_all_positive_feedback:
        return (False, "Associated bugs do not all have positive feedback.")

    if self.require_testcases and not self.testcases_all_positive_feedback:
        return (False, "Required test cases do not all have positive feedback.")
....

[harshith1118]

Hi *varun.* Thank you for the feedback. I appreciate the suggestion for a cleaner implementation approach using helper properties. I believe there are merits to both approaches: Your proposed approach (helper properties): Results in cleaner, more readable main function Separates concerns appropriately More testable individual components The current approach (inline checks): Provides immediate early returns when negative feedback is found Potentially more efficient for updates with many test cases/bugs where negative feedback is found early Consistent with the existing pattern in the function (immediate return on failure) I'm happy to refactor to use the helper properties approach as you've suggested for better code organization. Here's what the implementation would look like: @Property def bugs_all_positive_feedback(self): """ Check if all associated bugs have positive feedback (no negative karma). Returns: bool: True if all bugs have no negative feedback or no bugs exist, False if any bug has negative feedback. """ if not self.bugs: return True for bug in self.bugs: negative_karma, positive_karma = self.get_bug_karma(bug) if negative_karma < 0: return False return True @Property def testcases_all_positive_feedback(self): """ Check if all associated test cases have positive feedback (no negative karma). Returns: bool: True if all test cases have no negative feedback or no test cases exist, False if any test case has negative feedback. """ if not self.full_test_cases: return True for testcase in self.full_test_cases: negative_karma, positive_karma = self.get_testcase_karma(testcase) if negative_karma < 0: return False return True And then updating meets_requirements_why as you suggested. Please let me know which approach you prefer, and I'll implement accordingly. I can make either change to ensure it aligns with the project's coding standards and maintainability goals.

…

On Sat, 11 Oct 2025 at 16:23, Varun Choudhary ***@***.***> wrote: *varruunnn* left a comment (fedora-infra/bodhi#5955) <#5955 (comment)> @harshith1118 <https://github.com/harshith1118> @gridhead <https://github.com/gridhead> I'd like to take on this issue if the solution sounds good to the team. The fix involves adding checks for require_bugs and require_testcases within the Update.meets_requirements_why method, as indicated by the FIXME comment. Since the policy is "positive feedback (no negative karma)," the check should ensure that if the requirement is enabled, no associated bug or test case has received negative karma. To achieve this, I will implement two new helper properties on the Update model and integrate them into meets_requirements_why. New Helper Properties (models.py) I will add these properties to the Update class, leveraging the existing get_bug_karma and get_testcase_karma methods Modification to meets_requirements_why The meets_requirements_why method will be modified to include these checks early, ensuring policy failure is caught before evaluating minimum karma or days in testing: @Property def meets_requirements_why(self): req_days = self.mandatory_days_in_testing req_karma = self.min_karma if self.require_bugs and not self.bugs_all_positive_feedback: return (False, "Associated bugs do not all have positive feedback.") if self.require_testcases and not self.testcases_all_positive_feedback: return (False, "Required test cases do not all have positive feedback.") .... — Reply to this email directly, view it on GitHub <#5955 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BAIHD2FSFIYI73ELX77PEXD3XDOS3AVCNFSM6AAAAACILSLUUKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOJTGIYDENZZGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gridhead]

@mattiaverga as you are one of the primary maintainers of the project, please comment on if this is something that we are looking to implement.

[mattiaverga]

Sorry, I do not have time at the moment to look at this.
I could find some time in a couple of weeks, meanwhile I think we need to carefully apply the change, because I suppose it will cause "unexpected behavior". Perhaps it is better to be discussed/announced in the Fedora devel mailing list or on Fedora Discuss.

[AdamWill]

Note there's a pull request for this, with further discussion: #5956