Fix update_unavailable_reason leaking across rows in plugin and theme list - #552
Conversation
… list The reason string was set inside the item loop when an update did not meet the PHP or WordPress requirement, but it was never reset for the next item. Every plugin or theme listed after an incompatible one then showed the first item's reason, even with update none or available. Reset the variable for each item so only the affected row carries a reason. Covers both Plugin_Command::get_item_list() and the ParseThemeNameInput trait, which got the same pattern from the same commit.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe plugin and theme list builders now reset ChangesUnavailable reason scoping
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The unavailable-update reason is now isolated to the affected plugin or theme row, with regression coverage for subsequent empty rows. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Fixes #551
The reason string was set inside the item loop when an update did not meet the PHP or WordPress requirement, but it was never reset for the next item. Every plugin or theme listed after an incompatible one then showed the first item's reason, even with update
noneoravailable.This resets the variable for each item so only the affected row carries a reason, in both
Plugin_Command::get_item_list()and theParseThemeNameInputtrait. Both sides got the same pattern from the same commit (730da5d), so the leak exists since v2.1.24.New Behat scenarios for both sides sit next to the existing requirement scenarios: two local extensions where only the first has an incompatible update, asserting in csv output that the second row's reason stays empty. Both scenarios fail against the current main code and pass with the fix. The four existing plugin unavailable-reason scenarios and both theme requirement scenarios still pass.
Also verified on a live install: with a filter giving only the first plugin (or theme) an incompatible update, the later rows showed the leaked reason before the fix and are empty after it.
Summary by CodeRabbit