http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10663
--- Comment #35 from Katrin Fischer <[email protected]> --- Hi Galen, thx for taking a look at this, pushing the patches and improving the tests. In addition to what you wrote, I want to try to explain, why I think fixing GetReserveStatus like Marcel suggested is not the way to go. Writing GetReserveStatus without using CheckReserves is trying to simplify something that is really complicated: Before you block renewal on an item, you want to check if it can actually fill the hold. There are lots of things that should be taken into account here - branches, item types, circulation rules, item statuses... CheckReseves is used on checking in the items to determine that - so I think there is the logic we need and we should try to not replicate this logic in the code. The counter patch will pass the unit tests probably, but only because I didn't manage to cover all use cases. There are a lot more tests to be written here. After looking through the code trying to figure it all out I came to the conclusion that we should go back to use GetReserveStatus like it was used before the refactoring - only checking for 'waiting'. Or even better, refactor and have a properly named sub that does just this - backed up by unit tests - and remove GetReserveStatus. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
