Bug 36281 - Restricted info is displayed even if item is not restricted (OPAC details)
Summary: Restricted info is displayed even if item is not restricted (OPAC details)
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 32445
Blocks:
  Show dependency treegraph
 
Reported: 2024-03-08 10:44 UTC by Fridolin Somers
Modified: 2024-04-02 10:53 UTC (History)
1 user (show)

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


Attachments
Bug 36281: Display restricted info only if item is not restricted (OPAC details) (3.38 KB, patch)
2024-03-08 10:51 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 36281: Display restricted info only if item is not restricted (OPAC details) (3.44 KB, patch)
2024-03-09 18:11 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36281: Display restricted info only if item is not restricted (OPAC details) (3.50 KB, patch)
2024-04-01 13:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2024-03-08 10:44:14 UTC
If you define a description for restricted = 0 : 
In OPAC details, restricted info is displayed even if item is not restricted (value 0).
Comment 1 Fridolin Somers 2024-03-08 10:44:32 UTC
Since Bug 32445
Comment 2 Fridolin Somers 2024-03-08 10:51:48 UTC
Created attachment 162950 [details] [review]
Bug 36281: Display restricted info only if item is not restricted (OPAC details)

If you define a description for restricted = 0 :
In OPAC details, restricted info is displayed even if item is not restricted (value 0).

This patch fixes by adding 'IF item.restricted'.
Also creates a BLOCK to avoid code duplication.
Also adds span around status text to create easier translation.

Test plan :
1) Create a description in restricted autorised values category for value 0
2) Create a description in restricted autorised values category for value 1
3) Edit an biblio record to add an item with restricted=0 and an item with restricted=1
4) Display this biblio on OPAC details page
=> Check you see resticted info only for restricted item
Comment 3 Lucas Gass (lukeg) 2024-03-09 18:11:47 UTC
Created attachment 163009 [details] [review]
Bug 36281: Display restricted info only if item is not restricted (OPAC details)

If you define a description for restricted = 0 :
In OPAC details, restricted info is displayed even if item is not restricted (value 0).

This patch fixes by adding 'IF item.restricted'.
Also creates a BLOCK to avoid code duplication.
Also adds span around status text to create easier translation.

Test plan :
1) Create a description in restricted autorised values category for value 0
2) Create a description in restricted autorised values category for value 1
3) Edit an biblio record to add an item with restricted=0 and an item with restricted=1
4) Display this biblio on OPAC details page
=> Check you see resticted info only for restricted item

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Nick Clemens (kidclamp) 2024-04-01 13:52:36 UTC
Created attachment 164191 [details] [review]
Bug 36281: Display restricted info only if item is not restricted (OPAC details)

If you define a description for restricted = 0 :
In OPAC details, restricted info is displayed even if item is not restricted (value 0).

This patch fixes by adding 'IF item.restricted'.
Also creates a BLOCK to avoid code duplication.
Also adds span around status text to create easier translation.

Test plan :
1) Create a description in restricted autorised values category for value 0
2) Create a description in restricted autorised values category for value 1
3) Edit an biblio record to add an item with restricted=0 and an item with restricted=1
4) Display this biblio on OPAC details page
=> Check you see resticted info only for restricted item

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens (kidclamp) 2024-04-01 13:54:43 UTC
I added my sign off, this does what it says, but I would like the RM to decide

If there is no value defined for zero in RESTRICTED, then the item will be saved as null.
If you add a value, you still need to set the item to '0' and choose the new description.

Before this patch - create a 0, and you can use it or not, and it displays if used

After this patch - create a 0, use it, it doesn't show? That seems inconsistent - can you explain your use case?
Comment 6 Katrin Fischer 2024-04-01 14:01:23 UTC
(In reply to Nick Clemens from comment #5)
> I added my sign off, this does what it says, but I would like the RM to
> decide
> 
> If there is no value defined for zero in RESTRICTED, then the item will be
> saved as null.
> If you add a value, you still need to set the item to '0' and choose the new
> description.
> 
> Before this patch - create a 0, and you can use it or not, and it displays
> if used
> 
> After this patch - create a 0, use it, it doesn't show? That seems
> inconsistent - can you explain your use case?

Hm, this does sound a bit inconsistent.

I'd be interested in how we handle the same for other similar status (WITHDRAWN, DAMAGED, NOTLOAN ...), I think we should keep things consistent.

Also interested in the use case! Why would you define an entry for 0? 

(Failed QA = Waiting for Feedback)
Comment 7 Fridolin Somers 2024-04-02 07:04:21 UTC
Oh I see this in items table :
`restricted` tinyint(1) DEFAULT NULL

But other WITHDRAWN, DAMAGED, LOST have DEFAULT 0.
That explains why authorized value for 0 in not needed.

Some librarians which to see a text in items edition for 0 values, like "Not lost", "Not damaged" ...

Should we change restricted DEFAULT 0 ?
Comment 8 Katrin Fischer 2024-04-02 10:53:08 UTC
Hi Frido, I think that does sound like a better option. And maybe a database update to correct NULL "" to 0?