Some sysprefs are called "Elasticsearch*" (I can find 4 in the DB), and some are called "ES*" (2 in the DB). Those should probably be unified? select variable from systempreferences where variable like 'ES%' or variable like 'Elastic%' order by 1; +--------------------------------------+ | variable | +--------------------------------------+ | ElasticsearchCrossFields | | ElasticsearchIndexStatus_authorities | | ElasticsearchIndexStatus_biblios | | ElasticsearchMARCFormat | | ESBoostFieldMatch | | ESPreventAutoTruncate | +--------------------------------------+ We have more "Elasticsearch" than "ES", "Elasticsearch" is more self-documenting then "ES", and searching for "ES" in sysprefs finds a lot of false positives, so we should unify to "Elasticsearch*".
See thread on chat discussing consistent naming for Elasticsearch related system preferences https://chat.koha-community.org/koha-community/pl/78oauot6qfgkidne7gokc9bkjh
Created attachment 181356 [details] [review] Bug 39526: Unify syspref variable names to Elasticsearch We currently have a sysprefs named "Elasticsearch*" and some name only "ES*". This patch unifies the names to always start with "Elasticsearch". Test plan: * Start koha-testing-docker, open the staff interface and go to Administration. * Enter "Elasticsearch" into the System Preference search box and click "search" * You will find a bunch of sysprefs * Now search for "ESPreventAutoTruncate", which should find one match * Apply the patch * run (in ktd) the script `updatedatabase` * You should see output like DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_39526_unify_syspref_variable_names_for_elasticsearch.pl [09:35:03]: Bug 39526 - Unify system preference variable names for Elasticsearch Renamed system preference 'ES..' to 'Elasticsearch..' * Go again to to sysprefs and search again for "Elasticsearch" * you should now find "ElasticsearchPreventAutoTruncate" * Search for "ESPreventAutoTruncate", which should show "No system preferences matched your search for .." (and the list of ALL sysprefs) Please note that the qa script explains about installer/data/mysql/mandatory/sysprefs.sql, but the last line is not touched by this patch, so not sure why this happens. Sponsored-by: HKS3
Created attachment 181378 [details] [review] Bug 39526: Unify syspref variable names to Elasticsearch We currently have a sysprefs named "Elasticsearch*" and some name only "ES*". This patch unifies the names to always start with "Elasticsearch". Test plan: * Start koha-testing-docker, open the staff interface and go to Administration. * Enter "Elasticsearch" into the System Preference search box and click "search" * You will find a bunch of sysprefs * Now search for "ESPreventAutoTruncate", which should find one match * Apply the patch * run (in ktd) the script `updatedatabase` * You should see output like DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_39526_unify_syspref_variable_names_for_elasticsearch.pl [09:35:03]: Bug 39526 - Unify system preference variable names for Elasticsearch Renamed system preference 'ES..' to 'Elasticsearch..' * Go again to to sysprefs and search again for "Elasticsearch" * you should now find "ElasticsearchPreventAutoTruncate" * Search for "ESPreventAutoTruncate", which should show "No system preferences matched your search for .." (and the list of ALL sysprefs) Please note that the qa script explains about installer/data/mysql/mandatory/sysprefs.sql, but the last line is not touched by this patch, so not sure why this happens. Sponsored-by: HKS3 Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
If you are only updating 1 system preference why do a REGEXP_REPLACE in the atomicupdate? Would it not be safer to just update the 1 value?
Created attachment 186488 [details] [review] Bug 39526: Unify syspref variable names to Elasticsearch We currently have a sysprefs named "Elasticsearch*" and some name only "ES*". This patch unifies the names to always start with "Elasticsearch". Test plan: * Start koha-testing-docker, open the staff interface and go to Administration. * Enter "Elasticsearch" into the System Preference search box and click "search" * You will find a bunch of sysprefs * Now search for "ESPreventAutoTruncate", which should find one match * Apply the patch * run (in ktd) the script `updatedatabase` * You should see output like DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_39526_unify_syspref_variable_names_for_elasticsearch.pl [09:35:03]: Bug 39526 - Unify system preference variable names for Elasticsearch Renamed system preference 'ES..' to 'Elasticsearch..' * Go again to to sysprefs and search again for "Elasticsearch" * you should now find "ElasticsearchPreventAutoTruncate" * Search for "ESPreventAutoTruncate", which should show "No system preferences matched your search for .." (and the list of ALL sysprefs) Please note that the qa script explains about installer/data/mysql/mandatory/sysprefs.sql, but the last line is not touched by this patch, so not sure why this happens. Sponsored-by: HKS3 Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 186489 [details] [review] Bug 39526: Prevent other prefs to be renamed There is only one pref. It feel safer to rename this specific pref than other pref that might not be ES related. Other prefs starting with "ES" could be added, then this patch backported, etc. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Lucas Gass (lukeg) from comment #4) > If you are only updating 1 system preference why do a REGEXP_REPLACE in the > atomicupdate? Would it not be safer to just update the 1 value? I didn't see you comment! The patch is there now :)
Nice work everyone! Pushed to main for 25.11
Created attachment 186689 [details] [review] Bug 39526: Do not insert the pref if renamed The upgrade DB must be idempotent if run twice, the second pass fails with DBI Exception: DBD::mysql::db do failed: Duplicate entry 'ElasticsearchPreventAutoTruncate' for key 'PRIMARY' at /kohadevbox/koha/C4/Installer.pm line 825 We should not insert the pref if it has been renamed already.
follow-up pushed to main