Bug 19673

Summary: Patron batch modification tool cannot use authorised value "0"
Product: Koha Reporter: Emma Perks <emma.perks>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: charles.farmer, dcook, emma.perks, fridolin.somers, gmcharlt, jonathan.druart, kyle.m.hall, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 23217, 23218    
Attachments: Bug 19673: Allow to set patron attributes to 0 with batch patron modification
Bug 19673: Allow to set patron attributes to 0 with batch patron modification
Bug 19673: Allow to set patron attributes to 0 with batch patron modification

Description Emma Perks 2017-11-21 15:57:35 UTC
If a patron attribute has the authorized value 0, using the Batch patron modification tool does not save the value. 
Test case: Authorized values category YES_NO; No=0, Yes=1
Set patron attribute type to use the YES_NO category
Tools Batch patron modification
Select patrons
Set attribute to 0
Save
The attribute has not saved

Many thanks.
Comment 1 Jonathan Druart 2017-12-21 00:40:21 UTC
Created attachment 69984 [details] [review]
Bug 19673: Allow to set patron attributes to 0 with batch patron modification

This patch will have to be tested deeply to make sure it will not
introduce regression!

The idea is to display an empty option in the patron attributes select
and ignore it. That way we can deal with false values 0 and "" which
were skipped before.

Test plan:
Add several patron attributes
Use the batch patron modification tool to add/update/remove them
Play with empty "" and 0 values, as well as other values
Modify several attributes in a row
Comment 2 Charles Farmer 2018-03-23 20:40:28 UTC
Created attachment 73223 [details] [review]
Bug 19673: Allow to set patron attributes to 0 with batch patron modification

This patch will have to be tested deeply to make sure it will not
introduce regression!

The idea is to display an empty option in the patron attributes select
and ignore it. That way we can deal with false values 0 and "" which
were skipped before.

Test plan:
Add several patron attributes
Use the batch patron modification tool to add/update/remove them
Play with empty "" and 0 values, as well as other values
Modify several attributes in a row

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 3 Charles Farmer 2018-03-23 20:44:50 UTC
Could reproduce the original issue with newly created attributes. The values 0 and empty string ('') were not considered during update.

Applied patch, and now I could update with both 0 and an empty field.

Patch set to 'Signed off'
Comment 4 Katrin Fischer 2018-03-28 05:46:35 UTC
Created attachment 73367 [details] [review]
Bug 19673: Allow to set patron attributes to 0 with batch patron modification

This patch will have to be tested deeply to make sure it will not
introduce regression!

The idea is to display an empty option in the patron attributes select
and ignore it. That way we can deal with false values 0 and "" which
were skipped before.

Test plan:
Add several patron attributes
Use the batch patron modification tool to add/update/remove them
Play with empty "" and 0 values, as well as other values
Modify several attributes in a row

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Jonathan Druart 2018-03-29 15:02:59 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 6 Nick Clemens 2018-05-14 21:24:49 UTC
Pushed to stable for 17.11.06

Awesome work all!
Comment 7 Fridolin Somers 2018-05-22 09:28:57 UTC
Pushed to 17.05.x for v17.05.12
Comment 8 David Cook 2019-06-26 07:02:44 UTC
I think that I may have found a regression caused by this patch:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23217

Basically I'm getting database errors when I try to do Batch Patron Modification and there is no borrower attribute provided, although there are borrower attribute codes on the system so the select menu appears.
Comment 9 David Cook 2019-06-26 07:09:53 UTC
(In reply to David Cook from comment #8)
> I think that I may have found a regression caused by this patch:
> 
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23217
> 
> Basically I'm getting database errors when I try to do Batch Patron
> Modification and there is no borrower attribute provided, although there are
> borrower attribute codes on the system so the select menu appears.

The key is using DEBUG=1 in your Apache configuration to turn on RaiseError in DBI.
Comment 10 David Cook 2019-06-26 08:04:29 UTC
Also found this bug: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23218

Depending on user behaviour, it may or may not be an edge case