Bug 21869

Summary: Bad update statement loses values for MarkLostItemsAsReturned
Product: Koha Reporter: Nick Clemens <nick>
Component: Architecture, internals, and plumbingAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: fridolin.somers, jdemuth, jesse, m.de.rooy
Version: Main   
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: 19287    
Bug Blocks:    
Attachments: Bug 21869: Fix incorrect value setting in DB update
Bug 21869: Fix incorrect value setting in DB update

Description Nick Clemens 2018-11-20 16:32:58 UTC
bug 19287 added a new option to MarkLostItemsAsReturned

it accidentally set an incorrect value for the preference
Comment 1 Nick Clemens 2018-11-26 13:11:59 UTC
Created attachment 82640 [details] [review]
Bug 21869: Fix incorrect value setting in DB update

To test:
1 - Set some value for MarkLostAsReturned syspref
2 - Set your db version to 17.12.00.035 (or load an older db)
3 - updatedatabase
4 - Note your values are lost and db is incorrectly set
5 - Apply patch
6 - Reset values
7 - Reset DB version
8 - Run updatedatabase
9 - Original values are retained
Comment 2 Nick Clemens 2018-11-26 13:12:51 UTC
We can't restore any lost values, but this should allow us to prevent any further damage during upgrades
Comment 3 Marcel de Rooy 2018-11-26 13:24:21 UTC
For me now
Comment 4 Marcel de Rooy 2018-11-26 13:31:55 UTC
What about the dbrev .35 in front:

            UPDATE systempreferences
            SET type="multiple",
                options="batchmod|moredetail|cronjob|additem",
                value="batchmod,moredetail,cronjob,additem"
            WHERE variable="MarkLostItemsAsReturned"

Type is multiple so more values are allowed.
But options are separated with | and values are separated with , comma

Needs more attention
Comment 5 Marcel de Rooy 2018-11-26 13:58:27 UTC
IRC

 <kidclamp> marcelr: yeah, the original db update did that, this was a followup adding a new option to the list
[14:45] <kidclamp> so the values shouldn't be changed
Comment 6 Marcel de Rooy 2018-11-26 13:59:23 UTC
Created attachment 82647 [details] [review]
Bug 21869: Fix incorrect value setting in DB update

To test:
1 - Set some value for MarkLostAsReturned syspref
2 - Set your db version to 17.12.00.035 (or load an older db)
3 - updatedatabase
4 - Note your values are lost and db is incorrectly set
5 - Apply patch
6 - Reset values
7 - Reset DB version
8 - Run updatedatabase
9 - Original values are retained

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2018-11-26 14:00:19 UTC
oops two SOs: one for signoff, one for qa :)
Comment 8 Nick Clemens 2018-11-26 14:31:13 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Jesse Maseto 2018-12-13 20:50:27 UTC
Pushed to stable for 18.05.07
Comment 10 Fridolin Somers 2018-12-19 12:58:27 UTC
Depends on Bug 19287 not in 17.11.x