Fix: Enforce test case and bug feedback requirements in meets_requirements_why

[harshith1118]

This PR addresses the FIXME comment in the meets_requirements_why method that was not
enforcing test case and bug feedback requirements shown in the UI but not currently enforced.

 ### Issue
 The `meets_requirements_why` function was only checking test gating, karma thresholds, and
  mandatory days in testing, but was NOT checking:
 - `require_testcases` field (when True, all test cases should have positive feedback)
 - `require_bugs` field (when True, all linked bugs should have positive feedback)

 ### Solution
 Added logic to verify that:
- When `require_testcases` is True, all associated test cases have positive feedback (no
  negative karma)
- When `require_bugs` is True, all associated bugs have positive feedback (no negative karma)
- Return (False, reason) when requirements are not met, consistent with existing behavior

### Changes
- Modified `bodhi/server/models.py` in the `meets_requirements_why` method
- Added checks for test case feedback when `self.require_testcases` is True
- Added checks for bug feedback when `self.require_bugs` is True
- Updated docstring to reflect new behavior

### Technical Details
The function now calls `self.get_testcase_karma(testcase)` and `self.get_bug_karma(bug)` to
  check for negative feedback and returns early with a descriptive reason when requirements are
  not met.

This fix ensures the functionality matches what is shown in the UI and prevents updates from
  being pushed when they don't meet the specified requirements.

Fixes FIXME comment in line 4011 of bodhi/server/models.py

[gridhead]

Could you please look into the tests?

[harshith1118]

Hi Akashdeep Dhar. Just wanted to let you know that I've fixed the pre-commit coverage issue. The problem was that some lines in the meets_requirements_why method weren't covered by tests, specifically lines 4056 and 4061-4064. I added two new test methods to cover those scenarios where test cases or bugs have negative feedback. The changes have been committed and pushed to the repository. The pre-commit checks should now pass sincewe've got full coverage. Thanks for your patience while I worked through this..

…

On Mon, 13 Oct 2025 at 08:53, Akashdeep Dhar ***@***.***> wrote: ***@***.**** requested changes on this pull request. Could you please look into the tests? — Reply to this email directly, view it on GitHub <#5956 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BAIHD2FQNUYUCSIMPOPT7UD3XMLMPAVCNFSM6AAAAACILV3IYWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGMRZHA4DONZSGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[gridhead]

I am going to defer to @mattiaverga for a more formal review but could you please do something about the abysmally long commit messages? Especially those from cc2000a and 5956b98. In fact, I would recommend squashing the commits into one. As a general thumb rule, please try to keep your commit messages under 50 characters.

[AdamWill]

These additional, clearly AI-generated files are not needed or desired:

CONTRIBUTION_ACTIVITY_LOG.txt
PR_DESCRIPTION.md
PR_TITLE_AND_DESCRIPTION.txt
bodhi-server/test_fix.py

As far as the fix itself goes: by eyeball this probably would more or less work, but I think putting it at the bottom of the method and messing around with the existing flow is not necessary. These checks work similarly to the existing gating checks near the top of the method: they can never pass the update by themselves, but they can fail it by themselves. So we could handle them similarly. We can put the checks before the gating block, have them return False immediately if they fail, and have them initiate basemsg appropriately if they pass. (We'd also have to tweak the gating block to extend basemsg rather than create it, but that's a much smaller change than the one the PR currently proposes).

The tests are insufficient - they only test the failure case, not the success case. They need to also check that the result is True in all cases where it should be - no bug / test case requirements, or the requirements are met. Also it should check that we properly fail when there is both positive and negative karma, and when there is no karma, not just when there is negative karma. Ideally use pytest.mark.parametrize to test lots of combinations, as various tests already do (can use those as a guide).

I'm not the maintainer, but FWIW, as this is clearly AI-generated, I think it would be good to mark it as such. The best practice I've found so far for this is to add an Assisted-By: to the commit message with details on the AI system used, e.g. Assisted-By: Claude Sonnet 4.5 via Cursor 1.7.40, or whatever.

Finally, kinda most importantly: it is very dumb that we've had this as a FIXME forever and we've had elements in the web UI that have done nothing forever, but "fixing" this would be a somewhat surprising change to many folks, I think. They've probably subconsciously gotten used to the status quo that the weird 'require testcases' and 'require bugs' sliders default to on, but don't actually do anything. If we suddenly change it so they do what they're supposed to, I think people will be pretty surprised. We might want to consider changing the defaults of those sliders to 'off', to respect what the effective default behaviour has been for years, and/or doing prominent messaging about this change.

[mattiaverga]

Thanks, for this pull request. I quote everything @AdamWill has wrote in the previous comment. Also, instead of just pushing AI generated work, please have a look at https://bodhi.readthedocs.io/en/25.11.1/developer/index.html and try to adjust the PR to the standards we use in this repository.

[AdamWill]

In fact, I would recommend squashing the commits into one. As a general thumb rule, please try to keep your commit messages under 50 characters.

@gridhead yikes, I'll have to make sure you're never in a position to review my PRs. I'd never get anything merged. :P