Bug 17530 - Don't show 'article request' link when no article requests are permitted
Summary: Don't show 'article request' link when no article requests are permitted
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords: NeedsMoreQA
Depends on: 14610
Blocks:
  Show dependency treegraph
 
Reported: 2016-11-01 14:00 UTC by Katrin Fischer
Modified: 2018-07-02 11:57 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes (9.80 KB, patch)
2018-03-14 13:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use may_article_request method in opac-search (4.10 KB, patch)
2018-03-14 13:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.11 KB, patch)
2018-03-14 13:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.17 KB, patch)
2018-03-14 13:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes (9.80 KB, patch)
2018-03-15 08:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use may_article_request method in opac-search (4.10 KB, patch)
2018-03-15 08:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.11 KB, patch)
2018-03-15 08:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.17 KB, patch)
2018-03-15 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes (9.83 KB, patch)
2018-03-15 09:39 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17530: Use may_article_request method in opac-search (4.13 KB, patch)
2018-03-15 09:40 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.19 KB, patch)
2018-03-15 09:40 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Replace our variable by cached value (6.28 KB, patch)
2018-03-15 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Replace our variable by cached value (6.34 KB, patch)
2018-03-15 11:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Replace class_or_self variables (1.71 KB, patch)
2018-05-31 07:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Move may_article_request to ItemType (8.43 KB, patch)
2018-06-04 10:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Add Koha::IssuingRules->guess_article_requestable_itemtypes (9.85 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use may_article_request method in opac-search (4.17 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: Use can_article_request to control sidebar link (4.23 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Replace our variable by cached value (6.34 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Replace class_or_self variables (1.71 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17530: (QA follow-up) Move may_article_request to ItemType (8.43 KB, patch)
2018-07-02 11:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2016-11-01 14:00:51 UTC
Don't show the article request link on records where there is no article request possible.

At the moment the article request link shows on all records, no matter if the circulation conditions allow for an article request or not.

We should not show the link when an article request is in general not possible.
Comment 1 Kyle M Hall 2016-11-10 02:10:36 UTC
Please correct me if I'm wrong, but don't we always show the place hold link?
Comment 2 Kyle M Hall 2016-11-10 11:54:04 UTC
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?
Comment 3 Katrin Fischer 2016-11-10 12:02:18 UTC
Hi Kyle, please ignore the enh request for now - I will write up what I mean. We used to have better control of the place hold links in the results list - but we lost this a while ago with another development. I think the bugs with the tests and CSS need to be fixed first!
Comment 4 Marcel de Rooy 2018-03-01 14:44:42 UTC
Will provide an 'intelligent guess' at search results without the performance penalty of going through all items. Detail can be more precise.
Comment 5 Marcel de Rooy 2018-03-14 13:12:42 UTC
Created attachment 72857 [details] [review]
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>
Comment 6 Marcel de Rooy 2018-03-14 13:12:45 UTC
Created attachment 72858 [details] [review]
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>
Comment 7 Marcel de Rooy 2018-03-14 13:12:49 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2018-03-14 13:13:56 UTC
Created attachment 72860 [details] [review]
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>
Comment 9 Marcel de Rooy 2018-03-15 08:48:13 UTC
Created attachment 72909 [details] [review]
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>
Comment 10 Marcel de Rooy 2018-03-15 08:48:17 UTC
Created attachment 72910 [details] [review]
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>
Comment 11 Marcel de Rooy 2018-03-15 08:48:21 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2018-03-15 08:51:34 UTC
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).
Comment 13 Marcel de Rooy 2018-03-15 08:53:17 UTC
Created attachment 72912 [details] [review]
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>
Comment 14 Axelle Aix-Marseille Université 2018-03-15 08:56:06 UTC
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.
Comment 15 Marcel de Rooy 2018-03-15 08:59:35 UTC
(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.
Comment 16 Brendan Gallagher 2018-03-15 09:39:55 UTC
Created attachment 72913 [details] [review]
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>
Comment 17 Brendan Gallagher 2018-03-15 09:40:08 UTC
Created attachment 72914 [details] [review]
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>
Comment 18 Brendan Gallagher 2018-03-15 09:40:12 UTC
Created attachment 72915 [details] [review]
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>
Comment 19 Marcel de Rooy 2018-03-15 09:53:19 UTC
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.
Comment 20 Marcel de Rooy 2018-03-15 11:14:59 UTC Comment hidden (obsolete)
Comment 21 Marcel de Rooy 2018-03-15 11:15:29 UTC
Created attachment 72931 [details] [review]
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>
Comment 22 Marcel de Rooy 2018-03-15 11:16:37 UTC
(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.
Comment 23 Jonathan Druart 2018-05-28 19:20:44 UTC
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?
Comment 24 Marcel de Rooy 2018-05-31 07:56:34 UTC
(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?

> 2. In Koha::IssuingRules, $class_or_self must be named $class
Will fix that in a follow-up.

> 3. The cache is cleared in a very lazy way :)
Lazy is good enough here imo.

> 4. Maybe we do not need may_article_request, but only
> guess_article_requestable_itemtypes?
See point 1.
Comment 25 Marcel de Rooy 2018-05-31 07:57:32 UTC
Created attachment 75697 [details] [review]
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>
Comment 26 Jonathan Druart 2018-05-31 20:09:51 UTC
(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.


> > 3. The cache is cleared in a very lazy way :)
> Lazy is good enough here imo.

I do not think so, the cache is flushed every time the page is hit.
Comment 27 Marcel de Rooy 2018-06-01 06:37:36 UTC
(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?
Comment 28 Marcel de Rooy 2018-06-01 07:01:23 UTC
(In reply to Jonathan Druart from comment #26)
> > > 3. The cache is cleared in a very lazy way :)
> > Lazy is good enough here imo.
> 
> I do not think so, the cache is flushed every time the page is hit.

The L1 cache is cleared. I do not see the need to remove it frequently from memcached. If we change smart rules, it is cleared (even more strict than needed; read: programmer's lazy). This cache is just for faster presentation, no threat for bad data or so.
Comment 29 Jonathan Druart 2018-06-01 13:49:18 UTC
(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?"
Comment 30 Marcel de Rooy 2018-06-04 10:20:17 UTC
Created attachment 75751 [details] [review]
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>
Comment 31 Marcel de Rooy 2018-06-04 10:22:05 UTC
(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
Comment 32 Marcel de Rooy 2018-07-02 11:56:05 UTC
Created attachment 76617 [details] [review]
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>
Comment 33 Marcel de Rooy 2018-07-02 11:56:10 UTC
Created attachment 76618 [details] [review]
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>
Comment 34 Marcel de Rooy 2018-07-02 11:56:15 UTC
Created attachment 76619 [details] [review]
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>
Comment 35 Marcel de Rooy 2018-07-02 11:56:20 UTC
Created attachment 76620 [details] [review]
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>
Comment 36 Marcel de Rooy 2018-07-02 11:56:24 UTC
Created attachment 76621 [details] [review]
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>
Comment 37 Marcel de Rooy 2018-07-02 11:56:29 UTC
Created attachment 76622 [details] [review]
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>
Comment 38 Marcel de Rooy 2018-07-02 11:57:47 UTC
Jonathan: Did you see the last changes after your comment ?