Bug 33933

Summary:	Use restrictions appear twice for items on OPAC
Product:	Koha	Reporter:	David Cook <dcook>
Component:	OPAC	Assignee:	David Cook <dcook>
Status:	RESOLVED FIXED	QA Contact:	Katrin Fischer <katrin.fischer>
Severity:	minor
Priority:	P5 - low	CC:	eric.balluto, jonathan.druart
Version:	Main
Hardware:	All
OS:	All
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
Circulation function:
Bug Depends on:	32611
Bug Blocks:
Attachments:	Bug 33933: Only show use restriction once on OPAC detail page Bug 33933: Only show use restriction once on OPAC detail page Bug 33933: Only show use restriction once on OPAC detail page

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 (tcohen) 2023-07-12 17:31:56 UTC

Pushed to master for 23.11.

Nice work everyone, thanks!

Comment 11 Martin Renvoize (ashimema) 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