Bug 31245

Summary: Job detail view for batch mod explode if job not started
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: andrewfh, dcook, kyle, lucas, magnus, martin.renvoize, nayema, tomascohen, victor
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31223
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31290
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32305
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.06,21.11.12
Bug Depends on:    
Bug Blocks: 31336    
Attachments: Bug 31245: Do not try to report results if task not started
[ALTERNATIVE-PATCH] Bug 31245: Do not call report for job detail view if job not started
Bug 31245: Do not try to report results if task not started
Bug 31245: Do not call report for job detail view if job not started
Bug 31245: Do not call report for job detail view if job not started
Bug 31245: Do not try to report results if task not started

Description Jonathan Druart 2022-07-27 08:33:22 UTC
If the job has failed to start (status=new), the job detail view is exploding with 

Can't use an undefined value as an ARRAY reference at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateItem.pm line 175                                                                                                
        Koha::BackgroundJob::BatchUpdateItem::additional_report('Koha::BackgroundJob::BatchUpdateItem=HASH(0x5592c6c6ced8)') called at /kohadevbox/koha/Koha/BackgroundJob.pm line 339                                       Koha::BackgroundJob::additional_report('Koha::BackgroundJob=HASH(0x5592c6df3268)') called at /kohadevbox/koha/admin/background_jobs.pl line 55
Comment 1 Jonathan Druart 2022-07-27 08:35:01 UTC
Created attachment 138169 [details] [review]
Bug 31245: Do not try to report results if task not started

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 2 Jonathan Druart 2022-07-27 08:37:27 UTC
Created attachment 138170 [details] [review]
[ALTERNATIVE-PATCH] Bug 31245: Do not call report for job detail view if job not started

We should not generate the report if the job hasn't started yet

Test plan:
Stop the koha-worker processes, modify a new item using the batch mod
tool, then see the detail page of the job

Without this patch an error is generated:
Can't use an undefined value as an ARRAY reference at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateItem.pm line 175
Comment 3 Jonathan Druart 2022-07-27 08:37:57 UTC
What about this alternative patch, Tomas?
Comment 4 Kyle M Hall 2022-07-27 11:28:35 UTC
(In reply to Jonathan Druart from comment #3)
> What about this alternative patch, Tomas?

Why not both?
Comment 5 Tomás Cohen Arazi 2022-07-27 11:30:39 UTC
I felt like doing it your way, Jonathan, would place unnecessary constraints on what we want to display. Specially for plugins. We could want to display something on other background jobs in the 'new' status.

But I think either solution is fine. If required we can step back later.
Comment 6 Kyle M Hall 2022-07-27 11:37:46 UTC
(In reply to Tomás Cohen Arazi from comment #5)
> I felt like doing it your way, Jonathan, would place unnecessary constraints
> on what we want to display. Specially for plugins. We could want to display
> something on other background jobs in the 'new' status.


^ This is a good point I hadn't thought of
Comment 7 Jonathan Druart 2022-07-28 06:34:09 UTC
I don't understand your use case. "new": the job hasn't started yet. report: what happened, the job has started (still running or failed or succeeded).
Comment 8 Tomás Cohen Arazi 2022-07-29 19:20:06 UTC
(In reply to Jonathan Druart from comment #7)
> I don't understand your use case. "new": the job hasn't started yet. report:
> what happened, the job has started (still running or failed or succeeded).

Ok, let's go ahead with your alternative patch. We can enhance it later if needed.
Comment 9 Andrew Fuerste-Henry 2022-08-03 20:54:12 UTC
Without these patches, I'm getting this same error on master when trying to view a job that's currently underway (status='started'). I'm on a sandbox, so I don't have a way to stop the koha-workers for testing. Here's how I've been testing:

1 - write a report, SELECT itemnumber FROM items. With the default dev data, that's 961 items.
2 - use your report to send all of those to batch mod. The page will take a while to load
3 - while your batch mod page is loading, open background_jobs.pl in another tab and have it ready to go
4 - modify something in all 961 records -- add a public note or something -- once you submit this job it will take about 45 seconds to complete
5 - BEFORE THE JOB IS DONE - reload background_jobs.pl, see your job is listed and shows status=started. try to view details of your job, get an error
6 - once the job has finished, click View again and see your job details successfully

With these patches, I do not get an error. When I try to see the details for a job that's underway, the details page loads fine. It shows the status (started) and progress (122/961) correctly. However, it says "No items modified." This is inaccurate. If we can't load and list the items that have already been successfully modified, I'd rather the message say something like "This job is not yet complete."

All that said, these patches do what they say and correct a messy error, so I'm signing off. Maybe that message can get changed in a followup.
Comment 10 ByWater Sandboxes 2022-08-03 20:54:48 UTC
Created attachment 138550 [details] [review]
Bug 31245: Do not try to report results if task not started

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Andrew <andrewfh@dubcolib.org>
Comment 11 ByWater Sandboxes 2022-08-03 20:54:51 UTC
Created attachment 138551 [details] [review]
Bug 31245: Do not call report for job detail view if job not started

We should not generate the report if the job hasn't started yet

Test plan:
Stop the koha-worker processes, modify a new item using the batch mod
tool, then see the detail page of the job

Without this patch an error is generated:
Can't use an undefined value as an ARRAY reference at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateItem.pm line 175

Signed-off-by: Andrew <andrewfh@dubcolib.org>
Comment 12 Jonathan Druart 2022-08-10 08:56:03 UTC
Upping severity
Comment 13 Martin Renvoize 2022-08-10 09:42:04 UTC
Created attachment 138968 [details] [review]
Bug 31245: Do not call report for job detail view if job not started

We should not generate the report if the job hasn't started yet

Test plan:
Stop the koha-worker processes, modify a new item using the batch mod
tool, then see the detail page of the job

Without this patch an error is generated:
Can't use an undefined value as an ARRAY reference at /kohadevbox/koha/Koha/BackgroundJob/BatchUpdateItem.pm line 175

Signed-off-by: Andrew <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2022-08-10 09:43:11 UTC
Created attachment 138969 [details] [review]
Bug 31245: Do not try to report results if task not started

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Andrew <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2022-08-10 09:44:00 UTC
So, it's belt and braces with both patches.. one to prevent explosion of we somehow call the method still and one to prevent calling the method in the first place.

I'm happy here, Passing QA
Comment 16 Jonathan Druart 2022-08-10 13:43:05 UTC
With bug 31336 it seems that we are still not doing things correctly.

Shouldn't we display the detail only if status != new, started, running?
Comment 17 Tomás Cohen Arazi 2022-08-16 12:20:30 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 18 Lucas Gass 2022-09-30 22:49:16 UTC
Backported to 22.05.x for 22.05.06
Comment 19 Victor Grousset/tuxayo 2022-10-16 00:48:53 UTC
There isn't "View detail of the enqueued job" in 21.05.x so I bet it's not affected by it? Or is there another way to reach the view that blows up?

(note 21.11.x backported it)
Comment 20 Arthur Suzuki 2022-10-22 22:09:06 UTC
Thanks!

Pushed to 21.11 for 21.11.12
Comment 21 Jonathan Druart 2022-11-11 12:58:14 UTC
*** Bug 31290 has been marked as a duplicate of this bug. ***
Comment 22 Jonathan Druart 2022-12-02 14:10:04 UTC
*** Bug 31561 has been marked as a duplicate of this bug. ***