We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue...
Aren't the parameter confusing but the outcome correct?
This one could be considered critical actually.
Created attachment 144400 [details] [review] Bug 32394: Restore long tasks queue We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue... Test plan: Read code and confirm the above. More fun for test plan will be found on bug 32393
(In reply to Tomás Cohen Arazi from comment #1) > Aren't the parameter confusing but the outcome correct? All jobs are using the default queue.
(In reply to Jonathan Druart from comment #4) > All jobs are using the default queue. I thought ES indexing was supposed to use the long one?
(In reply to David Cook from comment #5) > (In reply to Jonathan Druart from comment #4) > > All jobs are using the default queue. > > I thought ES indexing was supposed to use the long one? Maybe I had that backwards. I see now on the patch...
Created attachment 144444 [details] [review] Bug 32394: Restore long tasks queue We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue... Test plan: Read code and confirm the above. More fun for test plan will be found on bug 32393 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
prove t/db_dependent/Koha/BackgroundJob* t/db_dependent/Koha/BackgroundJob.t ...................... ok t/db_dependent/Koha/BackgroundJobs/BatchDeleteBiblio.t ... ok t/db_dependent/Koha/BackgroundJobs/BatchDeleteItem.t ..... ok t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t ... ok t/db_dependent/Koha/BackgroundJobs/StageMARCForImport.t .. ok t/db_dependent/Koha/BackgroundJobs.t ..................... ok All tests successful.
Created attachment 144524 [details] [review] Bug 32394: Restore long tasks queue We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue... Test plan: Read code and confirm the above. More fun for test plan will be found on bug 32393 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Warning for the RM: trivial fix but module changes without unit tests ;) This got perhaps unnoticed since we didnt test ?
I will provide tests.
(In reply to Marcel de Rooy from comment #10) > Warning for the RM: trivial fix but module changes without unit tests ;) > This got perhaps unnoticed since we didnt test ? Thanks. My plan was to provide the tests on pushing. But better fail it
Created attachment 144567 [details] [review] Bug 32394: Add tests
Created attachment 144568 [details] [review] Bug 32394: Add tests
Created attachment 144585 [details] [review] Bug 32394: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 144586 [details] [review] Bug 32394: Restore long tasks queue We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue... Test plan: Read code and confirm the above. More fun for test plan will be found on bug 32393 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 144587 [details] [review] Bug 32394: Add tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to 22.11.x for the next release
Created attachment 144670 [details] [review] Bug 32394: (follow-up) Add param for Koha::BackgroundJob::StageMARCForImport->enqueue Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This doesn't apply cleanly to 22.05.x, if needed please rebase
Created attachment 145964 [details] [review] Bug 32394: [22.05.x] Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 145965 [details] [review] Bug 32394: [22.05.x] Restore long tasks queue We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue... Test plan: Read code and confirm the above. More fun for test plan will be found on bug 32393 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 145966 [details] [review] Bug 32394: [22.05.x] Add tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I have not tested the rebase!
(In reply to Jonathan Druart from comment #26) > I have not tested the rebase! Thanks Jonathan, it seems to work but I'd like one more sign-off besides my own before I push to 22.05.x
(In reply to Jonathan Druart from comment #26) > I have not tested the rebase! Tomas and I tested the rebase together and everything is good.
Backported to 22.05.x for upcoming 22.05.10
depends on 27783, can't applied on 21.11.x