[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

Katrin Fischer <katrin.fischer at bsz-bw.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA
            Summary|Move AllowOnShelfHolds      |Move AllowOnShelfHolds and
                   |system preference to the    |OpacItemHolds system
                   |Circulation Matrix          |preferences to the
                   |                            |Circulation Matrix

--- Comment #95 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
I got confused about the number of sign-offs and QAs, so I thought I'd add
another round of testing/QA. 
Overall this works pretty well (yay!), but I found some problems:

CODE REVIEW

1) Change the bug description to better reflect what the patch does:
  It's not only moving AllowOnShelfHolds, but also OpacItemHolds into the
  circulation matrix (DONE)
  Maybe also change the commit message? (trivial)

2) Why is this line removed from updatedatabaes.pl?
$dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE
variable='ReservesMaxPickupDelay'"); (?)

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)

4) Add bug number to database update. (trivial)


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

6) If you checked the checkbox on saving and open the rule for editing, the
checkbox is not checked, but it should be. (blocker)

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)


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)

9) Working with last setting from above: opacitemholds = F
- onshelfholds = no = OK, no hold allowed
- onshelfholds = no, but with one item checked out now = OK, item level hold on
checked out item allowed
- opacitemholds = N = OK, record level hold allowed
- onshelfholds = no, opacitemholds = Y = OK, item level hold on checked out
item allowed, record level hold allowed

Summary: Looks ok

10) All - Computer files: 10 days, reservesallowed 99, onshelfholds no,
opacitemholds no
   All - Books (BK): 10 days, reservesallowed 99, onshelfholds yes,
opacitemholds yes
   Adding another item = Computer files (CF)
- OK
   record level hold is allowed
   item level hold is only allowed for BK items 
- OK CF: onshelfholds = yes
   same
- OK CF: opacitemholds = yes
   item level hols is now allowed for CF items also

Summary: Looks ok

11) All - Computer files (CF): 10 days, reservesallowed 99...
   All - Books (BK): 10 days, reservesallowed 99...
   Another checked out CF item is added. 4 BK, 2 CF now, one of each itype
checked out
- OK
  BK: onshelfholds = no, opacitemholds = Y
  CF: onshelfholds = no, opacitemholds = Y
  record level hold is allowed.
  only item level holds on checked out items are allowed.

ILS-DI

OK Didn't explode and seemed ok for the few cases tested.

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