Bug 33933 - Use restrictions appear twice for items on OPAC
Summary: Use restrictions appear twice for items on OPAC
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 32611
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-07 04:26 UTC by David Cook
Modified: 2023-12-28 20:47 UTC (History)
2 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:
23.11.00,23.05.02,22.11.08


Attachments
Bug 33933: Only show use restriction once on OPAC detail page (1.55 KB, patch)
2023-06-07 05:33 UTC, David Cook
Details | Diff | Splinter Review
Bug 33933: Only show use restriction once on OPAC detail page (1.60 KB, patch)
2023-06-30 10:09 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 33933: Only show use restriction once on OPAC detail page (1.65 KB, patch)
2023-07-09 14:02 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2023-06-07 04:26:20 UTC
Bug 32611 fixed a problem with restricted_lib and notforloan_lib not being set correctly.

However, it also introduced another span.restricted, when that information is already shown next to "Available".
Comment 1 David Cook 2023-06-07 05:33:33 UTC
Created attachment 152073 [details] [review]
Bug 33933: Only show use restriction once on OPAC detail page

This patch removes a regression from bug 32611 that caused the
use restriction text to be shown twice for "Available" items.

Test plan:
0. Apply patch
1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
2. Edit item and add "Restricted access" for "5 - Use restrictions"
3. Go to http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=29
4. Note that "Status" says "Available (Restricted access)" and there is no
duplicate "Restricted access" at the top of that table cell
Comment 2 Jonathan Druart 2023-06-07 07:14:46 UTC
It looks correct but I am not confident in signing it off. The code is confusing!

How did we miss that on bug 32611?
Comment 3 David Cook 2023-06-08 01:13:41 UTC
(In reply to Jonathan Druart from comment #2)
> It looks correct but I am not confident in signing it off. The code is
> confusing!

I agree the code is confusing. At some point, I think we need to move more of this status information lower in the code, so that we're showing the same thing via all controllers (e.g. detail page, API, ILS-DI, etc). 

> How did we miss that on bug 32611?

I think only the "Not for loan" was tested. Since "Not for loan" turns off "Available", this regression wouldn't have appeared during testing. 

That said, bug 32611 says "The patch also adds another case for displaying the item restricted status. Previously items.restricted would only display if the item *also* had a notforloan value."

I don't think that's true, as I've been using use restrictions for quite a few years, and they've shown up for "available" items. (Many of my libraries use "Photocopy only" or "Electronic" use restrictions.)
Comment 4 David Cook 2023-06-08 01:33:25 UTC
Then again... if we look at C4::Circulation::CanBookBeIssued, we can see the following:

    if (   $item_object->restricted
        && $item_object->restricted == 1 )
    {
        $issuingimpossible{RESTRICTED} = 1;
    }

So if you can't check it out, it probably shouldn't say "Available"...

Item status drives me a little crazy...
Comment 5 Katrin Fischer 2023-06-08 07:58:48 UTC
Available can be on the shelf... usable in the library, not necessarily for loan.
Comment 6 David Cook 2023-06-09 00:37:02 UTC
(In reply to Katrin Fischer from comment #5)
> Available can be on the shelf... usable in the library, not necessarily for
> loan.

Mmm that's true and I think that's how we've been using it for years in our libraries.

I think the search results say "Available for loan", although I've customized that locally to things like "Available electronically" or "Available for photocopy" depending on the use restriction...
Comment 7 Biblibre Sandboxes 2023-06-30 10:09:17 UTC
Created attachment 152901 [details] [review]
Bug 33933: Only show use restriction once on OPAC detail page

This patch removes a regression from bug 32611 that caused the
use restriction text to be shown twice for "Available" items.

Test plan:
0. Apply patch
1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
2. Edit item and add "Restricted access" for "5 - Use restrictions"
3. Go to http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=29
4. Note that "Status" says "Available (Restricted access)" and there is no
duplicate "Restricted access" at the top of that table cell

Signed-off-by: ebal <eric.balluto@univ-lyon2.fr>
Comment 8 Eric Balluto 2023-06-30 10:12:09 UTC
marked signed off as the test plan was successfull. The question of the code quality is left to the QA step. The bug doesn't need to be in the need SO queue.
Comment 9 Katrin Fischer 2023-07-09 14:02:22 UTC
Created attachment 153217 [details] [review]
Bug 33933: Only show use restriction once on OPAC detail page

This patch removes a regression from bug 32611 that caused the
use restriction text to be shown twice for "Available" items.

Test plan:
0. Apply patch
1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
2. Edit item and add "Restricted access" for "5 - Use restrictions"
3. Go to http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=29
4. Note that "Status" says "Available (Restricted access)" and there is no
duplicate "Restricted access" at the top of that table cell

Signed-off-by: ebal <eric.balluto@univ-lyon2.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Tomás Cohen Arazi 2023-07-12 17:31:56 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 11 Martin Renvoize 2023-07-18 10:08:31 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 12 Matt Blenkinsop 2023-07-18 14:23:48 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x