It would be nice for the end user (or developer) to have a way to view the record as it exists in the ES index - it will help in adjusting mappings, understanding results, and identifying is a record is not being index
Created attachment 64456 [details] [review] Bug 18829 - Elasticsearch - Add ability to view the ES indexed record This is groundwork - we need to make the display prettier, but for now it does at least dump the record into the template We probably also need a swtich to enable/disable this - or just move it into the left hand tabs?
Created attachment 89001 [details] [review] Bug 18829: Elasticsearch - Add ability to view the ES indexed record This is groundwork - we need to make the display prettier, but for now it does at least dump the record into the template We probably also need a swtich to enable/disable this - or just move it into the left hand tabs?
Created attachment 89007 [details] [review] Bug 18829: Elasticsearch - Add ability to view the ES indexed record This is groundwork - we need to make the display prettier, but for now it does at least dump the record into the template We probably also need a swtich to enable/disable this - or just move it into the left hand tabs?
Created attachment 89248 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElsaticSearch" as value - Search whatever you want in Search the catalog and select a notice - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview)
Created attachment 89249 [details] [review] Bug 18829: Elasticsearch - Add ability to view the ES indexed record This is groundwork - we need to make the display prettier, but for now it does at least dump the record into the template We probably also need a swtich to enable/disable this - or just move it into the left hand tabs?
Created attachment 89250 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElsaticSearch" as value - Search whatever you want in Search the catalog and select a notice - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview)
Oups the ES connexion is hardcoded : + $.getJSON("http://localhost:9200/koha_kohadev_biblios/data/", biblionumber, function( data ){ You must get server+port and index name from C4::Context.
Created attachment 90370 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview)
Squashed as axel rewrote my patch :-) Some issues: 1 - Missing filter from QA tool - this is tricky too - if I add </script> to the title I can break the view - we need to make sure the JOSN can't break 2 - We should order the view, at least for _source - this could be later enhancement, but would make it much easier to read
Created attachment 108103 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record
Created attachment 108104 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record
Nice work, I like it.
Created attachment 108191 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 108944 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 108945 [details] [review] Bug 18829: (QA follow-up) Fix punctuation, capitalization and page title Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I think this is a nice little enhancement as is. I'd love to see codemirror JSON highlighting used on the modal in the future.
1. +catch{ + @es_fields = ("Error fetching record: see logs for details"); + warn $_; +}; This is not the way to deal with error, you should return an error, here certainly a 404. 2. Incorrect unicode characters handling. Add '❤' to the title then click the link, you get a JS error: "URIError: malformed URI sequence" and the modal is empty.
Created attachment 148757 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 148758 [details] [review] Bug 18829: (QA follow-up) Fix punctuation, capitalization and page title Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 148759 [details] [review] Bug 18829: (follow-up) Return 404 when record cannot be loaded
Created attachment 148760 [details] [review] Bug 18829: (follow-up) Don't messup record encoding
Created attachment 149400 [details] [review] Bug 18829: Elasticsearch - Put a better display for the view of ES indexed record To test: - apply the patch - go to global sysPref - make sure that SearchEngine have "ElasticSearch" as value - Search whatever you want in Search the catalog and select a record - Search the link (Ctrl f) : "Elasticsearch Record :" , click on the link - make sure that the pop-up open and you should see the elasticsearch result in JSON (the pop-up look the same as MARC preview) - note fields are alphabetized - from the command line delete the es record: curl -XDELETE es:9200/koha_kohadev_biblios/data/5 - click the preview link, it syas record not found - check the logs - you see the error on missing record Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 149401 [details] [review] Bug 18829: (QA follow-up) Fix punctuation, capitalization and page title Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 149402 [details] [review] Bug 18829: (follow-up) Return 404 when record cannot be loaded
Created attachment 149403 [details] [review] Bug 18829: (follow-up) Don't messup record encoding Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 149404 [details] [review] Bug 18829: (QA follow-up) I18N, test character and breadcrumbs * Removes a 'heart' character from the file, probably added for testing * Corrects breadbrumbs to make QA test tool happy * Makes JS message translatable (test by manipulating the link to an invalid record number in DOM) Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 23.05. Nice work everyone, thanks!
It looks like this broke one test on master: https://jenkins.koha-community.org/job/Koha_Master/2479/testReport/junit/(root)/t_00_testcritic_t/Test__Perl__Critic_for__catalogue_showelastic_pl___1972_/
Created attachment 149745 [details] [review] Bug 18829: (QA follow-up) Replace forbidden indirect syntax This pattern is forbidden and was overlooked by QA. To test: 1. Run: $ ktd --shell k$ prove t/00-testcritic.t => FAIL: Tests fail with 'Subroutine "new" called using indirect syntax...' 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Enhancement - not backporting to 22.11.x Nice work everyone!