|
Description
Jonathan Druart
2022-12-02 13:39:38 UTC
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> Nice work everyone! Pushed to 22.11.x for the next release 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 |