[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 #97 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
Hi Srdjan,

thx for your quick answer.

(In reply to Srdjan Jankovic from comment #96)
> (In reply to Katrin Fischer from comment #95)

> > 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?

I am not sure it's a noop. It's part of a real old database update:
$DBversion = "3.01.00.007";
We normally don't touch those, as it might give problems to someone updating
from an really old version of Koha.


> > 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).

The patches applied cleanly for me on last nights master: 

commit ac3f497f64c1854a275fe894c1070f5888c1c302
Bug 13235: Move onclick attr to javacsript code 

I know this one is really trivial, but as we are going to touch updatedatabase
for 3) we can fix this too.


> > 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

I will check with some people today how they feel. I agree that the checkbox
seems more ergonomic, but the Yes/No pull down reflects what is shown after
saving the rule, which seems more consistent to me. 

> > 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)

I think the edit functionality uses Javascript, it might be something in the
.tt that needs to be fixed here. The change in the follow-up seems not related.
To test:

1) Add a rule, check the check box.
2) Verify it's saved as 'Yes'
3) Edit the same rule using the link at the end
4) Verify the checkbox is now unchecked (last line of the table)


> > 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

More a note than a blocker, but it would be nice if we could come up with
something easier to understand. Maybe Martin or Kyle can suggest something (or
they like it how it is :) ).


> > 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 :)

I was worried that might happen - the overall message is: it works quite well
:), except for one thing in the display related to the 'forced' option, that I
think looks not quite right. Trying to give you a better test plan and will
also attach a screenshot in the next step:

1) Create an issuingrule:
   All - itype1 - opacitemholds=Force, onshefholds=yes
2) Create a record with only a itype1 item
3) Place a hold in the OPAC
4) Notice you can't place a 'next available' hold, but the option is
   displayed without the checkbox to check it.

In this case, the 'next available' bit can be hidden. It gets a bit more
complicated with a second item:

5) Create a second issuiingrule:
   All - itype2 - opacitemholds = Yes (record level allowed), onshelfholds =
yes
6) Add a second item of itype2 to the record
7) Place a hold in the OPAC
8) Notice the display is the same as above

The question here is: If one of the itemtypes allows for record level holds and
items of that itype are available for item level holds, should a record level
hold be allowed? 

Not totally sure about the behaviour question, would really like another
opinion here.

-- 
You are receiving this mail because:
You are watching all bug changes.