http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |sandboxes@biblibre.com --- Comment #2 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Patch tested with a sandbox, by Coralie Barsacq <c.barsacq@mairie-laciotat.fr> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 --- Comment #3 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 28634 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28634&action=edit Bug 12287 - Hold priority for new hold incorrect if record has holds in transit When placing a new hold, the listed priority for the hold will be incorrect if there are any holds with items in transit ( i.e. found = T ). Test Plan: 1) Place 3 holds on a record with at least 2 items 2) Fill one hold so it is marked as waiting 3) Fill on hold so it is marked as in transit 4) Begin placing a new hold 5) Note the expected priority would be 2, but is actually 3 6) Apply this patch 7) Reload the patch, now the expected priority shows correctly Signed-off-by: Coralie Barsacq <c.barsacq@mairie-laciotat.fr> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- I don't think so. That could cause future regressions if new found statuses are added. For example, there used to be a found status of 'F' for 'Finished' ( from before the existence of the old_reserves table iirc ). We never want to include any type of found hold when calculating a new hold's priority, set there is no need to list everything as a special case. (In reply to Jonathan Druart from comment #4)

Don't you think it's safer to test the 'T' value? Something like

if ( defined $res->{found} and ( $res->{found} eq 'W' or $res->{found} eq 'T' ) ) { $count--; }

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

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28634|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29911 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29911&action=edit Bug 12287 - Hold priority for new hold incorrect if record has holds in transit When placing a new hold, the listed priority for the hold will be incorrect if there are any holds with items in transit ( i.e. found = T ). Test Plan: 1) Place 3 holds on a record with at least 2 items 2) Fill one hold so it is marked as waiting 3) Fill on hold so it is marked as in transit 4) Begin placing a new hold 5) Note the expected priority would be 2, but is actually 3 6) Apply this patch 7) Reload the patch, now the expected priority shows correctly Signed-off-by: Coralie Barsacq <c.barsacq@mairie-laciotat.fr> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12287 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #5)

I don't think so. That could cause future regressions if new found statuses are added. For example, there used to be a found status of 'F' for 'Finished' ( from before the existence of the old_reserves table iirc ). We never want to include any type of found hold when calculating a new hold's priority, set there is no need to list everything as a special case.

Looking at the code in C4/Reserves.pm and reserves/*, the 'found' value is often compared to 'W' or 'T'. Both values are listed explicitly. That's why I supposed it is safer to use the same way. I added a small comment in the line (in order to get return if someone grep found.*W.*T) -- You are receiving this mail because: You are watching all bug changes.