Bug 21869 - Bad update statement loses values for MarkLostItemsAsReturned
Summary: Bad update statement loses values for MarkLostItemsAsReturned
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 19287
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-20 16:32 UTC by Nick Clemens
Modified: 2020-01-06 20:15 UTC (History)
4 users (show)

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


Attachments
Bug 21869: Fix incorrect value setting in DB update (1.56 KB, patch)
2018-11-26 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21869: Fix incorrect value setting in DB update (1.71 KB, patch)
2018-11-26 13:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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