[Koha-bugs] [Bug 5786] Move AllowOnShelfHolds and OpacItemHolds system preferences to the Circulation Matrix

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5786

--- Comment #96 from Srdjan Jankovic <srdjan at 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.