Bug 36281

Since Bug 32445

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

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>

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>

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?

(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)

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 ?

Hi Frido, I think that does sound like a better option. And maybe a database update to correct NULL "" to 0?