Bug 30350

Summary: Do not hardcode job types
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED DUPLICATE QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: dcook, fridolin.somers, jonathan.druart, kyle, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27434
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 30172    
Bug Blocks:    
Attachments: Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Description Tomás Cohen Arazi 2022-03-24 14:04:39 UTC
Currently background_jobs_worker.pl has a hardcoded list. It should be using a method in Koha::BackgrounJob(s) instead, avoiding the need to maintain this list. Right now it could use Koha::BackgroundJob::type_to_class_mapping.
Comment 1 Tomás Cohen Arazi 2022-03-25 13:06:33 UTC
Created attachment 132246 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Jonathan Druart 2022-03-25 14:10:07 UTC
The point was to have different workers with different job types.
Comment 3 Tomás Cohen Arazi 2022-03-25 15:15:06 UTC
(In reply to Jonathan Druart from comment #2)
> The point was to have different workers with different job types.

Yeah, I get it, but it is not implemented yet, right? Let's talk about it on monday :-D
Comment 4 Jonathan Druart 2022-03-28 09:17:15 UTC
Created attachment 132298 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

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

JD amended patch
-my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping};
+my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping()};
To remove the following error:
Ambiguous use of %{Koha::BackgroundJob::type_to_class_mapping} resolved to %Koha::BackgroundJob::type_to_class_mapping
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Kyle M Hall 2022-04-01 12:35:39 UTC
Created attachment 132857 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

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

JD amended patch
-my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping};
+my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping()};
To remove the following error:
Ambiguous use of %{Koha::BackgroundJob::type_to_class_mapping} resolved to %Koha::BackgroundJob::type_to_class_mapping
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Tomás Cohen Arazi 2022-04-01 14:41:32 UTC
I would like to take this out of the RM queue for now, as work on bug 27783 might make this require some changes.

If someone (including me) finds this is still relevant, let's make it PQA again.

Thanks Kyle!
Comment 7 Martin Renvoize 2022-04-29 14:13:16 UTC
We opted for Queues in bug 27783.. marking as Resolved Fixed

*** This bug has been marked as a duplicate of bug 27783 ***