Bug 9998 - Import/export and compare system preferences
Summary: Import/export and compare system preferences
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Marcel de Rooy
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-08 13:35 UTC by Marcel de Rooy
Modified: 2014-12-07 20:02 UTC (History)
2 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 9998: Import/export and compare system preferences (12.02 KB, patch)
2013-05-08 14:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9998: Import/export and compare system preferences (12.03 KB, patch)
2013-06-14 12:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9998: QA Followup for comparing sysprefs (17.13 KB, patch)
2013-06-17 07:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9998: QA Followup for comparing sysprefs (17.51 KB, patch)
2013-06-17 12:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9998: Import/export and compare system preferences (13.12 KB, patch)
2013-06-17 15:06 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2013-04-08 13:35:22 UTC
Would be useful if we could compare preferences between Koha instances or in time, and optionally restore them to some point.
Will soon submit an admin script for this purpose.
Comment 1 Marcel de Rooy 2013-05-08 14:30:48 UTC
# Two interesting features are:
# 1) It may help you to compare systempreferences between Koha instances.
# 2) You can also quickly restore subsets of preferences while testing.
#    Just leave only e.g. some circulations prefs in a file and compare with
#    the update flag.
Comment 2 Marcel de Rooy 2013-05-08 14:46:33 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-06-14 12:56:04 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-06-14 13:41:12 UTC
QA comment:

Note: I did not test the feature!

1/ It would be good to use perltidy on this new file.
2/ Don't you think it is better to have a pod instead of the "print <<_USAGE_" stuff (and to use Pod2Usage)?
3/ SQL strings should be between simple quotes.
Comment 5 Jonathan Druart 2013-06-14 14:22:38 UTC
Marked as Failed QA.
Comment 6 Marcel de Rooy 2013-06-17 07:28:39 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2013-06-17 07:32:05 UTC
(In reply to comment #4)
> Note: I did not test the feature!
Too bad, but thanks anyway..
 
> 1/ It would be good to use perltidy on this new file.
Fixed that

> 2/ Don't you think it is better to have a pod instead of the "print
> <<_USAGE_" stuff (and to use Pod2Usage)?
Not sure, but moved it to pod.

> 3/ SQL strings should be between simple quotes.
Also not sure about that.
I replaced most of them. But had one case:
    " AND COALESCE(explanation,'')='' AND COALESCE(options,'')=''"
where I could not replace them..

Back to Signed Off
Comment 8 Jonathan Druart 2013-06-17 11:15:40 UTC
QA comment:

Now I tested :)

1/ There is no error if the -file parameter is missing.
2/ Explanations are not restored, why don't you store them in the backup file? (explanations are useful when you search a pref without to know its name).
3/ 
  "17 Run with -cmd compare -file zz02 -del. pref zz02 should not be deleted."
returns "Adjusted (at most) 2 prefs from this compare."
=> but no change done.
I think you should change in HandleCompareChanges:
   ++$t && DeleteOnePref($k) if $compare_del;
with
   $t += DeleteOnePref($k) if $compare_del # But need to check "0E0" for 0

Maybe 2 first patches could be squashed.

Marked as Failed QA.
Comment 9 Marcel de Rooy 2013-06-17 12:46:34 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2013-06-17 12:49:23 UTC
(In reply to comment #8)
> Now I tested :)
Great.

> 1/ There is no error if the -file parameter is missing.
Shows help now.

> 2/ Explanations are not restored, why don't you store them in the backup
> file? (explanations are useful when you search a pref without to know its
> name).
This is intentional. See bug 10199. Depends on solution there. This script got the -ign-opt parameter just for this reason.

> 3/ 
>   "17 Run with -cmd compare -file zz02 -del. pref zz02 should not be
> deleted."
> returns "Adjusted (at most) 2 prefs from this compare."
> => but no change done.
At most is a maximum :) Replaced this lazy programming with your suggestion. Thanks.
Comment 11 Jonathan Druart 2013-06-17 15:04:28 UTC
QA comment:

Just some minor things:
1/ I hate the $_ variable loop... :)
2/ the die would be replaced with pod2usage
3/ pod2usage has a message param (in order to display "No filename given", etc.)
4/ I think that "verbose => 1" would be more appropriate for warning messages.
5/ use Modern::Perl instead of warnings ad strict

I don't know if it is blocker. Let the RM takes the decision.
Marked as Passed QA.
Comment 12 Jonathan Druart 2013-06-17 15:06:34 UTC
Created attachment 19074 [details] [review]
Bug 9998: Import/export and compare system preferences

This patch adds a new maintenance script: cmp_sysprefs.pl

Test plan:
1 Run with -cmd backup -file zz01. Check that file.
  Run with -cmd test -file zz01. Check zz01.sav.
2 Run with -cmd compare -file zz01. No differences expected.
3 Edit zz01: delete two prefs, change two prefs and add two new prefs.
4 Run with -cmd compare -file zz01. Are all six changes reported?
5 Add local use preference zz02 in the staff client (no explanation, options).
6 Run with -cmd backup -file zz02.
7 Delete local use pref zz02 from the staff client.
8 Run with -cmd restore -file zz02. Check if local pref zz02 came back.
9 Delete local use pref zz02 again from the staff client.
10 Run with -cmd compare -file zz02 -add. Check if local pref zz02 came back.
11 Change the value of zz02 in the staff client.
12 Run with -cmd compare -file zz02 -add. Check the value: not updated?
13 Run with -cmd compare -file zz02 -upd. Check the value: updated now?
14 Edit file zz02. Add a comment line and delete the line for pref zz02.
15 Run with -cmd compare -file zz02 -del. Is pref zz02 deleted?
16 Add local use preference zz02 in the staff client (WITH explanation).
17 Run with -cmd compare -file zz02 -del. pref zz02 should not be deleted.
18 Run with -cmd compare -file zz02 -del -ign-opt. zz02 should be deleted now.

Do the next steps only on a restorable test db:
19 Create file zz03. Leave it empty.
   Compare with: -cmd compare -file zz03 -del -ign-opt.
   All prefs gone except Version?
20 Restore with: -cmd restore -file zz01.sav.
   Compare with -cmd compare -file zz01.sav. Nothing reported?
   Note: The explanation or options are not recovered. (See also BZ 10199.)
   This affects local use preferences only.
   If you need them, restore your test db. Remove the zz files.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Amended patch
Only cosmetic stuff:
Perltidied with xt/perltidyrc.
Replaced most double quotes by single quotes around SQL statements.
Moved the usage into POD for pod2usage.
Passing no file shows help screen too.
Counting the db updates more accurately with return value of do.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 13 Jonathan Druart 2013-06-17 15:07:09 UTC
Marcel, I squashed the 2 patches, I hope it is ok for you.
Comment 14 Galen Charlton 2013-07-02 14:11:05 UTC
Pushed to master.  Thanks, Marcel!