We don't do a lot of processing to the data, but we can still make the display a bit nicer, to show how many records were done, and show the record ids
Created attachment 148570 [details] [review] Bug 33316: Improve display
This could certainly be improved, but is better than it was To test: 1 - Do some batch modifications on a system running ES 2 - Check background jobs to see ES jobs 3 - Note display 4 - Apply patch 5 - Confirm display is improved
Testing notes: 1. I used 'Batch item modification' to add a public note to two items. 2. Repeated step 1, but added a non-public note for the same items. 3. List of jobs shows 4 jobs - 2 for batch item modifications, and 2 for Elasticsearch index updates. 4. When viewing the details for an Elastic search index update, it now lists the record ids updated in the detailed messages.
Created attachment 148576 [details] [review] Bug 33316: Improve display Signed-off-by: David Nind <david@davidnind.com>
This should depend on the decision of keeping or removing Koha::BackgroundJob::UpdateElasticIndex->process (see bug 32594 comment 41 & 43).
Please make sure commit message has a descriptive subject and includes test plan!
Created attachment 149394 [details] [review] Bug 33316: Improve display of ES indexer jobs We don't do a lot of processing to the data, but we can still make the display a bit nicer, to show how many records were done, and show the record ids. To test: 1 - Do some batch modifications on a system running ES 2 - Check background jobs to see ES jobs 3 - Note display 4 - Apply patch 5 - Confirm display is improved Signed-off-by: David Nind <david@davidnind.com>
I've fixed the test, but I have a problem with the indexing job - it doesn't get done, so I can't verify the display. I remain stuck with: 3 New null/1 Update Elasticsearch index 04/10/2023 16:25
Created attachment 149395 [details] [review] Bug 33316: Improve display of ES indexer jobs We don't do a lot of processing to the data, but we can still make the display a bit nicer, to show how many records were done, and show the record ids. To test: 1 - Do some batch modifications on a system running ES 2 - Check background jobs to see ES jobs 3 - Note display 4 - Apply patch 5 - Confirm display is improved Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 149396 [details] [review] Bug 33316: (QA follow-up) Add link to record, fix typo and heading * h2 h2 directly following each other make no sense. Changed to h2 h3 * ids changed to IDs * Linked the record ID to the catalog entry Note: We also have this link in the batch record mod job details Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Katrin Fischer from comment #8) > I've fixed the test, but I have a problem with the indexing job - it doesn't > get done, so I can't verify the display. I remain stuck with: > > 3 New null/1 Update Elasticsearch index 04/10/2023 16:25 Turns out, things work better when Elastisearch is actually running... (In reply to Jonathan Druart from comment #5) > This should depend on the decision of keeping or removing > Koha::BackgroundJob::UpdateElasticIndex->process (see bug 32594 comment 41 & > 43). I am not sure about this one. Bug 32594 is now pushed to master, the job is still there. I think this won't hurt.
Created attachment 149397 [details] [review] Bug 33316: (QA follow-up) Add link to record, fix typo and heading * h2 h2 directly following each other make no sense. Changed to h2 h3 * ids changed to IDs * Linked the record ID to the catalog entry Note: We also have this link in the batch record mod job details Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 still valid. Nick?
(In reply to Jonathan Druart from comment #13) > Comment 5 still valid. Nick? I am happy either way - if you want to remove 'process', please file a bug :-)
Sorry, I don't have time for this now. Back to PQA, but we should not push as it IMO.
(In reply to Jonathan Druart from comment #15) > Sorry, I don't have time for this now. > > Back to PQA, but we should not push as it IMO. This is only a display change - can you explain your worries?
Created attachment 149585 [details] [review] Bug 33316: Improve display of ES indexer jobs We don't do a lot of processing to the data, but we can still make the display a bit nicer, to show how many records were done, and show the record ids. To test: 1 - Do some batch modifications on a system running ES 2 - Check background jobs to see ES jobs 3 - Note display 4 - Apply patch 5 - Confirm display is improved Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 149586 [details] [review] Bug 33316: (QA follow-up) Add link to record, fix typo and heading * h2 h2 directly following each other make no sense. Changed to h2 h3 * ids changed to IDs * Linked the record ID to the catalog entry Note: We also have this link in the batch record mod job details Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Katrin Fischer from comment #16) > (In reply to Jonathan Druart from comment #15) > > Sorry, I don't have time for this now. > > > > Back to PQA, but we should not push as it IMO. > > This is only a display change - can you explain your worries? Not better than on bug 32594 comment 41. This is all good now. Thanks, Nick!
Pushed to master for 23.05. Nice work everyone, thanks!
Enhancement - not backporting to 22.11.x Nice work everyone!
Backport requested - simple change, happy to backport. Resetting bug and backporting to 22.11.x
Nice work everyone! Pushed to oldstable for 22.11.x