Bugzilla – Attachment 140596 Details for
Bug 30982
Use the REST API for background job list view
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30982: API tweaks
Bug-30982-API-tweaks.patch (text/plain), 17.18 KB, created by
Martin Renvoize (ashimema)
on 2022-09-13 14:57:29 UTC
(
hide
)
Description:
Bug 30982: API tweaks
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-09-13 14:57:29 UTC
Size:
17.18 KB
patch
obsolete
>From d03d15125337a4e647e0ea8696f9e01a10e728ff Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 12 Sep 2022 13:02:25 -0300 >Subject: [PATCH] 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> >--- > Koha/BackgroundJob.pm | 26 ++++++- > Koha/REST/V1/BackgroundJobs.pm | 39 +++++++--- > .../swagger/definitions/background_job.yaml | 63 --------------- > api/v1/swagger/definitions/job.yaml | 54 +++++++++++++ > .../paths/{background_jobs.yaml => jobs.yaml} | 77 ++++--------------- > api/v1/swagger/swagger.yaml | 27 ++++--- > .../api/v1/{background_jobs.t => jobs.t} | 42 +++++----- > t/lib/TestBuilder.pm | 3 + > 8 files changed, 162 insertions(+), 169 deletions(-) > delete mode 100644 api/v1/swagger/definitions/background_job.yaml > create mode 100644 api/v1/swagger/definitions/job.yaml > rename api/v1/swagger/paths/{background_jobs.yaml => jobs.yaml} (50%) > rename t/db_dependent/api/v1/{background_jobs.t => jobs.t} (68%) > >diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm >index 2a2e9a4185..5b14a78ef2 100644 >--- a/Koha/BackgroundJob.pm >+++ b/Koha/BackgroundJob.pm >@@ -469,6 +469,27 @@ sub plugin_types_to_classes { > return $self->{_plugin_mapping}; > } > >+=head3 to_api >+ >+ my $json = $job->to_api; >+ >+Overloaded method that returns a JSON representation of the Koha::BackgroundJob object, >+suitable for API output. >+ >+=cut >+ >+sub to_api { >+ my ( $self, $params ) = @_; >+ >+ my $json = $self->SUPER::to_api( $params ); >+ >+ $json->{context} = $self->json->decode($self->context) >+ if defined $self->context; >+ $json->{data} = $self->decoded_data; >+ >+ return $json; >+} >+ > =head3 to_api_mapping > > This method returns the mapping for representing a Koha::BackgroundJob object >@@ -478,8 +499,11 @@ on the API. > > sub to_api_mapping { > return { >- id => 'background_job_id', >+ id => 'job_id', > borrowernumber => 'patron_id', >+ ended_on => 'ended_date', >+ enqueued_on => 'enqueued_date', >+ started_on => 'started_date', > }; > } > >diff --git a/Koha/REST/V1/BackgroundJobs.pm b/Koha/REST/V1/BackgroundJobs.pm >index 251d5657f3..cb1543d0fe 100644 >--- a/Koha/REST/V1/BackgroundJobs.pm >+++ b/Koha/REST/V1/BackgroundJobs.pm >@@ -29,49 +29,66 @@ use Try::Tiny; > > =head3 list > >+Controller function that handles listing Koha::BackgrounJob objects >+ > =cut > > sub list { > my $c = shift->openapi->valid_input or return; > > return try { >- my $background_jobs_set = Koha::BackgroundJobs->new; >- my $background_jobs = $c->objects->search($background_jobs_set); >- return $c->render( status => 200, openapi => $background_jobs ); >- } >- catch { >+ >+ my $only_current = delete $c->validation->output->{only_current}; >+ >+ my $bj_rs = Koha::BackgroundJobs->new; >+ >+ if ($only_current) { >+ $bj_rs = $bj_rs->filter_by_current; >+ } >+ >+ return $c->render( >+ status => 200, >+ openapi => $c->objects->search($bj_rs) >+ ); >+ } catch { > $c->unhandled_exception($_); > }; > } > >+=head3 get >+ >+Controller function that handles retrieving a single Koha::BackgroundJob object >+ >+=cut >+ > sub get { > my $c = shift->openapi->valid_input or return; > > return try { > >- my $background_job_id = $c->validation->param('background_job_id'); >- my $patron = $c->stash('koha.user'); >+ my $job_id = $c->validation->param('job_id'); >+ my $patron = $c->stash('koha.user'); > > my $can_manage_background_jobs = > $patron->has_permission( { parameters => 'manage_background_jobs' } ); > >- my $background_job = Koha::BackgroundJobs->find($background_job_id); >+ my $job = Koha::BackgroundJobs->find($job_id); > > return $c->render( > status => 404, > openapi => { error => "Object not found" } >- ) unless $background_job; >+ ) unless $job; > > return $c->render( > status => 403, > openapi => { error => "Cannot see background job info" } > ) > if !$can_manage_background_jobs >- && $background_job->borrowernumber != $patron->borrowernumber; >+ && $job->borrowernumber != $patron->borrowernumber; > > return $c->render( > status => 200, >- openapi => $background_job->to_api >+ openapi => $job->to_api > ); > } > catch { >diff --git a/api/v1/swagger/definitions/background_job.yaml b/api/v1/swagger/definitions/background_job.yaml >deleted file mode 100644 >index 09cbbe652c..0000000000 >--- a/api/v1/swagger/definitions/background_job.yaml >+++ /dev/null >@@ -1,63 +0,0 @@ >---- >-type: object >-properties: >- background_job_id: >- type: integer >- description: internally assigned background_job identifier >- readOnly: true >- status: >- description: background_job status >- type: >- - string >- - "null" >- progress: >- description: background_job progress >- type: >- - string >- - "null" >- size: >- description: background_job size >- type: >- - string >- - "null" >- patron_id: >- description: background_job enqueuer >- type: >- - string >- - "null" >- type: >- description: background_job type >- type: >- - string >- - "null" >- queue: >- description: background_job queue >- type: >- - string >- - "null" >- data: >- description: background_job data >- type: >- - string >- - "null" >- context: >- description: background_job context >- type: >- - string >- - "null" >- enqueued_on: >- description: background_job enqueue date >- type: >- - string >- - "null" >- started_on: >- description: background_job start date >- type: >- - string >- - "null" >- ended_on: >- description: background_job end date >- type: >- - string >- - "null" >-additionalProperties: false >diff --git a/api/v1/swagger/definitions/job.yaml b/api/v1/swagger/definitions/job.yaml >new file mode 100644 >index 0000000000..966e3a3ef8 >--- /dev/null >+++ b/api/v1/swagger/definitions/job.yaml >@@ -0,0 +1,54 @@ >+--- >+type: object >+properties: >+ job_id: >+ type: integer >+ description: internally assigned job identifier >+ readOnly: true >+ status: >+ description: job status >+ type: string >+ progress: >+ description: job progress >+ type: >+ - string >+ - "null" >+ size: >+ description: job size >+ type: >+ - string >+ - "null" >+ patron_id: >+ description: job enqueuer >+ type: >+ - string >+ - "null" >+ type: >+ description: job type >+ type: string >+ queue: >+ description: job queue >+ type: string >+ data: >+ description: job data >+ type: object >+ context: >+ description: job context >+ type: object >+ enqueued_date: >+ description: job enqueue date >+ type: string >+ format: date-time >+ started_date: >+ description: job start date >+ type: >+ - string >+ - "null" >+ format: date-time >+ ended_date: >+ description: job end date >+ type: >+ - string >+ - "null" >+ format: date-time >+additionalProperties: false >diff --git a/api/v1/swagger/paths/background_jobs.yaml b/api/v1/swagger/paths/jobs.yaml >similarity index 50% >rename from api/v1/swagger/paths/background_jobs.yaml >rename to api/v1/swagger/paths/jobs.yaml >index ecc80a21f2..c7f0f98904 100644 >--- a/api/v1/swagger/paths/background_jobs.yaml >+++ b/api/v1/swagger/paths/jobs.yaml >@@ -1,64 +1,19 @@ > --- >-/background_jobs: >+/jobs: > get: > x-mojo-to: BackgroundJobs#list >- operationId: listBackgroundJobs >+ operationId: listJobs > tags: >- - background_jobs >- summary: List background jobs >+ - jobs >+ summary: List jobs > produces: > - application/json > parameters: >- - name: status >+ - name: only_current > in: query >- description: Case insensative search on job status > required: false >- type: string >- - name: progress >- in: query >- description: Case insensative search on job progress >- required: false >- type: string >- - name: size >- in: query >- description: Case insensative search on job size >- required: false >- type: string >- - name: patron_id >- in: query >- description: Case insensative search on job enqueuer id >- required: false >- type: string >- - name: type >- in: query >- description: Case insensative search on job type >- required: false >- type: string >- - name: queue >- in: query >- description: Case insensative search on job queue >- required: false >- type: string >- - name: context >- in: query >- description: Case insensative search on context >- required: false >- type: string >- - name: enqueued_on >- in: query >- description: Case insensative search on job enqueue date >- required: false >- type: string >- - name: started_on >- in: query >- description: Case insensative search on job start date >- required: false >- type: string >- - name: ended_on >- in: query >- description: Case insensative search on job end date >- required: false >- type: string >+ type: boolean >+ description: If should filter out not current jobs > - $ref: "../swagger.yaml#/parameters/match" > - $ref: "../swagger.yaml#/parameters/order_by" > - $ref: "../swagger.yaml#/parameters/page" >@@ -73,7 +28,7 @@ > schema: > type: array > items: >- $ref: "../swagger.yaml#/definitions/background_job" >+ $ref: "../swagger.yaml#/definitions/job" > "403": > description: Access forbidden > schema: >@@ -92,28 +47,28 @@ > x-koha-authorization: > permissions: > catalogue: "1" >-"/background_jobs/{background_job_id}": >+"/jobs/{job_id}": > get: > x-mojo-to: BackgroundJobs#get >- operationId: getBackgroundJob >+ operationId: getJob > tags: >- - background_job >- summary: Get background job >+ - jobs >+ summary: Get a job > parameters: >- - $ref: "../swagger.yaml#/parameters/background_job_id_pp" >+ - $ref: "../swagger.yaml#/parameters/job_id_pp" > produces: > - application/json > responses: > "200": >- description: A background job >+ description: A job > schema: >- $ref: "../swagger.yaml#/definitions/background_job" >+ $ref: "../swagger.yaml#/definitions/job" > "403": > description: Access forbidden > schema: > $ref: "../swagger.yaml#/definitions/error" > "404": >- description: Background job not found >+ description: Job not found > schema: > $ref: "../swagger.yaml#/definitions/error" > "500": >diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml >index 3d4be6e63d..5bdbab2fa8 100644 >--- a/api/v1/swagger/swagger.yaml >+++ b/api/v1/swagger/swagger.yaml >@@ -8,8 +8,6 @@ definitions: > $ref: ./definitions/advancededitormacro.yaml > allows_renewal: > $ref: ./definitions/allows_renewal.yaml >- background_job: >- $ref: ./definitions/background_job.yaml > basket: > $ref: ./definitions/basket.yaml > bundle_link: >@@ -48,6 +46,8 @@ definitions: > $ref: ./definitions/item.yaml > item_group: > $ref: ./definitions/item_group.yaml >+ job: >+ $ref: ./definitions/job.yaml > library: > $ref: ./definitions/library.yaml > order: >@@ -105,10 +105,6 @@ paths: > $ref: "./paths/article_requests.yaml#/~1article_requests~1{article_request_id}" > /auth/otp/token_delivery: > $ref: paths/auth.yaml#/~1auth~1otp~1token_delivery >- /background_jobs: >- $ref: ./paths/background_jobs.yaml#/~1background_jobs >- "/background_jobs/{background_job_id}": >- $ref: "./paths/background_jobs.yaml#/~1background_jobs~1{background_job_id}" > "/biblios/{biblio_id}": > $ref: "./paths/biblios.yaml#/~1biblios~1{biblio_id}" > "/biblios/{biblio_id}/checkouts": >@@ -185,6 +181,10 @@ paths: > $ref: ./paths/items.yaml#/~1items~1{item_id}~1bundled_items~1{bundled_item_id} > "/items/{item_id}/pickup_locations": > $ref: "./paths/items.yaml#/~1items~1{item_id}~1pickup_locations" >+ /jobs: >+ $ref: ./paths/jobs.yaml#/~1jobs >+ "/jobs/{job_id}": >+ $ref: "./paths/jobs.yaml#/~1jobs~1{job_id}" > /libraries: > $ref: ./paths/libraries.yaml#/~1libraries > "/libraries/{library_id}": >@@ -260,12 +260,6 @@ parameters: > name: advancededitormacro_id > required: true > type: integer >- background_job_id_pp: >- description: Job internal identifier >- in: path >- name: background_job_id >- required: true >- type: integer > biblio_id_pp: > description: Record internal identifier > in: path >@@ -338,6 +332,12 @@ parameters: > name: item_id > required: true > type: integer >+ job_id_pp: >+ description: Job internal identifier >+ in: path >+ name: job_id >+ required: true >+ type: integer > library_id_pp: > description: Internal library identifier > in: path >@@ -604,6 +604,9 @@ tags: > - description: "Manage items\n" > name: items > x-displayName: Items >+ - description: "Manage jobs\n" >+ name: jobs >+ x-displayName: Jobs > - description: "Manage libraries\n" > name: libraries > x-displayName: Libraries >diff --git a/t/db_dependent/api/v1/background_jobs.t b/t/db_dependent/api/v1/jobs.t >similarity index 68% >rename from t/db_dependent/api/v1/background_jobs.t >rename to t/db_dependent/api/v1/jobs.t >index 11181b13a8..4d993c68e0 100755 >--- a/t/db_dependent/api/v1/background_jobs.t >+++ b/t/db_dependent/api/v1/jobs.t >@@ -65,11 +65,11 @@ my $patron = $builder->build_object( > $patron->set_password( { password => $password, skip_validation => 1 } ); > my $patron_userid = $patron->userid; > >-$t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs") >+$t->get_ok("//$librarian_userid:$password@/api/v1/jobs") > ->status_is(200) > ->json_is( [] ); > >-my $background_job = $builder->build_object( >+my $job = $builder->build_object( > { > class => 'Koha::BackgroundJobs', > value => { >@@ -86,39 +86,39 @@ my $background_job = $builder->build_object( > ); > > { >- $t->get_ok("//$superlibrarian_userid:$password@/api/v1/background_jobs") >- ->status_is(200)->json_is( [ $background_job->to_api ] ); >+ $t->get_ok("//$superlibrarian_userid:$password@/api/v1/jobs") >+ ->status_is(200)->json_is( [ $job->to_api ] ); > >- $t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs") >+ $t->get_ok("//$librarian_userid:$password@/api/v1/jobs") > ->status_is(200)->json_is( [] ); > >- $t->get_ok("//$patron_userid:$password@/api/v1/background_jobs") >+ $t->get_ok("//$patron_userid:$password@/api/v1/jobs") > ->status_is(403); > >- $background_job->borrowernumber( $librarian->borrowernumber )->store; >+ $job->borrowernumber( $librarian->borrowernumber )->store; > >- $t->get_ok("//$librarian_userid:$password@/api/v1/background_jobs") >- ->status_is(200)->json_is( [ $background_job->to_api ] ); >+ $t->get_ok("//$librarian_userid:$password@/api/v1/jobs") >+ ->status_is(200)->json_is( [ $job->to_api ] ); > } > > { >- $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/background_jobs/" >- . $background_job->id )->status_is(200) >- ->json_is( $background_job->to_api ); >+ $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/jobs/" >+ . $job->id )->status_is(200) >+ ->json_is( $job->to_api ); > >- $t->get_ok( "//$librarian_userid:$password@/api/v1/background_jobs/" >- . $background_job->id )->status_is(200) >- ->json_is( $background_job->to_api ); >+ $t->get_ok( "//$librarian_userid:$password@/api/v1/jobs/" >+ . $job->id )->status_is(200) >+ ->json_is( $job->to_api ); > >- $background_job->borrowernumber( $superlibrarian->borrowernumber )->store; >- $t->get_ok( "//$librarian_userid:$password@/api/v1/background_jobs/" >- . $background_job->id )->status_is(403); >+ $job->borrowernumber( $superlibrarian->borrowernumber )->store; >+ $t->get_ok( "//$librarian_userid:$password@/api/v1/jobs/" >+ . $job->id )->status_is(403); > } > > { >- $background_job->delete; >- $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/background_jobs/" >- . $background_job->id )->status_is(404) >+ $job->delete; >+ $t->get_ok( "//$superlibrarian_userid:$password@/api/v1/jobs/" >+ . $job->id )->status_is(404) > ->json_is( '/error' => 'Object not found' ); > } > >diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm >index 59ba592da0..d6e88e7b8a 100644 >--- a/t/lib/TestBuilder.pm >+++ b/t/lib/TestBuilder.pm >@@ -558,6 +558,9 @@ sub _gen_blob { > sub _gen_default_values { > my ($self) = @_; > return { >+ BackgroundJob => { >+ context => '{}' >+ }, > Borrower => { > login_attempts => 0, > gonenoaddress => undef, >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30982
:
136245
|
136246
|
136670
|
136671
|
136672
|
136707
|
136708
|
136715
|
138114
|
139636
|
139637
|
139638
|
139639
|
139640
|
139641
|
140488
|
140489
|
140490
|
140491
|
140492
|
140493
|
140494
|
140495
|
140496
|
140497
|
140498
|
140589
|
140590
|
140591
|
140592
|
140593
|
140594
|
140595
|
140596
|
140597
|
140598
|
140599
|
140789
|
140884
|
140885
|
140886
|
140887
|
140888
|
140889
|
140890
|
140891
|
140892
|
140893
|
140894
|
140895
|
140896
|
140897
|
142859