# 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.

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.

Marked as Failed QA.

(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

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.

(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.

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.

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>

Marcel, I squashed the 2 patches, I hope it is ok for you.

Pushed to master.  Thanks, Marcel!