Bug 33316 - Improve display of ES indexer jobs
Summary: Improve display of ES indexer jobs
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 33486
Blocks:
  Show dependency treegraph
 
Reported: 2023-03-22 19:54 UTC by Nick Clemens
Modified: 2023-12-28 20:47 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.10


Attachments
Bug 33316: Improve display (2.17 KB, patch)
2023-03-22 19:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33316: Improve display (2.22 KB, patch)
2023-03-22 22:52 UTC, David Nind
Details | Diff | Splinter Review
Bug 33316: Improve display of ES indexer jobs (2.55 KB, patch)
2023-04-10 16:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33316: Improve display of ES indexer jobs (2.61 KB, patch)
2023-04-10 16:40 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33316: (QA follow-up) Add link to record, fix typo and heading (1.42 KB, patch)
2023-04-10 16:40 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33316: (QA follow-up) Add link to record, fix typo and heading (1.42 KB, patch)
2023-04-10 16:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33316: Improve display of ES indexer jobs (2.68 KB, patch)
2023-04-13 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33316: (QA follow-up) Add link to record, fix typo and heading (1.49 KB, patch)
2023-04-13 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-03-22 19:54:26 UTC
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
Comment 1 Nick Clemens 2023-03-22 19:55:06 UTC
Created attachment 148570 [details] [review]
Bug 33316: Improve display
Comment 2 Nick Clemens 2023-03-22 19:56:14 UTC
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
Comment 3 David Nind 2023-03-22 22:51:54 UTC
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.
Comment 4 David Nind 2023-03-22 22:52:37 UTC
Created attachment 148576 [details] [review]
Bug 33316: Improve display

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Jonathan Druart 2023-03-23 08:20:13 UTC
This should depend on the decision of keeping or removing Koha::BackgroundJob::UpdateElasticIndex->process (see bug 32594 comment 41 & 43).
Comment 6 Katrin Fischer 2023-04-10 16:19:19 UTC
Please make sure commit message has a descriptive subject and includes test plan!
Comment 7 Katrin Fischer 2023-04-10 16:21:05 UTC
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>
Comment 8 Katrin Fischer 2023-04-10 16:26:25 UTC
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
Comment 9 Katrin Fischer 2023-04-10 16:40:34 UTC
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>
Comment 10 Katrin Fischer 2023-04-10 16:40:37 UTC
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>
Comment 11 Katrin Fischer 2023-04-10 16:41:30 UTC
(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.
Comment 12 Katrin Fischer 2023-04-10 16:44:06 UTC
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 13 Jonathan Druart 2023-04-11 07:15:51 UTC
Comment 5 still valid. Nick?
Comment 14 Nick Clemens 2023-04-11 12:53:34 UTC
(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 :-)
Comment 15 Jonathan Druart 2023-04-11 13:14:15 UTC
Sorry, I don't have time for this now.

Back to PQA, but we should not push as it IMO.
Comment 16 Katrin Fischer 2023-04-11 13:47:22 UTC
(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?
Comment 17 Jonathan Druart 2023-04-13 07:18:21 UTC
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>
Comment 18 Jonathan Druart 2023-04-13 07:18:24 UTC
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>
Comment 19 Jonathan Druart 2023-04-13 07:18:58 UTC
(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!
Comment 20 Tomás Cohen Arazi 2023-04-13 14:49:48 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 21 Matt Blenkinsop 2023-04-17 09:12:33 UTC
Enhancement - not backporting to 22.11.x

Nice work everyone!
Comment 22 Matt Blenkinsop 2023-09-08 09:55:01 UTC
Backport requested - simple change, happy to backport.

Resetting bug and backporting to 22.11.x
Comment 23 Matt Blenkinsop 2023-09-08 09:55:19 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x