Bug 8479

Summary:	Opac Holds being created with priority set to NULL
Product:	Koha	Reporter:	Colin Campbell <colin.campbell>
Component:	Hold requests	Assignee:	Colin Campbell <colin.campbell>
Status:	CLOSED FIXED	QA Contact:	Mason James <mtj>
Severity:	normal
Priority:	P5 - low	CC:	chris, gmcharlt, kyle, mtj, paul.poulain
Version:	3.10
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Proposed Patch Bug 8479 Calculate rank irrespective of display option

Description Colin Campbell 2012-07-20 09:31:18 UTC

As part of the patch introduced by bug 7398 the calculation of priority is governed by the display option. However the if the rank is not calculated because you have not opted to display it undef is passed to AddReserve and holds are not being picked up by subsequent pull lists

Comment 1 Colin Campbell 2012-07-20 10:22:39 UTC Comment hidden (obsolete)

Created attachment 11037 [details] [review]
Proposed Patch

Removes the if related to the syspref

perltidied the reindented code

For clarity used rank + 1 rather than rank++ then assigning it as incrementing the variable may suggest to the casual reader that rank is used elsewhere rather than just about to cease to exist.

Comment 2 Kyle M Hall (khall) 2012-08-03 16:23:51 UTC

Created attachment 11344 [details] [review]
Bug 8479 Calculate rank irrespective of display option

Calculation of rank was being skipped based on display
option but it is used in calculating the rank passed
to AddReserve resulting in reserves being created with
null priorities causing them to be skipped in subsequent
processing

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Comment 3 Mason James 2012-08-10 07:27:30 UTC

(In reply to comment #2)
> Created attachment 11344 [details] [review]
> Bug 8479 Calculate rank irrespective of display option
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

patch looks good, passing QA


mason@xen1:~/git/head$ koha-qa.pl
        * 90dd666 Bug 8479 Calculate rank irrespective of display option
                opac/opac-reserve.pl

- perlcritic-progressive tests... OK
- perl -c syntax tests... OK
- xt/tt_valid.t tests... OK
- xt/author/valid-template.t tests... OK

Comment 4 Chris Cormack 2012-08-12 06:12:18 UTC

Patch doesn't apply cleanly on 3.8.x, if it is needed please submit a patch based on 3.8.x

Comment 5 Colin Campbell 2012-08-13 15:12:33 UTC

3.8 does not have the syspref introduced with bug 7398 so should not require this.
But will need applying id bug 7398 applied

Comment 6 Chris Cormack 2012-08-14 01:10:03 UTC

(In reply to comment #5)
> 3.8 does not have the syspref introduced with bug 7398 so should not require
> this.
> But will need applying id bug 7398 applied

Thanks for this Colin.

Comment 7 Chris Cormack 2013-04-20 22:10:28 UTC

Released in 3.10.0