Description
Frédéric Demians
2020-04-30 05:49:44 UTC
Created attachment 104004 [details] [review] Bug 25325 ElasticSearch mapping export lacks staff_client/opac fields Add to export_elasticsearch_mappings.pl two missing fields: staff_client and opac. Test plan: 1. Run export_elasticsearch_mappings.pl script. Write the result in mappings.yaml (the one defined in koha-conf.xml). 2. In Admin > Search engine configuration, click on Reset Mappings 3. Confirm that in Search fields tab, the Searchable column hasn't changed. Created attachment 104063 [details] [review] Bug 25325 ElasticSearch mapping export lacks staff_client/opac fields Add to export_elasticsearch_mappings.pl two missing fields: staff_client and opac. Test plan: 1. Run export_elasticsearch_mappings.pl script. Write the result in mappings.yaml (the one defined in koha-conf.xml). 2. In Admin > Search engine configuration, click on Reset Mappings 3. Confirm that in Search fields tab, the Searchable column hasn't changed. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Hi, I tried to test and improvised a lot because Koha ES is new for me. First, I switch SearchEngine syspref to ES but I don't know what else to do. (I have ES running as part of koha-testing-docker) > 3. Confirm that in Search fields tab, the Searchable column hasn't > changed. Without the patch, the Searchable column hasn't changed. So couldn't reproduce the issue. I suspect that it's because my test data (from koha-testing-docker) has everything to "yes". And that "yes" might be the default. So in the intranet I change few a columns to "no" and save. > An error occurred when updating Elasticsearch index mappings: > Unable to update mappings for index "koha_kohadev_biblios". > Reason was: "no such index". Index needs to be recreated and reindexed. > An error occurred when updating Elasticsearch index mappings: > Unable to update mappings for index "koha_kohadev_authorities". > Reason was: "no such index". Index needs to be recreated and reindexed. > Index 'koha_kohadev_biblios' needs to be recreated. > Index 'koha_kohadev_authorities' needs to be recreated. Ok that doesn't look good. I must be missing some basic ES config. At least I can confirm that with the patch there is a diff on the output of export_elasticsearch_mappings.pl It's precisely on the changes I made which is encouraging. Let's try anyway. So my test plan completed was: - SearchEngine syspref to ES (don't know if that's useful) - set few fields to not searchable - scary errors... - misc/search_tools/export_elasticsearch_mappings.pl - save to mappings.yaml - for me it's: /kohadevbox/koha/admin/searchengine/elasticsearch/mappings.yaml - restart_all (don't know if that's useful) - reset mappings - changes to the fields are lost - apply patch - restart_all - set few fields to not searchable - scary errors... - misc/search_tools/export_elasticsearch_mappings.pl - save to mappings.yaml - restart_all (don't know if that's useful) - reset mappings - changes to the fields are still there! :D So in spite of the scary errors and my dubious setup. It really look like it's working :D *** Bug 25337 has been marked as a duplicate of this bug. *** Victor, your correct 'yes' is default value for 'staff_client' and 'opac' fields. It means you have to set some ES fields to 'false' to see the problem. You can also see the reverse function of exporting ES config which is: Koha::SearchEngine::ElasticSearch.pm::reset_elasticsearch_mappings http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=Koha/SearchEngine/Elasticsearch.pm;h=66166713259f4a175295384fb30486ee2d8b6431;hb=HEAD#l360 Thks for testing/signing. Salut Frédéric, could you add tests for that? (In reply to Jonathan Druart from comment #6) > Salut Frédéric, could you add tests for that? Yes, sorry. Created attachment 104376 [details] [review] Bug 25325: Test ES lack of staff_client/opac fields Run this test to see that export_elasticsearch_mappings.pl doesn't export staff_client/opac fields. Created attachment 104377 [details] [review] Bug 25325: ElasticSearch mapping export lacks staff_client/opac fields Run test: prove t/db_dependent/Koha/SearchEngine/Elasticsearch/ExportConfig.t Or: Run export_elasticsearch_mappings.pl script. See that opac/staff_client fields are exorted. Should the first patch be manually tested again? Wait, it's all automated tests now. I got also confused that the implementation changed from the one on comment 1 Created attachment 104451 [details] [review] Bug 25325: Test ES lack of staff_client/opac fields Run this test to see that export_elasticsearch_mappings.pl doesn't export staff_client/opac fields. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 104452 [details] [review] Bug 25325: ElasticSearch mapping export lacks staff_client/opac fields Run test: prove t/db_dependent/Koha/SearchEngine/Elasticsearch/ExportConfig.t Or: Run export_elasticsearch_mappings.pl script. See that opac/staff_client fields are exorted. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 104482 [details] [review] Bug 25325: Test ES lack of staff_client/opac fields Run this test to see that export_elasticsearch_mappings.pl doesn't export staff_client/opac fields. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 104483 [details] [review] Bug 25325: ElasticSearch mapping export lacks staff_client/opac fields Run test: prove t/db_dependent/Koha/SearchEngine/Elasticsearch/ExportConfig.t Or: Run export_elasticsearch_mappings.pl script. See that opac/staff_client fields are exorted. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Nice work everyone! Pushed to master for 20.05 not backported to 19.11.x On bug 23204 we made sure that the mappings.yaml file included in the codebase was the same as the output of misc/search_tools/export_elasticsearch_mappings.pl However since this change (pushed only two months later) they differ. Is that a big deal? Should we resync them? I say we should update mapping.yaml, it is a reference when starting to manage Elasticsearch with files. |