Bug 29168 - Only show "Request article" if allowed on the detail view
Summary: Only show "Request article" if allowed on the detail view
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
: 28360 (view as bug list)
Depends on: 27945
Blocks:
  Show dependency treegraph
 
Reported: 2021-10-05 09:12 UTC by Jonathan Druart
Modified: 2021-12-05 08:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 29168: Only show 'Request article' if allowed on the detail view (1.68 KB, patch)
2021-10-07 14:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29168: Only show 'Request article' if allowed on the detail view (1.77 KB, patch)
2021-10-08 06:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29168: (follow-up) Indentation of next code block (2.41 KB, patch)
2021-10-08 06:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29168: Only show 'Request article' if allowed on the detail view (1.79 KB, patch)
2021-11-01 14:59 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29168: Only show 'Request article' if allowed on the detail view (1.94 KB, patch)
2021-11-08 12:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29168 - Error trace when viewing detail in OPAC when logged in (25.02 KB, text/plain)
2021-12-05 08:39 UTC, David Nind
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-10-05 09:12:18 UTC
Since bug 27945 we can limit article request per patron's category. However the "request article" links (detail and search views) are still active.
We could disable them and add a tooltip.
Comment 1 Marcel de Rooy 2021-10-05 10:06:34 UTC
Are we doing the same for holds now? I think we didnt, we only calculated if we allow when the user clicked further?
Comment 2 Marcel de Rooy 2021-10-05 10:10:31 UTC
Note that we may not even be able to say if a request is possible. We are doing sort of intelligent guess now. You have biblio and item level requests. You depend on multiple circ rules. Etc. It will be too costly to calculate beforehand?
Comment 3 Jonathan Druart 2021-10-05 10:21:44 UTC
Not a lot of cost for the detail view (and we try to do it for holds on this view).
Comment 4 Marcel de Rooy 2021-10-05 11:05:59 UTC
(In reply to Jonathan Druart from comment #3)
> Not a lot of cost for the detail view (and we try to do it for holds on this
> view).

True. I had the results view in mind.
Comment 5 Marcel de Rooy 2021-10-07 13:31:41 UTC
I would suggest to change the title if this is about detail view btw.
Comment 6 Jonathan Druart 2021-10-07 13:51:06 UTC
We can easily hide the link on the search result view in some case (like the limit per category)
Comment 7 Tomás Cohen Arazi 2021-10-07 14:44:17 UTC
Created attachment 125890 [details] [review]
Bug 29168: Only show 'Request article' if allowed on the detail view

This patch adds a check for the patron being able to place the request,
in the case of logged in patron. The anonymous patron will still be
displayed the link, and redirected to the login page as required.

To test:
1. Set the article request limit to 1
2. Place a request on behalf of a known patron
3. With the patron session, in the OPAC, try to place a new article
   request from the detail page
=> SUCCESS: There's no link
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Marcel de Rooy 2021-10-08 06:20:41 UTC
Created attachment 125920 [details] [review]
Bug 29168: Only show 'Request article' if allowed on the detail view

This patch adds a check for the patron being able to place the request,
in the case of logged in patron. The anonymous patron will still be
displayed the link, and redirected to the login page as required.

To test:
1. Set the article request limit to 1
2. Place a request on behalf of a known patron
3. With the patron session, in the OPAC, try to place a new article
   request from the detail page
=> SUCCESS: There's no link
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2021-10-08 06:20:45 UTC
Created attachment 125921 [details] [review]
Bug 29168: (follow-up) Indentation of next code block

Indentationt of lines 811-823 directly after changed code corrected.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2021-10-08 08:47:55 UTC
Comment on attachment 125921 [details] [review]
Bug 29168: (follow-up) Indentation of next code block

Not related changes that can cause unnecessary conflicts.
Comment 11 Marcel de Rooy 2021-10-08 09:19:38 UTC
(In reply to Jonathan Druart from comment #10)
> Comment on attachment 125921 [details] [review] [review]
> Bug 29168: (follow-up) Indentation of next code block
> 
> Not related changes that can cause unnecessary conflicts.

Beetje flauw.
Comment 12 Katrin Fischer 2021-11-01 14:59:20 UTC
Created attachment 127175 [details] [review]
Bug 29168: Only show 'Request article' if allowed on the detail view

This patch adds a check for the patron being able to place the request,
in the case of logged in patron. The anonymous patron will still be
displayed the link, and redirected to the login page as required.

To test:
1. Set the article request limit to 1
2. Place a request on behalf of a known patron
3. With the patron session, in the OPAC, try to place a new article
   request from the detail page
=> SUCCESS: There's no link
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2021-11-01 14:59:42 UTC
I expected a big complicated patch and was pleasantly disappointed.
Comment 14 Jonathan Druart 2021-11-02 11:39:21 UTC
This is getting complicated. I think it should be simplified.

Isn't the following correct/enough?

if ( $patron && $patron->can_request_article ) {
    $artreqpossible = $biblio->can_article_request( $patron );
} elsif ( $itemtype ) {
    $artreqpossible = $itemtype->may_article_request
}
Comment 15 Tomás Cohen Arazi 2021-11-04 11:44:33 UTC
(In reply to Jonathan Druart from comment #14)
> This is getting complicated. I think it should be simplified.
> 
> Isn't the following correct/enough?
> 
> if ( $patron && $patron->can_request_article ) {
>     $artreqpossible = $biblio->can_article_request( $patron );
> } elsif ( $itemtype ) {
>     $artreqpossible = $itemtype->may_article_request
> }

It looks similar to me, a bit shorter. Do you want me to resubmit the patch in this form?
Comment 16 Jonathan Druart 2021-11-04 11:51:17 UTC
Shorter and less unnecessary calls in some cases. Yes please resubmit.
Comment 17 Tomás Cohen Arazi 2021-11-08 12:00:50 UTC
Created attachment 127441 [details] [review]
Bug 29168: Only show 'Request article' if allowed on the detail view

This patch adds a check for the patron being able to place the request,
in the case of logged in patron. The anonymous patron will still be
displayed the link, and redirected to the login page as required.

To test:
1. Set the article request limit to 1
2. Place a request on behalf of a known patron
3. With the patron session, in the OPAC, try to place a new article
   request from the detail page
=> SUCCESS: There's no link
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2021-11-08 14:37:32 UTC
*** Bug 28360 has been marked as a duplicate of this bug. ***
Comment 19 David Nind 2021-12-05 08:39:53 UTC
Created attachment 128251 [details]
Bug 29168 - Error trace when viewing detail in OPAC when logged in

When viewing the detail page for a record in the OPAC when logged in I get the error as attached.

This is after applying the patch, enabling article requests, but before setting any article request limits.

If you are not logged in, the detail page is displayed as expected.