[Bug 23233] New: AllowItemsOnHoldCheckout is misnamed and does not indicate it is for SIP communication only
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Bug ID: 23233 Summary: AllowItemsOnHoldCheckout is misnamed and does not indicate it is for SIP communication only Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well. As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|AllowItemsOnHoldCheckout is |AllowItemsOnHoldCheckout is |misnamed and does not |misnamed and does not |indicate it is for SIP |indicate it is for |communication only |SIP-based checkouts only -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91114&action=edit Bug 23233: AllowItemsOnHoldCheckout is misnamed and does not indicate it is for SIP-based checkouts only The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well. As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report. Test Plan: 1) Test AllowItemsOnHoldCheckout via SIP 2) Apply this patch 3) Test AllowItemsOnHoldCheckoutSIP via SIP 4) Note there is no difference! 5) Note the syspref description has been update to mention SIP -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion CC| |joonas.kylmala@helsinki.fi --- Comment #2 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- The syspref affects also the code in Koha/Item.pm, function "has_pending_hold". I believe it gets indirectly used in opac/intranet so we can't do this by simply renaming. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21572 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21572 [Bug 21572] Allow checkout an item with reserve that is not yet in waiting state -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #2)
The syspref affects also the code in Koha/Item.pm, function "has_pending_hold". I believe it gets indirectly used in opac/intranet so we can't do this by simply renaming.
The one and only use of this method is in koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc so supressing the pending hold status here based on the system preference should be considered buggy behavior as well! I'll file a followup to remove the use of the syspref for that method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91139 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91139&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from Koha::Item::has_pending_hold, it can cause false data to be returned -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #5 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Kyle M Hall from comment #4)
Created attachment 91139 [details] [review] Bug 23233: Remove use of AllowItemsOnHoldCheckout from Koha::Item::has_pending_hold, it can cause false data to be returned
Thanks. So just to summarize what this does: In koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc it is shown to patron whether the item is available for checkout and without this patch it actually showed whether it is available through SIP server and didn't consider it could have been available through intranet or SCO. This same bug seems to be in two other lines in the same Koha/Items.pm file: C4/Items.pm: $query .= ",IF(tmp_holdsqueue.itemnumber,1,0) AS has_pending_hold" if !C4::Context->preference('AllowItemsOnHoldCheckout'); C4/Items.pm: LEFT JOIN tmp_holdsqueue USING (itemnumber)" if !C4::Context->preference('AllowItemsOnHoldCheckout'); Shouldn't those also be fixed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #5)
(In reply to Kyle M Hall from comment #4)
Created attachment 91139 [details] [review] [review] Bug 23233: Remove use of AllowItemsOnHoldCheckout from Koha::Item::has_pending_hold, it can cause false data to be returned
Thanks. So just to summarize what this does: In koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc it is shown to patron whether the item is available for checkout and without this patch it actually showed whether it is available through SIP server and didn't consider it could have been available through intranet or SCO. This same bug seems to be in two other lines in the same Koha/Items.pm file:
C4/Items.pm: $query .= ",IF(tmp_holdsqueue.itemnumber,1,0) AS has_pending_hold" if !C4::Context->preference('AllowItemsOnHoldCheckout');
C4/Items.pm: LEFT JOIN tmp_holdsqueue USING (itemnumber)" if !C4::Context->preference('AllowItemsOnHoldCheckout');
Shouldn't those also be fixed?
You are absolutely right! Good catch :) I'll update my followup to include those changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91143&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91143|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #8 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Kyle M Hall from comment #7)
Created attachment 91143 [details] [review] Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo
You accidentally obsoleted the new patch instead of old one? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 91144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91144&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #8)
(In reply to Kyle M Hall from comment #7)
Created attachment 91143 [details] [review] [review] Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo
You accidentally obsoleted the new patch instead of old one?
I realized it wasn't complete and I decided to keep it separate for clarity. Thanks for checking! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91114|0 |1 is obsolete| | --- Comment #11 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94526&action=edit Bug 23233: AllowItemsOnHoldCheckout is misnamed and does not indicate it is for SIP-based checkouts only The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well. As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report. Test Plan: 1) Test AllowItemsOnHoldCheckout via SIP 2) Apply this patch 3) Test AllowItemsOnHoldCheckoutSIP via SIP 4) Note there is no difference! 5) Note the syspref description has been update to mention SIP Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91139|0 |1 is obsolete| | --- Comment #12 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94527&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from Koha::Item::has_pending_hold, it can cause false data to be returned Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91144|0 |1 is obsolete| | --- Comment #13 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94528&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #14 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94529 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94529&action=edit Bug 23233: (follow-up) Remove accidentally left comment The line t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckout', 0 ); was removed from below this comment line but the comment explaining what it does was forgotten. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #15 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94530&action=edit Bug 23233: (follow-up) Add description to atomic upgrade -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #16 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 94531 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94531&action=edit Bug 23233: (follow-up) Correct test description AllowItemsOnHoldCheckout is not checked anymore but instead only whether the tmp_holdsqueue is empty or not. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94529|0 |1 is obsolete| | --- Comment #17 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 94556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94556&action=edit Bug 23233: (follow-up) Remove accidentally left comment The line t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckout', 0 ); was removed from below this comment line but the comment explaining what it does was forgotten. Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94530|0 |1 is obsolete| | --- Comment #18 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 94557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94557&action=edit Bug 23233: (follow-up) Add description to atomic upgrade Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94531|0 |1 is obsolete| | --- Comment #19 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 94558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94558&action=edit Bug 23233: (follow-up) Correct test description AllowItemsOnHoldCheckout is not checked anymore but instead only whether the tmp_holdsqueue is empty or not. Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |Failed QA --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Joonas, this one is confusing to me - from the bug title/description it only seemed to affect the description of the pref/naming of the pref, so I expected an easy QA, but there is definitely more going on here. Can you explain why you removed the checks for the prefs in several parts of the code? Especially the changes to GetItemsInfo. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #21 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Katrin Fischer from comment #20)
Hi Joonas,
this one is confusing to me - from the bug title/description it only seemed to affect the description of the pref/naming of the pref, so I expected an easy QA, but there is definitely more going on here.
Can you explain why you removed the checks for the prefs in several parts of the code? Especially the changes to GetItemsInfo.
Does the comment #6 explain (and the ones before that)? Kyle, maybe you would like to improve the commit messages? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #21)
(In reply to Katrin Fischer from comment #20)
Hi Joonas,
this one is confusing to me - from the bug title/description it only seemed to affect the description of the pref/naming of the pref, so I expected an easy QA, but there is definitely more going on here.
Can you explain why you removed the checks for the prefs in several parts of the code? Especially the changes to GetItemsInfo.
Does the comment #6 explain (and the ones before that)?
Kyle, maybe you would like to improve the commit messages?
I've re-read the commit message and even with the followups if still reads accurately. What happened is that Joonas noticed that some places in Koha would present certain data only if AllowItemsOnHoldCheckoutSIP although that data should be displayed regardless of AllowItemsOnHoldCheckoutSIP! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|AllowItemsOnHoldCheckout is |AllowItemsOnHoldCheckout is |misnamed and does not |misnamed and should only |indicate it is for |work for for SIP-based |SIP-based checkouts only |checkouts Status|Failed QA |Signed Off QA Contact|testopia@bugs.koha-communit | |y.org | --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Switching back to Signed off. Changes make sense to me now, tests pass, database update is ok, but I can't test the SIP part - can someone take over from here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #24 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I do not understand the following change: +++ b/Koha/Item.pm @@ -382,7 +382,7 @@ This method checks the tmp_holdsqueue to see if this item has been selected for sub has_pending_hold { my ( $self ) = @_; my $pending_hold = $self->_result->tmp_holdsqueues; - return !C4::Context->preference('AllowItemsOnHoldCheckout') && $pending_hold->count ? 1: 0; + return $pending_hold->count ? 1: 0; } I also do not understand what it meant originally. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This needs feedback from the author. See previous question of Jonathan. Changing status. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #26 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Jonathan Druart from comment #24)
I do not understand the following change:
+++ b/Koha/Item.pm @@ -382,7 +382,7 @@ This method checks the tmp_holdsqueue to see if this item has been selected for sub has_pending_hold { my ( $self ) = @_; my $pending_hold = $self->_result->tmp_holdsqueues; - return !C4::Context->preference('AllowItemsOnHoldCheckout') && $pending_hold->count ? 1: 0; + return $pending_hold->count ? 1: 0; }
I also do not understand what it meant originally.
This was explained in more detail in the comment number 3. I'm switching this back to Signed Off, OK? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #27 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Jonathan confirmed on IRC that it is clear now so QA please review this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94526|0 |1 is obsolete| | Attachment #94527|0 |1 is obsolete| | Attachment #94528|0 |1 is obsolete| | Attachment #94556|0 |1 is obsolete| | Attachment #94557|0 |1 is obsolete| | Attachment #94558|0 |1 is obsolete| | --- Comment #28 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96247&action=edit Bug 23233: AllowItemsOnHoldCheckout is misnamed and does not indicate it is for SIP-based checkouts only The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well. As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report. Test Plan: 1) Test AllowItemsOnHoldCheckout via SIP 2) Apply this patch 3) Test AllowItemsOnHoldCheckoutSIP via SIP 4) Note there is no difference! 5) Note the syspref description has been update to mention SIP Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #29 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96248 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96248&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from Koha::Item::has_pending_hold, it can cause false data to be returned Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96249 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96249&action=edit Bug 23233: Remove use of AllowItemsOnHoldCheckout from C4::Items::GetItemsInfo Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96250 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96250&action=edit Bug 23233: (follow-up) Remove accidentally left comment The line t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckout', 0 ); was removed from below this comment line but the comment explaining what it does was forgotten. Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96251 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96251&action=edit Bug 23233: (follow-up) Add description to atomic upgrade Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96252 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96252&action=edit Bug 23233: (follow-up) Correct test description AllowItemsOnHoldCheckout is not checked anymore but instead only whether the tmp_holdsqueue is empty or not. Signed-off-by: Lari Taskula <lari.taskula@hypernova.fi> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I did not test deeply this patchset but the changes make sense to me. A quick note: Koha::Item->has_pending_hold should be replaced by Koha::Item->pending_holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 96345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96345&action=edit Bug 23233: Fix UsageStats.t Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 96358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96358&action=edit Bug 23233: (RM follow-up) Rename syspref in test The AllowItemsOnHoldCheckout system preference was renamed to AllowItemsOnHoldCheckoutSIP to clarify it's use, but one case was missed in the test suit. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #38 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Ooops. Totally missed that you had also provided that exact same followup Jonathan :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com Version(s)|20.05.00 |20.05.00,, 19.11.02 released in| | Status|Pushed to master |Pushed to stable --- Comment #40 from Joy Nelson <joy@bywatersolutions.com> --- Pushed to 19.11.x branch for 19.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |lucas@bywatersolutions.com --- Comment #41 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 19.05.x for 19.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 Hayley Mapley <hayleymapley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hayleymapley@catalyst.net.n | |z Version(s)|20.05.00,, 19.11.02 |20.05.00, 19.11.02, released in| |19.05.08 --- Comment #42 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- Enhancement will not be backported to 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23233 --- Comment #43 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** Bug 23223 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org