https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14610 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14610 [Bug 14610] Add ability to place article requests in Koha -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- I've been looking at Search.pm, it looks like the "Place hold" links display unless the item is not lost, withdrawn, damaged or notforloan. I don't think we should limit it by notforloan for reasons I think are pretty clear. Other than that I'd say we follow the lost/withdrawn/damaged logic that exists. Would you agree? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 72858 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72858&action=edit Bug 17530: Use may_article_request method in opac-search If you enable pref ArticleRequests, until now all search results got the 'Request article' link. This patch tries to improve the situation by using the new sub with the itemtype of the search result. In most cases the number of links will drastically decrease. It may still be possible sometimes that a link is shown while it effectively is not possible, but we do not get the performance burden of determining that and going through all items. Test plan: [1] Look for two borrowers P1 and P2 within categories C1 resp. C2. [2] Look for two biblios B1 and B2 with default item types I1 resp. I2. (See 942c in case of MARC21.) [3] Make sure that no circ rules allow article requests. Enable the pref. [4] Add/modify circ rule category=C1, itemtype=I1, art_req=yes. Log out. Search for B1 and B2, verify that only B1 has AR link. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that no biblio has AR link. [5] Add/modify circ rule category=C2, itemtype=I2, art_req=item_only. Log out. Search for B1 and B2, verify that both have AR links. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that only B2 has AR link. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72859|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 72860 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72860&action=edit Bug 17530: Use can_article_request to control sidebar link Before this patch, the 'Request article' link is displayed whenever the pref is enabled. In many cases this might be useless. Instead of a guess as in opac-search, we now call can_article_request to know for sure. Note: at least this is the case when a user has logged in. Update sidebar template with template variable artreqpossible. Add code in opac-detail, MARCdetail and ISBDdetail to fill it. Test plan: [1] Look for two biblios with items: one that should allow article requests and one that should not (respecting branch, patron, item type). [2] Verify on detail, ISBD and MARC that the link is displayed for the first biblio and hidden for the second biblio. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72858|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 72910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72910&action=edit Bug 17530: Use may_article_request method in opac-search If you enable pref ArticleRequests, until now all search results got the 'Request article' link. This patch tries to improve the situation by using the new sub with the itemtype of the search result. In most cases the number of links will drastically decrease. It may still be possible sometimes that a link is shown while it effectively is not possible, but we do not get the performance burden of determining that and going through all items. Test plan: [1] Look for two borrowers P1 and P2 within categories C1 resp. C2. [2] Look for two biblios B1 and B2 with default item types I1 resp. I2. (See 942c in case of MARC21.) [3] Make sure that no circ rules allow article requests. Enable the pref. [4] Add/modify circ rule category=C1, itemtype=I1, art_req=yes. Log out. Search for B1 and B2, verify that only B1 has AR link. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that no biblio has AR link. [5] Add/modify circ rule category=C2, itemtype=I2, art_req=item_only. Log out. Search for B1 and B2, verify that both have AR links. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that only B2 has AR link. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Fixed a warning like: Use of uninitialized value $itemtype in hash element at /usr/share/koha/shadowclone/Koha/Biblio.pm line 125. Acquisition records may not yet have a 942c (default item type). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Axelle Aix-Marseille Université <axelle.clarisse@univ-amu.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |axelle.clarisse@univ-amu.fr Status|Needs Signoff |Failed QA --- Comment #14 from Axelle Aix-Marseille Université <axelle.clarisse@univ-amu.fr> --- Tested on sandbox Biblibre n°11 The patch doesn't work : the link remains on all pages (normal, ISBD, marc) and all biblios on the OPAC and in the staff client. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Axelle Aix-Marseille Université from comment #14)

Tested on sandbox Biblibre n°11

The patch doesn't work : the link remains on all pages (normal, ISBD, marc) and all biblios on the OPAC and in the staff client.

Please explain what you did. Article requests / circ rules may not be trivial. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72909|0 |1 is obsolete| | --- Comment #16 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 72913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72913&action=edit Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes This routine looks at the issuingrules and makes a fast 'intelligent guess' if article requests may be allowed for item types while branch code and patron category may not be available. We also add Koha::Biblio->may_article_request using the routine based on default item type or parameter. Implemented as both class and instance method. Added a few tests. Test plan: [1] Run t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t [2] Run t/db_dependent/ArticleRequests.t to test Koha::Biblio changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72912|0 |1 is obsolete| | --- Comment #18 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 72915 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72915&action=edit Bug 17530: Use can_article_request to control sidebar link Before this patch, the 'Request article' link is displayed whenever the pref is enabled. In many cases this might be useless. Instead of a guess as in opac-search, we now call can_article_request to know for sure. Note: at least this is the case when a user has logged in. Update sidebar template with template variable artreqpossible. Add code in opac-detail, MARCdetail and ISBDdetail to fill it. Test plan: [1] Look for two biblios with items: one that should allow article requests and one that should not (respecting branch, patron, item type). [2] Verify on detail, ISBD and MARC that the link is displayed for the first biblio and hidden for the second biblio. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 72930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72930&action=edit Bug 17530: (QA follow-up) Replace our variable by cached value Instead of saving the state locally in a variable during Plack lifetime, we move the saved hash to the cache. We clear the key when we enter smart-rules.pl. This makes a change in circ rules immediately effective. Test plan: Run the modified tests. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #19)

Note for QA: Saving state in Plack makes that changing the circ rules needs either restarting Plack or waiting until Plack served a configured number of requests per worker. Will still improve this.

QA: If you feel uncomfortable with the last follow-up, feel free to obsolete. I can move it to another report too. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Keywords| |NeedsMoreQA --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have not tested it, just had a quick look. 1. This: Koha::Biblio->may_article_request shows that Koha::Biblio is not the right place to put it. Koha::Object class should not have class methods. 2. In Koha::IssuingRules, $class_or_self must be named $class 3. The cache is cleared in a very lazy way :) 4. Maybe we do not need may_article_request, but only guess_article_requestable_itemtypes? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 75697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75697&action=edit Bug 17530: (QA follow-up) Replace class_or_self variables See comment23. Requested by QA. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #26)

(In reply to Marcel de Rooy from comment #24)

...

(In reply to Jonathan Druart from comment #23)

...

I have not tested it, just had a quick look. Thats a good start ;)

...

1. This: Koha::Biblio->may_article_request shows that Koha::Biblio is not the right place to put it. Koha::Object class should not have class methods. Yes, this was arbitrary. I could move it to Koha::ItemType. What do you think? 4. Maybe we do not need may_article_request, but only guess_article_requestable_itemtypes? See point 1.

If we remove it, there is no need to move it... I do not understand your point.

I do not want to remove it. I read: "I could move it to Koha::ItemType". Move is not remove? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #29 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #27)

Move is not remove?

I suggested to remove it: "4. Maybe we do not need may_article_request, but only guess_article_requestable_itemtypes?" -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #29)

(In reply to Marcel de Rooy from comment #27)

...

Move is not remove?

I suggested to remove it: "4. Maybe we do not need may_article_request, but only guess_article_requestable_itemtypes?"

Moved to Koha::ItemType and removed in opac-search.pl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72914|0 |1 is obsolete| | --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 76618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76618&action=edit Bug 17530: Use may_article_request method in opac-search If you enable pref ArticleRequests, until now all search results got the 'Request article' link. This patch tries to improve the situation by using the new sub with the itemtype of the search result. In most cases the number of links will drastically decrease. It may still be possible sometimes that a link is shown while it effectively is not possible, but we do not get the performance burden of determining that and going through all items. Test plan: [1] Look for two borrowers P1 and P2 within categories C1 resp. C2. [2] Look for two biblios B1 and B2 with default item types I1 resp. I2. (See 942c in case of MARC21.) [3] Make sure that no circ rules allow article requests. Enable the pref. [4] Add/modify circ rule category=C1, itemtype=I1, art_req=yes. Log out. Search for B1 and B2, verify that only B1 has AR link. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that no biblio has AR link. [5] Add/modify circ rule category=C2, itemtype=I2, art_req=item_only. Log out. Search for B1 and B2, verify that both have AR links. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that only B2 has AR link. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72931|0 |1 is obsolete| | --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 76620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76620&action=edit Bug 17530: (QA follow-up) Replace our variable by cached value Instead of saving the state locally in a variable during Plack lifetime, we move the saved hash to the cache. We clear the key when we enter smart-rules.pl. This makes a change in circ rules immediately effective. Test plan: Run the modified tests. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75751|0 |1 is obsolete| | --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 76622 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76622&action=edit Bug 17530: (QA follow-up) Move may_article_request to ItemType As requested by QA, we should move may_article_request out of Biblio. For reasons of performance removed the wrapper layer of may_article_request in opac-search. No need to look up all item types. For readability kept the routine in the detail scripts. Note for running ArticleRequests.t: A possible failure on the subtest 'search_limited' is addressed on bug 20866. So you can ignore that one. As long as the subtest for may_article_request passes. Test plan: See previous patches. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76617|0 |1 is obsolete| | --- Comment #39 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 77258 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77258&action=edit Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes This routine looks at the issuingrules and makes a fast 'intelligent guess' if article requests may be allowed for item types while branch code and patron category may not be available. We also add Koha::Biblio->may_article_request using the routine based on default item type or parameter. Implemented as both class and instance method. Added a few tests. Test plan: [1] Run t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t [2] Run t/db_dependent/ArticleRequests.t to test Koha::Biblio changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76619|0 |1 is obsolete| | --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 77260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77260&action=edit Bug 17530: Use can_article_request to control sidebar link Before this patch, the 'Request article' link is displayed whenever the pref is enabled. In many cases this might be useless. Instead of a guess as in opac-search, we now call can_article_request to know for sure. Note: at least this is the case when a user has logged in. Update sidebar template with template variable artreqpossible. Add code in opac-detail, MARCdetail and ISBDdetail to fill it. Test plan: [1] Look for two biblios with items: one that should allow article requests and one that should not (respecting branch, patron, item type). [2] Verify on detail, ISBD and MARC that the link is displayed for the first biblio and hidden for the second biblio. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76621|0 |1 is obsolete| | --- Comment #43 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 77262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77262&action=edit Bug 17530: (QA follow-up) Replace class_or_self variables See comment23. Requested by QA. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #45 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Simple rebase on Koha/Biblio.pm QA still needs to be finished ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77258|0 |1 is obsolete| | --- Comment #46 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 78030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78030&action=edit Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes This routine looks at the issuingrules and makes a fast 'intelligent guess' if article requests may be allowed for item types while branch code and patron category may not be available. We also add Koha::Biblio->may_article_request using the routine based on default item type or parameter. Implemented as both class and instance method. Added a few tests. Test plan: [1] Run t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t [2] Run t/db_dependent/ArticleRequests.t to test Koha::Biblio changes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77260|0 |1 is obsolete| | --- Comment #48 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 78032 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78032&action=edit Bug 17530: Use can_article_request to control sidebar link Before this patch, the 'Request article' link is displayed whenever the pref is enabled. In many cases this might be useless. Instead of a guess as in opac-search, we now call can_article_request to know for sure. Note: at least this is the case when a user has logged in. Update sidebar template with template variable artreqpossible. Add code in opac-detail, MARCdetail and ISBDdetail to fill it. Test plan: [1] Look for two biblios with items: one that should allow article requests and one that should not (respecting branch, patron, item type). [2] Verify on detail, ISBD and MARC that the link is displayed for the first biblio and hidden for the second biblio. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77262|0 |1 is obsolete| | --- Comment #50 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 78034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78034&action=edit Bug 17530: (QA follow-up) Replace class_or_self variables See comment23. Requested by QA. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77263|0 |1 is obsolete| | --- Comment #51 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 78035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78035&action=edit Bug 17530: (QA follow-up) Move may_article_request to ItemType As requested by QA, we should move may_article_request out of Biblio. For reasons of performance removed the wrapper layer of may_article_request in opac-search. No need to look up all item types. For readability kept the routine in the detail scripts. Note for running ArticleRequests.t: A possible failure on the subtest 'search_limited' is addressed on bug 20866. So you can ignore that one. As long as the subtest for may_article_request passes. Test plan: See previous patches. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion CC| |nick@bywatersolutions.com --- Comment #53 from Nick Clemens <nick@bywatersolutions.com> --- The code here seems solid, but I worry about false negatives when itemtypes/itypes might be inconsistent (i.e. initial result doesn't have a link so patron never clicks in to record to see that they can place a request) Can a syspref be added to control this? Something like "ArticleRequests: Show article request link 'Always/Use best guess' " -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78031|0 |1 is obsolete| | --- Comment #55 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 78363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78363&action=edit Bug 17530: Use may_article_request method in opac-search If you enable pref ArticleRequests, until now all search results got the 'Request article' link. This patch tries to improve the situation by using the new sub with the itemtype of the search result. In most cases the number of links will drastically decrease. It may still be possible sometimes that a link is shown while it effectively is not possible, but we do not get the performance burden of determining that and going through all items. Test plan: [1] Look for two borrowers P1 and P2 within categories C1 resp. C2. [2] Look for two biblios B1 and B2 with default item types I1 resp. I2. (See 942c in case of MARC21.) [3] Make sure that no circ rules allow article requests. Enable the pref. [4] Add/modify circ rule category=C1, itemtype=I1, art_req=yes. Log out. Search for B1 and B2, verify that only B1 has AR link. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that no biblio has AR link. [5] Add/modify circ rule category=C2, itemtype=I2, art_req=item_only. Log out. Search for B1 and B2, verify that both have AR links. Log in as P1. Verify that only B1 has AR link. Log in as P2. Verify that only B2 has AR link. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78033|0 |1 is obsolete| | --- Comment #57 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 78365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78365&action=edit Bug 17530: (QA follow-up) Replace our variable by cached value Instead of saving the state locally in a variable during Plack lifetime, we move the saved hash to the cache. We clear the key when we enter smart-rules.pl. This makes a change in circ rules immediately effective. Test plan: Run the modified tests. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78035|0 |1 is obsolete| | --- Comment #59 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 78367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78367&action=edit Bug 17530: (QA follow-up) Move may_article_request to ItemType As requested by QA, we should move may_article_request out of Biblio. For reasons of performance removed the wrapper layer of may_article_request in opac-search. No need to look up all item types. For readability kept the routine in the detail scripts. Note for running ArticleRequests.t: A possible failure on the subtest 'search_limited' is addressed on bug 20866. So you can ignore that one. As long as the subtest for may_article_request passes. Test plan: See previous patches. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|NeedsMoreQA | Status|In Discussion |Passed QA --- Comment #61 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Back in the PQA queue. Removed some lines from patch 6 and added patch 7 in order to meet comment53. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 --- Comment #63 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #62)

(In reply to Marcel de Rooy from comment #61)

...

Back in the PQA queue. Removed some lines from patch 6 and added patch 7 in order to meet comment53.

These lines don't seem correct, I keep getting a 'Can't use string ("1") as a hashref' 174 return $last_article_requestable_guesses->{$key} 175 if $last_article_requestable_guesses && exists $last_article_requestable_guesses->{$key};

Should that be "return $last_article_requestable_guesses"?

Did you flush the cache? Is memcache causing this problem ? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21475 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21475 [Bug 21475] Error in the OPAC when viewing a record which has no biblio-level itemtype -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17530 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.