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

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |P3 Severity|major |normal -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Mirko Tietgen <mirko@abunchofthings.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14152|0 |1 is obsolete| | --- Comment #3 from Mirko Tietgen <mirko@abunchofthings.net> --- Created attachment 14158 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14158&action=edit Bug 6918 - can't place holds on 'on order' items with AllowOnShelfHolds off Koha documenation suggests that NOT_LOAN values less than 0 should be hold-able. This patch enables that.

From the manual: Negative number values will still allow holds (use for on order statuses for example) where as positive numbers will not allow holds or checkouts

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Works as expected -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA Comments: I am not sure at all but the check on the notforloan value should not be made in the CanItemBeReserved routine ? Is not it more relevant ? If not, your patch has to be resubmitted for a little modification. The test in IsAvailableForItemLevelRequest is: return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W" or $item->{notforloan} < 0)); it will be optimized if you change the order, as: return ($available_per_item and ($item->{onloan} or $item->{notforloan} < 0 or GetReserveStatus($itemnumber) eq "W" )); Marked as Failed QA. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- That was my first instinct as well, but the way the scripts work, it didn't 'fit' in that function. In fact, that function doesn't deal with items statuses at all! IsAvailableForItemLevelRequest actually deals with the status so that was where the changes needed to be made. I have implemented your optimization for the return value. Kyle (In reply to comment #4)

QA Comments:

I am not sure at all but the check on the notforloan value should not be made in the CanItemBeReserved routine ? Is not it more relevant ?

If not, your patch has to be resubmitted for a little modification.

The test in IsAvailableForItemLevelRequest is:

return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W" or $item->{notforloan} < 0));

it will be optimized if you change the order, as:

return ($available_per_item and ($item->{onloan} or $item->{notforloan} < 0 or GetReserveStatus($itemnumber) eq "W" ));

Marked as Failed QA.

-- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA Comment: If I correctly understand your patch, the increment of the count items issued have to be made in opac/opac-ISBDdetail.pl and opac/opac-MARCdetail.pl too. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Jonathan, here is a variation that keeps the code from being repeated multiple times. It also takes care of your issues in comment 8. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14451|0 |1 is obsolete| | --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 15077 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15077&action=edit Bug 6918 - can't place holds on 'on order' items with AllowOnShelfHolds off Test Plan: 1) Apply patch 2) Turn off AllowOnShelfHolds 3) Create a bib with one item, mark the item as 'on order' 4) Attempt to place a hold on the item, you should now be able to do so -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- I changed the prototype for CountItemsIssued, so I had to update all calls to it. Still, it wouldn't hurt to test in multiple ways. Kyle (In reply to comment #13)

(In reply to comment #12)

...

4) Attempt to place a hold on the item, you should now be able to do so

I see that this patch affects several scripts. Should one test placing a hold from multiple pages or in multiple ways?

-- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #15 from Owen Leonard <oleonard@myacpl.org> --- I'm guessing from the diff that I need to test these pages: opac-ISBDdetail.pl opac-MARCdetail.pl opac-detail.pl opac-search.pl opac-shelves.pl It seems to be working correctly on opac-ISBDdetail.pl, opac-MARCdetail.pl, and opac-detail.pl. I see problems with opac-search.pl and opac-shelves.pl: the "place hold" link which should appear within the search results/list contents does not appear--that is, within the table of results, on the line labeled "Actions." -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15077|0 |1 is obsolete| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19087 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19087&action=edit Bug 6918 - can't place holds on 'on order' items with AllowOnShelfHolds off Test Plan: 1) Apply patch 2) Turn off AllowOnShelfHolds 3) Create a bib with one item, mark the item as 'on order' 4) Attempt to place a hold on the item, you should now be able to do so -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gitbot@bugs.koha-community. | |org When did the bot| |2013-09-29 last check this| | --- Comment #18 from I'm just a bot <gitbot@bugs.koha-community.org> --- Patch applied cleanly, go forth and signoff -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6918 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, could you take a look at Srdjan's comment? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.