Bug 19673 - Patron batch modification tool cannot use authorised value "0"
Summary: Patron batch modification tool cannot use authorised value "0"
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-21 15:57 UTC by Emma Perks
Modified: 2018-05-22 09:28 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19673: Allow to set patron attributes to 0 with batch patron modification (4.65 KB, patch)
2017-12-21 00:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19673: Allow to set patron attributes to 0 with batch patron modification (4.69 KB, patch)
2018-03-23 20:40 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19673: Allow to set patron attributes to 0 with batch patron modification (4.76 KB, patch)
2018-03-28 05:46 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 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