Bug 39526

Summary: Unify system preference variable names for Elasticsearch
Product: Koha Reporter: Thomas Klausner <domm>
Component: Searching - ElasticsearchAssignee: Thomas Klausner <domm>
Status: Pushed to main --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, dcook, domm, jonathan.druart, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38964
GIT URL: Change sponsored?: Sponsored
Patch complexity: String patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:
Attachments: Bug 39526: Unify syspref variable names to Elasticsearch
Bug 39526: Unify syspref variable names to Elasticsearch
Bug 39526: Unify syspref variable names to Elasticsearch
Bug 39526: Prevent other prefs to be renamed
Bug 39526: Do not insert the pref if renamed

Description Thomas Klausner 2025-04-02 13:00:38 UTC
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*".
Comment 1 David Nind 2025-04-08 20:12:26 UTC
See thread on chat discussing consistent naming for Elasticsearch related system preferences https://chat.koha-community.org/koha-community/pl/78oauot6qfgkidne7gokc9bkjh
Comment 2 Thomas Klausner 2025-04-23 09:49:22 UTC
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
Comment 3 Roman Dolny 2025-04-23 14:26:47 UTC
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>
Comment 4 Lucas Gass (lukeg) 2025-05-02 17:54:21 UTC
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?
Comment 5 Jonathan Druart 2025-09-16 14:38:42 UTC
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>
Comment 6 Jonathan Druart 2025-09-16 14:38:45 UTC
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>
Comment 7 Jonathan Druart 2025-09-16 14:39:30 UTC
(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 :)
Comment 8 Lucas Gass (lukeg) 2025-09-19 14:44:53 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 9 Jonathan Druart 2025-09-22 12:57:19 UTC
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.
Comment 10 Lucas Gass (lukeg) 2025-09-22 22:27:24 UTC
follow-up pushed to main