https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19532
--- Comment #358 from Martin Renvoize <[email protected]> --- I've uploaded an updated patchset with some simple followups added. However, I've got a few code review comments, and I've not got to the full testing of the feature itself yet. I don't think this is likely to be ready for 18.11 personally, but it would be good to get the groundwork done to get it in at the beginning of the next cycle. Review: This really feels like it should be built atop the existing Holds/Reserves system. Everyone I've spoken to, to date, has been really surprised that it appears to be an entirely distinct feature. It also feels like allot of code was borrowed from reserves, leading to repetition. Putting that aside for now, however: 1) Please update the POD for C4::Circulation::transferbook to state how the functionality has changed 2) There is commented out code in the above routine which looks as though it needs more thought 3) Shouldn't the librarian be able to override a recalls at checkout the same way they can override a reserve (based on sysprefs I believe) 4) Please document the recalls status codes somewhere 5) C4::Circulation::AddReturn - There are superfluous DB calls here I believe 6) Reserves/Holds take precedence over Recalls during renewal, is that deliberate (everywhere else it seems to be the opposite way around) 7) Superfluous `use Data::Dumper` in C4::Letters 8) I couldn't work out from the code how this affects fines.. only that is does 9) I don't understand why you've changed Koha/Patron.pm 10) Superfluous `use Data::Dumper` in catalogue/detail.pl 11) Why 3 distinct DB hits in catalogue/detail.pl 12) circ/recall-pickup-slip.pl appears to contain unused imports 13) circ/recalls_overdue.pl, circ/recalls_queue.pl, circ/recalls_waiting all have references to Fast Cataloguing.. not sure why 14) Why the mix of filenames with underscores and hyphens? 15) circ/returns.pl has commented out code, leaving the feeling that it needs further thought 16) atomicupdate .sql files should be converted to .perl and use the template. 17) Rebase issue has resulted in SR_SLIP being contained within many of the sql files 18) All JS should be pushed to the bottom of the file now as per recent(ish) guidelines.. I think this has partially been done, but some cases have been missed. Lots of little things.. now I need to fully test the feature to really get to grips with why it's entirely distinct from Reserves. I wish there was a full RFC we could see and a Work In Progress branch for updating the manual. I'm struggling to pick out the whole modus operandi of this patch as it's not what anyone here understands 'recalls' to be. Hopefully, we can keep this moving along.. sorry it's not all good news, but I am keen to help get it moving again. -- 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/
