|
Description
Martin Renvoize (ashimema)
2019-05-02 06:58:36 UTC
Created attachment 116722 [details] [review] Bug 22824: Add tests Writing some tests to see how things are working currently Created attachment 116723 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS in tests In order to understand how things will work after the changes Created attachment 116724 [details] [review] Bug 22824: Fixing first error in tests There was an encoding issue. The parent bug report have been fixed, see bug 27673 and patches related to Encode Created attachment 116725 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS Created attachment 116726 [details] [review] Bug 22824: Fix searching.pref One .pref file make YAML::XS fails. The correct display must be, before and after this patchset: If "Unescape escaped" is selected this will allow writing regular expressions "\/like this\/" while "/this/", "or/this" will be escaped and interpreted as regular strings. Created attachment 116727 [details] [review] Bug 22824: Correct on/off behaviour Here things are starting to be a bit tricky. IMO we don't want to replicate the ImplicitTyping behaviour from YAML::Syck but handle correctly the different value. We have the list of available values in the .pref file, the values in DB must match them. We want 1 or 0 in DB. 2 more occurrences: finesMode => Dealt on bug 27676 ('off' is ok here) itemBarcodeInputFilter => 'OFF' is correct here Note that we don't use ~ or true/false in *.pref Created attachment 116728 [details] [review] Bug 22824: Fix yes/no vs 1/0 Patch generated with: perl -p -i -e 's# no: # 0: #' **/*.pref perl -p -i -e 's# yes: # 1: #' **/*.pref no must be 0 and yes must be 1! Created attachment 116729 [details] [review] Bug 22824: Replace wrong auto replacement This must be "no", it has been caught by the previous sed Created attachment 116730 [details] [review] Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc. It seems that we don't really need all this overhead. YesNo must be a boolean and contain 1 or 0. Created attachment 116731 [details] [review] Bug 22824: Fix default values for YesNo select variable, value from systempreferences where type="YesNo" and ( ( value <>'1' and value <>'0' ) or value is null ); Created attachment 116732 [details] [review] Bug 22824: Atomic update Created attachment 116733 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Created attachment 116734 [details] [review] Bug 22824: Remove YAML::Syck dependency Created attachment 116735 [details] [review] Bug 22824: Adjust tests to match final behaviours Finally we diverge quite a lot with how it worked prior to this patch set. Reflect the changes in the tests Created attachment 116736 [details] [review] Bug 22824: Fix ES reindex koha-elastic --rebuild was failing with Unable to update mappings for index "koha_kohadev_biblios". Reason was: "Could not convert [marc_data.index] to boolean" I am sorry, I lost myself! This was supposed to be a quite trivial patch and it ended up huge! I think the move is interesting and going into the right direction. But I have the feeling that it's not finished and some tricky checks need to be done on the syspref table/.pref. This patch set is dealing with the yes/no, on/off weird options we have, historically I bet, in our table and .pref files. In this patch we are removing all of that and expect a boolean to be 1 or 0 (wow!), and it removes the C4::Boolean weirdness. So far the prefs, installer and translate scripts seem to be working correctly. The edge cases may be hard to catch! Don't be afraid by the number of patches, they are all very trivial! What do you think? Created attachment 116741 [details] [review] Bug 22824: Add tests Writing some tests to see how things are working currently Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116742 [details] [review] Bug 22824: Add tests Writing some tests to see how things are working currently Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116743 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS in tests In order to understand how things will work after the changes Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116744 [details] [review] Bug 22824: Fixing first error in tests There was an encoding issue. The parent bug report have been fixed, see bug 27673 and patches related to Encode Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116745 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116746 [details] [review] Bug 22824: Fix searching.pref One .pref file make YAML::XS fails. The correct display must be, before and after this patchset: If "Unescape escaped" is selected this will allow writing regular expressions "\/like this\/" while "/this/", "or/this" will be escaped and interpreted as regular strings. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116747 [details] [review] Bug 22824: Correct on/off behaviour Here things are starting to be a bit tricky. IMO we don't want to replicate the ImplicitTyping behaviour from YAML::Syck but handle correctly the different value. We have the list of available values in the .pref file, the values in DB must match them. We want 1 or 0 in DB. 2 more occurrences: finesMode => Dealt on bug 27676 ('off' is ok here) itemBarcodeInputFilter => 'OFF' is correct here Note that we don't use ~ or true/false in *.pref Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116748 [details] [review] Bug 22824: Fix yes/no vs 1/0 Patch generated with: perl -p -i -e 's# no: # 0: #' **/*.pref perl -p -i -e 's# yes: # 1: #' **/*.pref no must be 0 and yes must be 1! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116749 [details] [review] Bug 22824: Replace wrong auto replacement This must be "no", it has been caught by the previous sed Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116750 [details] [review] Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc. It seems that we don't really need all this overhead. YesNo must be a boolean and contain 1 or 0. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116751 [details] [review] Bug 22824: Fix default values for YesNo select variable, value from systempreferences where type="YesNo" and ( ( value <>'1' and value <>'0' ) or value is null ); Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116752 [details] [review] Bug 22824: Atomic update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116753 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116754 [details] [review] Bug 22824: Remove YAML::Syck dependency Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116755 [details] [review] Bug 22824: Adjust tests to match final behaviours Finally we diverge quite a lot with how it worked prior to this patch set. Reflect the changes in the tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 116756 [details] [review] Bug 22824: Fix ES reindex koha-elastic --rebuild was failing with Unable to update mappings for index "koha_kohadev_biblios". Reason was: "Could not convert [marc_data.index] to boolean" Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to Jonathan Druart from comment #16) > What do you think? Looks great to me! We've plenty of time before release in case there are any more edge cases that need catching, but I can't think of any! (In reply to Kyle M Hall from comment #33) > (In reply to Jonathan Druart from comment #16) > > What do you think? > > Looks great to me! We've plenty of time before release in case there are any > more edge cases that need catching, but I can't think of any! Indeed.. I've been down this path once before with another app.. I think this looks like a really solid start and covers all the cases that caused me issues in my last experience. I'll happily kick the wheels with what we've got with a view to QAing; I'm confident I won't find every issue, but feel we're still early enough in the cycle to push if we push soon and sort out any fallout... I think devs' should quickly spot potential issues once it's in master. misc/admin/koha-preferences -$YAML::Syck::SortKeys = 1; => This line needs attention. There are some issues in misc/admin/koha-preferences:
- It now allows to set any value for boolean (YesNo) preferences. In master there is a warning message and value is set to 0 when an invalid value is given
- documentation probably needs an update ("If the syspref is YesNo, will accept only a boolean value, but the syntax for these is fairly lax (yes/no, on/off, 1/0, n/y, true/false are all accepted).")
- 'dump' subcommand does not work, it just prints 'HASH(0x12345)'.
Created attachment 116881 [details] [review] Bug 22824: Adjust POD for koha-preferences Created attachment 116882 [details] [review] Bug 22824: Fix koha-preferences dump (In reply to Julian Maurice from comment #36) > There are some issues in misc/admin/koha-preferences: > - It now allows to set any value for boolean (YesNo) preferences. In master > there is a warning message and value is set to 0 when an invalid value is > given I think we are good with the warning I added on the about page, don't you think? I don't really want to reintroduce the C4::Boolean module (or equivalent code) only for this script. > - documentation probably needs an update ("If the syspref is YesNo, will > accept only a boolean value, but the syntax for these is fairly lax (yes/no, > on/off, 1/0, n/y, true/false are all accepted).") Adjusted. > - 'dump' subcommand does not work, it just prints 'HASH(0x12345)'. Oops, fixed! (In reply to Jonathan Druart from comment #39) > (In reply to Julian Maurice from comment #36) > > There are some issues in misc/admin/koha-preferences: > > - It now allows to set any value for boolean (YesNo) preferences. In master > > there is a warning message and value is set to 0 when an invalid value is > > given > > I think we are good with the warning I added on the about page, don't you > think? > I don't really want to reintroduce the C4::Boolean module (or equivalent > code) only for this script. I think the script should prevent to set wrong value for those sysprefs. It could at least check that the value given is either "0" or "1". Created attachment 116883 [details] [review] Bug 22824: Prevent YesNo to be filled with invalid values This patch prevents koha-preferences to set other values than 1/0 to be used for YesNo prefs (In reply to Jonathan Druart from comment #41) > Created attachment 116883 [details] [review] [review] > Bug 22824: Prevent YesNo to be filled with invalid values > > This patch prevents koha-preferences to set other values than 1/0 to be > used for YesNo prefs This patch does not work. If I execute misc/admin/koha-preferences set viewMARC 2 it warns "System preference viewMARC is YesNo and expects 1 or 0. '2' was given, using '0' (OK) but the value is set to '2'. (NOT OK) And the dump subcommand output is different from master. On master it prints a serie of <sysprefName>: <sysprefValue> but with the patches it prints <sysprefName>: type: <sysprefType> value: <sysprefValue> variable: <sysprefName> which prevents to do things like: koha-preferences dump | koha-preferences load Created attachment 116884 [details] [review] Bug 22824: Prevent YesNo to be filled with invalid values This patch prevents koha-preferences to set other values than 1/0 to be used for YesNo prefs (In reply to Julian Maurice from comment #42) > (In reply to Jonathan Druart from comment #41) > > Created attachment 116883 [details] [review] [review] [review] > > Bug 22824: Prevent YesNo to be filled with invalid values > > > > This patch prevents koha-preferences to set other values than 1/0 to be > > used for YesNo prefs > > This patch does not work. > If I execute > misc/admin/koha-preferences set viewMARC 2 > it warns "System preference viewMARC is YesNo and expects 1 or 0. '2' was > given, using '0' (OK) > but the value is set to '2'. (NOT OK) Right, I tested it very poorly, sorry about that. Should work now. > And the dump subcommand output is different from master. On master it prints > a serie of > <sysprefName>: <sysprefValue> > but with the patches it prints > <sysprefName>: > type: <sysprefType> > value: <sysprefValue> > variable: <sysprefName> > > which prevents to do things like: > > koha-preferences dump | koha-preferences load The dump|load actually worked, but not using a dump generated from master. I restored the current behaviour. Sorry, but I think there is still a problem with dump/load
If I do
koha-preferences dump | koha-preferences load
I get an error
YAML::XS::Load Error: The problem:
invalid trailing UTF-8 octet
was found at document: 1
If I do
koha-preferences dump > prefs && file prefs
I get
prefs: ISO-8859 text, with very long lines
but If I do
koha-preferences dump -o prefs && file prefs
I get
prefs: UTF-8 Unicode text, with very long lines
If I remove the call to decode_utf8 in dump subcommand, it works (ie. I get UTF-8 Unicode in both cases, and dump|load seems to work)
Hum $ perl misc/admin/koha-preferences dump | perl misc/admin/koha-preferences load It works $ perl misc/admin/koha-preferences dump > prefs && file prefs prefs: ASCII text indeed, but edit URLLinkText, set "✔ ❤️ ★" $ perl misc/admin/koha-preferences dump > prefs && file prefs prefs: UTF-8 Unicode text $ perl misc/admin/koha-preferences dump | perl misc/admin/koha-preferences load Still works Edit URLLinkText, it contains the characters in the correct encoding. Can you try adding binmode STDOUT, ':encoding(UTF-8)'; before Dump? Switching back to NSO hoping to get other feedbacks. If I set "✔ ❤️ ★" to a syspref, `dump > prefs` give me a "wide-char-in-print" warning. I think I found a fix and will attach the patch. Created attachment 117037 [details] [review] Bug 22824: Fix encoding of misc/admin/koha-preferences output Created attachment 117039 [details] [review] Bug 22824: Fix encoding of misc/admin/koha-preferences output Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 117040 [details] [review] Bug 22824: Add tests Writing some tests to see how things are working currently Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117041 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS in tests In order to understand how things will work after the changes Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117042 [details] [review] Bug 22824: Fixing first error in tests There was an encoding issue. The parent bug report have been fixed, see bug 27673 and patches related to Encode Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117043 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117044 [details] [review] Bug 22824: Fix searching.pref One .pref file make YAML::XS fails. The correct display must be, before and after this patchset: If "Unescape escaped" is selected this will allow writing regular expressions "\/like this\/" while "/this/", "or/this" will be escaped and interpreted as regular strings. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117045 [details] [review] Bug 22824: Correct on/off behaviour Here things are starting to be a bit tricky. IMO we don't want to replicate the ImplicitTyping behaviour from YAML::Syck but handle correctly the different value. We have the list of available values in the .pref file, the values in DB must match them. We want 1 or 0 in DB. 2 more occurrences: finesMode => Dealt on bug 27676 ('off' is ok here) itemBarcodeInputFilter => 'OFF' is correct here Note that we don't use ~ or true/false in *.pref Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117046 [details] [review] Bug 22824: Fix yes/no vs 1/0 Patch generated with: perl -p -i -e 's# no: # 0: #' **/*.pref perl -p -i -e 's# yes: # 1: #' **/*.pref no must be 0 and yes must be 1! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117047 [details] [review] Bug 22824: Replace wrong auto replacement This must be "no", it has been caught by the previous sed Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117048 [details] [review] Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc. It seems that we don't really need all this overhead. YesNo must be a boolean and contain 1 or 0. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117049 [details] [review] Bug 22824: Fix default values for YesNo select variable, value from systempreferences where type="YesNo" and ( ( value <>'1' and value <>'0' ) or value is null ); Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117050 [details] [review] Bug 22824: Atomic update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117051 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117052 [details] [review] Bug 22824: Remove YAML::Syck dependency Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117053 [details] [review] Bug 22824: Adjust tests to match final behaviours Finally we diverge quite a lot with how it worked prior to this patch set. Reflect the changes in the tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117054 [details] [review] Bug 22824: Fix ES reindex koha-elastic --rebuild was failing with Unable to update mappings for index "koha_kohadev_biblios". Reason was: "Could not convert [marc_data.index] to boolean" Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117055 [details] [review] Bug 22824: Adjust POD for koha-preferences Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117056 [details] [review] Bug 22824: Fix koha-preferences dump Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117057 [details] [review] Bug 22824: Prevent YesNo to be filled with invalid values This patch prevents koha-preferences to set other values than 1/0 to be used for YesNo prefs Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117058 [details] [review] Bug 22824: Fix encoding of misc/admin/koha-preferences output Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 117636 [details] [review] Bug 22824: Add tests Writing some tests to see how things are working currently Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117637 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS in tests In order to understand how things will work after the changes Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117638 [details] [review] Bug 22824: Fixing first error in tests There was an encoding issue. The parent bug report have been fixed, see bug 27673 and patches related to Encode Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117639 [details] [review] Bug 22824: Replace YAML::Syck with YAML::XS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117640 [details] [review] Bug 22824: Fix searching.pref One .pref file make YAML::XS fails. The correct display must be, before and after this patchset: If "Unescape escaped" is selected this will allow writing regular expressions "\/like this\/" while "/this/", "or/this" will be escaped and interpreted as regular strings. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117641 [details] [review] Bug 22824: Correct on/off behaviour Here things are starting to be a bit tricky. IMO we don't want to replicate the ImplicitTyping behaviour from YAML::Syck but handle correctly the different value. We have the list of available values in the .pref file, the values in DB must match them. We want 1 or 0 in DB. 2 more occurrences: finesMode => Dealt on bug 27676 ('off' is ok here) itemBarcodeInputFilter => 'OFF' is correct here Note that we don't use ~ or true/false in *.pref Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117642 [details] [review] Bug 22824: Fix yes/no vs 1/0 Patch generated with: perl -p -i -e 's# no: # 0: #' **/*.pref perl -p -i -e 's# yes: # 1: #' **/*.pref no must be 0 and yes must be 1! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117643 [details] [review] Bug 22824: Replace wrong auto replacement This must be "no", it has been caught by the previous sed Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117644 [details] [review] Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc. It seems that we don't really need all this overhead. YesNo must be a boolean and contain 1 or 0. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117645 [details] [review] Bug 22824: Fix default values for YesNo select variable, value from systempreferences where type="YesNo" and ( ( value <>'1' and value <>'0' ) or value is null ); Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117646 [details] [review] Bug 22824: Atomic update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117647 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117648 [details] [review] Bug 22824: Adjust tests to match final behaviours Finally we diverge quite a lot with how it worked prior to this patch set. Reflect the changes in the tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117649 [details] [review] Bug 22824: Fix ES reindex koha-elastic --rebuild was failing with Unable to update mappings for index "koha_kohadev_biblios". Reason was: "Could not convert [marc_data.index] to boolean" Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117650 [details] [review] Bug 22824: Adjust POD for koha-preferences Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117651 [details] [review] Bug 22824: Fix koha-preferences dump Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117652 [details] [review] Bug 22824: Prevent YesNo to be filled with invalid values This patch prevents koha-preferences to set other values than 1/0 to be used for YesNo prefs Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117653 [details] [review] Bug 22824: Fix encoding of misc/admin/koha-preferences output Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Manual testing and I can't find any issues.. this is a great patchset and covers all the usual cases I've come across when making similar moves elsewhere.
Couple of QA script failures.. the first one has me confused:
FAIL koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
FAIL valid_template
parse error - /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt line 210: unexpected token ( warnNoActiveCurrency)
[% IF warnPrefBiblioAddsAuthorities || warnPrefEasyAnalyticalRecords || warnPrefAnonymousPatronOPACPrivacy || warnPrefAnonymousPatronAnonSuggestions || warnPrefAnonymousPatronOPACPrivacy_PatronDoesNotExist || warnPrefAnonymousPatronAnonSuggestions_PatronDoesNotExist || warnPrefKohaAdminEmailAddress || invalid_yesn
o.count || warnNoActiveCurrency || warnIsRootUser || xml_config_warnings.size || AutoSelfCheckPatronDoesNotHaveSelfCheckPerm || AutoSelfCheckPatronHasTooManyPerm || warnStatisticsFieldsError || warnNoTemplateCaching || warnILLConfiguration || has_ai_issues || oauth2_missing_deps || bad_yaml_prefs || warnRelationship
s || log4perl_errors || config_bcrypt_settings_no_set || warnHiddenBiblionumbers.size || warnConnectBroker %]
The second one is a false positive and can be ignored:
FAIL t/db_dependent/yaml.t
FAIL spelling
dont ==> don't
Also.. I dropped a patch during rebase (the removal of YAML::Syck from cpanfile as it's already done in a followup to bug 27673) (In reply to Martin Renvoize from comment #88) > FAIL koha-tmpl/intranet-tmpl/prog/en/modules/about.tt > FAIL valid_template I saw this one too. There is a space that is not really a space, probably a non-breaking space. I don't why know it breaks the tests whereas the template works fine otherwise. Created attachment 117664 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> JD amended patch: there was a "bad space" between "warnNoActiveCurrency" (In reply to Jonathan Druart from comment #91) > Created attachment 117664 [details] [review] [review] > Bug 22824: Add warning to the about page > > Just in case, add a warning to the about page. > > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> > Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> > Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > > JD amended patch: there was a "bad space" between "warnNoActiveCurrency" ^between^before^ Created attachment 117666 [details] [review] Bug 22824: Add warning to the about page Just in case, add a warning to the about page. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> JD amended patch: there was a "bad space" between "warnNoActiveCurrency" Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Excellent, that final patch squashed the warn in the QA script. Going for PQA here. It wouldn't surprise me if there's something we've missed somewhere, but I don't think we're going to spot it here and now and we're better off pushing now and fixing an issue down the line during this cycle when/if we spot it.. with all the devs running this branch I'm sure it'll "come out in the wash if there is anything lurking" Great work Jonathan, Koha's really moving forward at the moment, modernising and cleaning code.. great to see! Pushed to master for 21.05, thanks to everybody involved! Hi all, When I went to master in my devbox, I got this error when restarting services and my sites wouldn't load: BEGIN failed--compilation aborted at /etc/koha/sites/kohadev/plack.psgi line 29. Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't locate C4/Boolean.pm in @INC (you may need to install the C4::Boolean module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/kohaclone/installer /home/vagrant/kohaclone/lib/installer /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at /etc/koha/sites/kohadev/plack.psgi line 29. I removed the use line in /etc/koha/sites/kohadev/plack.psgi, but is there something else I should have done? And will this happen for other Kohas? Cheers, Hayley (In reply to Hayley Pelham from comment #96) > Hi all, > > When I went to master in my devbox, I got this error when restarting > services and my sites wouldn't load: > > BEGIN failed--compilation aborted at /etc/koha/sites/kohadev/plack.psgi line > 29. > Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't locate > C4/Boolean.pm in @INC (you may need to install the C4::Boolean module) (@INC > contains: /home/vagrant/kohaclone /home/vagrant/kohaclone/installer > /home/vagrant/kohaclone/lib/installer /etc/perl > /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 > /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 > /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 > /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at > /etc/koha/sites/kohadev/plack.psgi line 29. > > I removed the use line in /etc/koha/sites/kohadev/plack.psgi, but is there > something else I should have done? And will this happen for other Kohas? > > Cheers, > Hayley reset_all should do the job of replacing the old plack.psgi. (In reply to Tomás Cohen Arazi from comment #97) > (In reply to Hayley Pelham from comment #96) > > Hi all, > > > > When I went to master in my devbox, I got this error when restarting > > services and my sites wouldn't load: > > > > BEGIN failed--compilation aborted at /etc/koha/sites/kohadev/plack.psgi line > > 29. > > Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't locate > > C4/Boolean.pm in @INC (you may need to install the C4::Boolean module) (@INC > > contains: /home/vagrant/kohaclone /home/vagrant/kohaclone/installer > > /home/vagrant/kohaclone/lib/installer /etc/perl > > /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 > > /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 > > /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 > > /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at > > /etc/koha/sites/kohadev/plack.psgi line 29. > > > > I removed the use line in /etc/koha/sites/kohadev/plack.psgi, but is there > > something else I should have done? And will this happen for other Kohas? > > > > Cheers, > > Hayley > > reset_all should do the job of replacing the old plack.psgi. Ah, of course! Gracias :) Enhancement not pushed to 20.11.x @@ -78,14 +78,14 @@ Web services:
choices:
persistent: will never be emptied or truncated (persistent)
transient: might be emptied or truncated at some point (transient)
- no: will never have any data in it (no)
+ 0: will never have any data in it (no)
Isn't this replacement wrong?
|