We should implement and use a REST API route to retrieve the background job list.
Created attachment 136245 [details] [review] Bug 30982: REST API specs
Created attachment 136246 [details] [review] Bug 30982: Use the REST API for background job list view
Would that work for you, Martin?
Created attachment 136670 [details] [review] Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136671 [details] [review] Bug 30982: Use the REST API for background job list view Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136672 [details] [review] Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call.
This is great, much cleaner! Signing off.
Comment on attachment 136671 [details] [review] Bug 30982: Use the REST API for background job list view Review of attachment 136671 [details] [review]: ----------------------------------------------------------------- ::: Koha/REST/V1/BackgroundJobs.pm @@ +43,5 @@ > + my $background_jobs_set = > + $can_manage_background_jobs > + ? Koha::BackgroundJobs->new > + : Koha::BackgroundJobs->search( > + { borrowernumber => $patron->borrowernumber } ); This feels like it should be achievable with the 'allow_owner: true' swagger definition rather than needing special handling here.
Created attachment 136707 [details] [review] Bug 30982: Add tests and implement GET /background_jobs/$id
Created attachment 136708 [details] [review] Bug 30982: Try allow-owner
Martin, can you have a look at the last patch please? I've tried to implement your suggestion but either it does not work or I don't understand how it's supposed to work. I've added missing tests and implement the GET /background_jobs/$id route.
From IRC: @ashimema> ack.. looking at the code it's allow-owner that I've discussed before and bemoaned it's a bit of a mess. @ashimema> I'm thinking drop that last patch as you've proved it doesn't work right now.. and we can follow through on that stuff in another way..
Created attachment 136715 [details] [review] Bug 30982: Make code more re-usable
(In reply to Martin Renvoize from comment #13) > Created attachment 136715 [details] [review] [review] > Bug 30982: Make code more re-usable Shouldn't you call search_limited instead of search then?
(In reply to Jonathan Druart from comment #14) > (In reply to Martin Renvoize from comment #13) > > Created attachment 136715 [details] [review] [review] [review] > > Bug 30982: Make code more re-usable > > Shouldn't you call search_limited instead of search then? We do so internally: https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/REST/Plugin/Objects.pm#L189-L191 So $c->objects->search is calling a mojolicious helper method above.. as you can see it looks for the presence of a search_limited in the plural object and calls it in place of search if one is found.
I think this is pretty much ready to go.. just needs a QA eye.. which I can't lend as I SO'd :(
Can you please update the patches with some descriptions and a test plan?
Those patches are simply using a new REST API routes to search and display the jobs. It make use of the DT wrapper to let the user search and filter the table, like we do in other places. There is nothing much I could add. Test plan: Add new jobs and filter the table (?)
Applied the patches, restart_all: * The page loads ok, table is empty in my installation * I check the "Only include jobs started in the last hour" checkbox: Error: Something went wrong when loading the table. 500: Internal Server Error In my background_jobs table I have several entries, one for batch_item_record_modification and several update_elastic_index. All are finished.
Created attachment 138114 [details] [review] Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889
Created attachment 139636 [details] [review] Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139637 [details] [review] Bug 30982: Use the REST API for background job list view Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139638 [details] [review] Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call.
Created attachment 139639 [details] [review] Bug 30982: Add tests and implement GET /background_jobs/$id
Created attachment 139640 [details] [review] Bug 30982: Make code more re-usable
Created attachment 139641 [details] [review] Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889
Rebased to fix conflict. Tests still pass. (t/db_dependent/api/v1/background_jobs.t)
(In reply to Katrin Fischer from comment #19) > Applied the patches, restart_all: > > * The page loads ok, table is empty in my installation > * I check the "Only include jobs started in the last hour" checkbox: > Error: Something went wrong when loading the table. > 500: Internal Server Error > > In my background_jobs table I have several entries, one for > batch_item_record_modification and several update_elastic_index. All are > finished. I don't get the issue. Tested with empty db table and after checking in an item of a record on hold I get a Holds queue update and Update Elasticsearch index entry. (why is there an ES entry update?) The change here since your test doesn't look like to be able to fix that. And there were no changes on bug 30462.
Maybe another patch/bug changed things?
The table filter "Type" doesn't work, if you select something else than the empty filter you get no results.
(In reply to Victor Grousset/tuxayo from comment #28) > (In reply to Katrin Fischer from comment #19) > > Applied the patches, restart_all: > > > > * The page loads ok, table is empty in my installation > > * I check the "Only include jobs started in the last hour" checkbox: > > Error: Something went wrong when loading the table. > > 500: Internal Server Error > > > > In my background_jobs table I have several entries, one for > > batch_item_record_modification and several update_elastic_index. All are > > finished. > > I don't get the issue. Tested with empty db table and after checking in an > item of a record on hold I get a Holds queue update and Update Elasticsearch > index entry. > > (why is there an ES entry update?) > > The change here since your test doesn't look like to be able to fix that. > And there were no changes on bug 30462. Unable to reproduce here as well, so maybe fixed in the dependency or there was some configuration issue in the setup katrin used, although if so, then I think we have a bug somewhere.
Created attachment 140488 [details] [review] Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140489 [details] [review] Bug 30982: Use the REST API for background job list view Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140490 [details] [review] Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call.
Created attachment 140491 [details] [review] Bug 30982: Add tests and implement GET /background_jobs/$id
Created attachment 140492 [details] [review] Bug 30982: Make code more re-usable
Created attachment 140493 [details] [review] Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889
Created attachment 140494 [details] [review] Bug 30982: Add Koha::BackgroundJobs->filter_by_current Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 140495 [details] [review] Bug 30982: API tweaks This patch makes the following changes to the 'background_jobs' API: * We now call them 'jobs' * Removed deprecated query parameter definitions * Added only_current query parameter * Controller gets adapted to use $rs->filter_by_current when only_current is passed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 140496 [details] [review] Bug 30982: (follow-up) Only escape asterisk chars when non-exact search Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 140497 [details] [review] Bug 30982: Adapt table to new API spec Disclaimer: this patch is highly opinionated :-D When I started looking at this patch I felt like the two tables (current/past jobs) implemented on bug 30462 was the way to go. In order to make this patches apply after it I had to redo all the things. Or most of them. But I decided to keep the idea of filtering out completed tasks, not just having the option to display 'the last hour' tasks. For the task I added some required helper methods and the relevant tests as well. So a behavior change. Hope you all agree with it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 140498 [details] [review] [OPTIONAL] Bug 30982: Rename 'Background Jobs' => 'Jobs' Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Please review ASAP for pushing it. Thanks everyone!
(In reply to Joonas Kylmälä from comment #30) > The table filter "Type" doesn't work, if you select something else than the > empty filter you get no results. Solved on 'Bug 30982: (follow-up) Only escape asterisk chars when non-exact search'
Created attachment 140589 [details] [review] Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140590 [details] [review] Bug 30982: Use the REST API for background job list view Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140591 [details] [review] Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140592 [details] [review] Bug 30982: Add tests and implement GET /background_jobs/$id Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140593 [details] [review] Bug 30982: Make code more re-usable Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140594 [details] [review] Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140595 [details] [review] Bug 30982: Add Koha::BackgroundJobs->filter_by_current Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140596 [details] [review] Bug 30982: API tweaks This patch makes the following changes to the 'background_jobs' API: * We now call them 'jobs' * Removed deprecated query parameter definitions * Added only_current query parameter * Controller gets adapted to use $rs->filter_by_current when only_current is passed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140597 [details] [review] Bug 30982: (follow-up) Only escape asterisk chars when non-exact search Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140598 [details] [review] Bug 30982: Adapt table to new API spec Disclaimer: this patch is highly opinionated :-D When I started looking at this patch I felt like the two tables (current/past jobs) implemented on bug 30462 was the way to go. In order to make this patches apply after it I had to redo all the things. Or most of them. But I decided to keep the idea of filtering out completed tasks, not just having the option to display 'the last hour' tasks. For the task I added some required helper methods and the relevant tests as well. So a behavior change. Hope you all agree with it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 140599 [details] [review] Bug 30982: Rename 'Background Jobs' => 'Jobs' Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I really like the follow-ups here and think the end result is nicely polished in general. We could perhaps continue to refine UI was at a later stage after the staff client UI refresh.. things like the checkboxes for the set filter on the datatable feel slightly inconsistent/out of place and we do it inconsistently elsewhere too. I wonder if 'Pills' might be a nice generic approach.. I digress. As for the API, I think generally it's an improvement. I again wonder slightly about the quick filtering options tied to additional parameters, but so long as they're well documented on the spec I think they mostly make sense. It's an area I've contemplated a few times now and not come up with a nice solution for. Signing off
Comment on attachment 140597 [details] [review] Bug 30982: (follow-up) Only escape asterisk chars when non-exact search Obsoleting patch as of bug 31565.
If no one beats me, I will qa friday
Created attachment 140789 [details] [review] Bug 30982: (QA follow-up) Resolve pod warning Empty section in previous paragraph at line 32 in file Koha/BackgroundJobs.pm Test plan: Run podchecker again on this module. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
My first impression for this patch Subject: [PATCH 10/11] Bug 30982: Rename 'Background Jobs' => 'Jobs' is: No. This does not make things clearer to me. What is Jobs. New positions within library staff? :) Just curious if I change opinion while going on. As a side note, only changing two templates for this will not make it easier to switch terminology. Example: [% IF CAN_user_parameters_manage_background_jobs %] - <h3>Background jobs</h3> + <h3>Jobs</h3> We will be constantly thinking jobs or background_jobs while editing code..
Resuming later on
Working here now
No blocker, not fixing it here: const job_statuses = [ {'_id': 'new', '_str': _("New")}, {'_id': 'cancelled', '_str': _("Cancelled")}, {'_id': 'finished', '_str': _("Finished")}, {'_id': 'started', '_str': _("Started")}, {'_id': 'running', '_str': _("Running")}, {'_id': 'failed', '_str': _("Failed")}, We have status running and started. Probably an error. The status 'running' may probably be coming from C4::BackgroundJob.pm $self->{'status'} = "running"; It seems that Koha::BackgroundJob does not use 'running'.
> Disclaimer: this patch is highly opinionated :-D I personally think that that is highly exaggerated :)
Created attachment 140884 [details] [review] Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140885 [details] [review] Bug 30982: Use the REST API for background job list view Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140886 [details] [review] Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140887 [details] [review] Bug 30982: Add tests and implement GET /background_jobs/$id Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140888 [details] [review] Bug 30982: Make code more re-usable Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140889 [details] [review] Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140890 [details] [review] Bug 30982: Add Koha::BackgroundJobs->filter_by_current Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140891 [details] [review] Bug 30982: API tweaks This patch makes the following changes to the 'background_jobs' API: * We now call them 'jobs' * Removed deprecated query parameter definitions * Added only_current query parameter * Controller gets adapted to use $rs->filter_by_current when only_current is passed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140892 [details] [review] Bug 30982: Adapt table to new API spec Disclaimer: this patch is highly opinionated :-D When I started looking at this patch I felt like the two tables (current/past jobs) implemented on bug 30462 was the way to go. In order to make this patches apply after it I had to redo all the things. Or most of them. But I decided to keep the idea of filtering out completed tasks, not just having the option to display 'the last hour' tasks. For the task I added some required helper methods and the relevant tests as well. So a behavior change. Hope you all agree with it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140893 [details] [review] Bug 30982: Rename 'Background Jobs' => 'Jobs' Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140894 [details] [review] Bug 30982: (QA follow-up) Resolve pod warning Empty section in previous paragraph at line 32 in file Koha/BackgroundJobs.pm Test plan: Run podchecker again on this module. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140895 [details] [review] Bug 30982: (QA follow-up) Spelling [1] Correct: BackgrounJob [2] If should filter out not current jobs => Had a hard time reading that one until I replaced if by it. => Decided to rephrase it in a more positive way. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140896 [details] [review] Bug 30982: (QA follow-up) Remove redundancy from template The template now contains two lists for both status and type: a TT list and a JS list. The type list already proves that redundancy leads to bugs. We miss three types at one side: Unknown job type 'stage_marc_for_import' Unknown job type 'marc_import_commit_batch' Unknown job type 'marc_import_revert_batch' This patch removes the TT list. And gets the status and type via an additional js call. For that reason I hide the fieldset until document ready. This can be improved later when needed. Test plan: Look at status and type on both job list and detail view. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 140897 [details] [review] Bug 30982: (QA follow-up) No userenv, no jobs + # Assume permission if context has no user + my $can_manage_background_jobs = 1; => This felt a bit unsafe. Test plan: Try interface for jobs. Call API with cookie. Call API with OAuth. Run t/db_dependent/Koha/BackgroundJobs.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Passing QA but with some further comments that imo need to be addressed at some point. [1] The switch Jobs / Background jobs is still a bit arguable for me, especially since we create another patron/borrower/member thing in the codebase. [2] Just want to note that this patch set did not contain any test plans. Although it is REST API, we should just add test plans. [3] A superlibrarian of one with can_manage may see the jobs of other users, no problem, but the interface should show the job owner. It is confusing now since I do not see if it is my job. (The feature was apparently developed with showing only your own jobs in mind.) [4] The default setting for Current jobs only and Started last hour is arguable. With the current setting I never see a job, since they just finished. I tend to say that Current jobs only should be off at the start. Great work for the rest !
s/of one/or one/
Pushed to master for 22.11. Nice work everyone, thanks!
This is wonderful. While I know it is an enhancement, the presence of elastic indexing and holds queue updates in the jobs table can lead to large tables quickly. I would request this one be considered for backport
(In reply to Nick Clemens from comment #82) > This is wonderful. > > While I know it is an enhancement, the presence of elastic indexing and > holds queue updates in the jobs table can lead to large tables quickly. I > would request this one be considered for backport I would be happy to add this patchset to 22.05.x but there are conflicts with this patchset in 22.05.x that I do not feel comfortable solving. Could you rebase?
Created attachment 142859 [details] [review] Bug 30982: [22.05.x] Use the REST API for background job list view Bug 30982: REST API specs Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: (QA follow-up) Double quoting and console.log This patch fixes the issues highlighted by the QA script; We use double quotes for translatable strings in JS and remove an errant console.log call. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Add tests and implement GET /background_jobs/$id Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Make code more re-usable Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Add 'context' to the REST API specs context has been added by bug 30889 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Add Koha::BackgroundJobs->filter_by_current Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: API tweaks This patch makes the following changes to the 'background_jobs' API: * We now call them 'jobs' * Removed deprecated query parameter definitions * Added only_current query parameter * Controller gets adapted to use $rs->filter_by_current when only_current is passed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Adapt table to new API spec Disclaimer: this patch is highly opinionated :-D When I started looking at this patch I felt like the two tables (current/past jobs) implemented on bug 30462 was the way to go. In order to make this patches apply after it I had to redo all the things. Or most of them. But I decided to keep the idea of filtering out completed tasks, not just having the option to display 'the last hour' tasks. For the task I added some required helper methods and the relevant tests as well. So a behavior change. Hope you all agree with it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: Rename 'Background Jobs' => 'Jobs' Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: (QA follow-up) Resolve pod warning Empty section in previous paragraph at line 32 in file Koha/BackgroundJobs.pm Test plan: Run podchecker again on this module. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: (QA follow-up) Spelling [1] Correct: BackgrounJob [2] If should filter out not current jobs => Had a hard time reading that one until I replaced if by it. => Decided to rephrase it in a more positive way. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: (QA follow-up) Remove redundancy from template The template now contains two lists for both status and type: a TT list and a JS list. The type list already proves that redundancy leads to bugs. We miss three types at one side: Unknown job type 'stage_marc_for_import' Unknown job type 'marc_import_commit_batch' Unknown job type 'marc_import_revert_batch' This patch removes the TT list. And gets the status and type via an additional js call. For that reason I hide the fieldset until document ready. This can be improved later when needed. Test plan: Look at status and type on both job list and detail view. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30982: (QA follow-up) No userenv, no jobs + # Assume permission if context has no user + my $can_manage_background_jobs = 1; => This felt a bit unsafe. Test plan: Try interface for jobs. Call API with cookie. Call API with OAuth. Run t/db_dependent/Koha/BackgroundJobs.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Backported to 22.05.x for upcoming 22.05.07 release
unmet dependencies, can't apply on 21.11.14
+ is( Koha::BackgroundJobs->search_limited->count, 0, 'No jobs found without userenv' ); This is wrong if jobs exist. Jenkins is failing because of this.
I don't recreate locally. I've run the different tests that (directly) insert background jobs, there are some leftover in the DB, but the tests are still passing. 21:28:55 koha_1 | # Failed test 'No jobs found without userenv' 21:28:55 koha_1 | # at t/db_dependent/Koha/BackgroundJobs.t line 135. 21:28:55 koha_1 | # got: '2' 21:28:55 koha_1 | # expected: '0' 21:28:55 koha_1 | # Looks like you failed 1 test of 3. 21:28:55 koha_1 | 21:28:55 koha_1 | # Failed test 'search_limited' 21:32:22 koha_1 | # at t/db_dependent/Koha/BackgroundJobs.t line 142. 21:32:22 koha_1 | # Looks like you failed 1 test of 15. 21:32:22 koha_1 | [20:28:55] t/db_dependent/Koha/BackgroundJobs.t
(In reply to Jonathan Druart from comment #88) > I don't recreate locally. I've run the different tests that (directly) > insert background jobs, there are some leftover in the DB, but the tests are > still passing. > > 21:28:55 koha_1 | # Failed test 'No jobs found without userenv' > 21:28:55 koha_1 | # at t/db_dependent/Koha/BackgroundJobs.t line > 135. > 21:28:55 koha_1 | # got: '2' > 21:28:55 koha_1 | # expected: '0' > 21:28:55 koha_1 | # Looks like you failed 1 test of 3. > 21:28:55 koha_1 | > 21:28:55 koha_1 | # Failed test 'search_limited' > 21:32:22 koha_1 | # at t/db_dependent/Koha/BackgroundJobs.t line 142. > 21:32:22 koha_1 | # Looks like you failed 1 test of 15. > 21:32:22 koha_1 | [20:28:55] t/db_dependent/Koha/BackgroundJobs.t See bug 32304.