http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5786 --- Comment #96 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- (In reply to Katrin Fischer from comment #95)
CODE REVIEW
2) Why is this line removed from updatedatabaes.pl? $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='ReservesMaxPickupDelay'"); (?)
I don't remember clearly. This is a combination of bugs that could not be done separately, they are too interdependent. This was part of 5788. But it seems that ReservesMaxPickupDelay was never implemented. I am confused. So maybe put it back, although it seems to be a noop?
3) Why set the issuingrules to 1, after finding out the original setting first? + $dbh->do("UPDATE issuingrules SET opacitemholds=1"); Shouldn't it update to $opacitemholds with Y, N or F? (blocker)
That's a bug, it should be: + $dbh->do("UPDATE issuingrules SET opacitemholds='$opacitemholds'");
4) Add bug number to database update. (trivial)
I have no problems fixing that, however this patch has changed and when I do git bz apply I get: fatal: sha1 information is lacking or useless (C4/Auth.pm). Any hints?
TESTING
Issuingrules
5) Automatic renewal is a yes/no pull down, on shelf holds is a checkbox. I think to be more consistent we should use one or the other. (?)
Ok, which one? To me tick box makes more sense for yes/no
6) If you checked the checkbox on saving and open the rule for editing, the checkbox is not checked, but it should be. (blocker)
I can see there was something done in the follow up, but I cannot pick it up (git bz appply above)
7) I feel like the description and options of the new opacitemholds is hard to interpret, if you don't know about the former behaviour. But not sure how to rename. I feel like item-level holds might be a little more understandable, but not sure. (trivial)
I can do that
Placing holds (not sure that's understandable to anyone but me...)
8) All - Books: 10 days, reservesallowed 99, onshelfholds = yes Record: 4 items, all Books and available, one being notforloan = 'on order' There is a positive all-all-all rule. Maxreserves is > 0 item-level_itypes is set to specific item - opacitemholds = Y = OK, both options are available - opacitemholds = N = OK, only title level hold available ! opacitemholds = F = NOT OK? display is confusing, as it still shows "Next available item A specific item" but the first without the checkbox Tested in 3.18.2 - there "Next available item" is not shown in this case
Summary: All items one itype, forced item level holds - display still offers "Next available" but no checkbox I feel like the combination of one itype F and another set to Y (allow bib level) is problematic. We need to decide what to do here - allow bib level (activate the checkbox) or remove the mention of it from the templates altogether. I tend to do the first. (normal)
I'm afraid that goes way beyond my understanding. But if you tell me what you want to happen, I can make it happen :) -- You are receiving this mail because: You are watching all bug changes.