From e77982d9f060767da7e38e1e0a10a323a95e9a56 Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Thu, 29 Jun 2023 13:57:06 +0000 Subject: [PATCH] Bug 30719: (QA follow-up) Squash: This is a squash of 25 QA patches located at: https://github.com/PTFS-Europe/koha/commits/new_30719 Bug 30719: (QA follow-up) Batch column should be hidden by default Bug 30719: (QA follow-up) Fix wrong tt filter type Bug 30719: (QA follow-up) Make atomicupdate idempotent Bug 30719: (QA follow-up) Use COMMENT syntax in database files Bug 30719: (QA follow-up) Fix tiny boolean is_system Bug 30719: (QA follow-up) Add missing CONSTRAINT entries from kohastructure.sql to the atomicupdate file Bug 30719: (QA follow-up) Add missing koha_object_class and koha_objects_class methods Bug 30719: (QA follow-up) Swap search to find Bug 30719: (QA follow-up) Fix tests Bug 30719: (QA follow-up) API terminology - id -> batch_id Bug 30719: (QA follow-up) API terminology - borrowernumber -> patron_id Bug 30719: (QA follow-up) API terminology - branchcode -> library_id Bug 30719: (QA follow-up) Make mandatory illbatch_statuses translatable Bug 30719: (QA follow-up) Improve translatability Bug 30719: (QA follow-up) Fix capitalization of Interlibrary Loan Bug 30719: (QA follow-up) Change Branch to Library in ILL batches table Bug 30719: (QA follow-up) Add template WRAPPER to batch statuses breadrcrumbs Bug 30719: (QA follow-up) Utilize patron_to_html function to display patron info in batches table Bug 30719: (QA follow-up) Add mandatory batch statuses to the atomicupdate Bug 30719: (QA follow-up) Add page-section to the batch statuses list page Bug 30719: (QA follow-up) Style Save button on batch status edit page Bug 30719: (QA follow-up) Add question mark to label string, rephrase new ILL batch button Bug 30719: (QA follow-up) Add noExport class to action columns in batch list table and batch modal table Bug 30719: (QA follow-up) Add page-section and headers to ILL batch table Bug 30719: (QA follow-up) Perltidy --- Koha/Illbatch.pm | 80 +++--- Koha/IllbatchStatus.pm | 88 +++--- Koha/REST/V1/IllbatchStatuses.pm | 41 ++- Koha/REST/V1/Illbatches.pm | 108 ++++---- Koha/REST/V1/Illrequests.pm | 8 +- Koha/Schema/Result/Illbatch.pm | 8 + Koha/Schema/Result/IllbatchStatus.pm | 12 + admin/columns_settings.yml | 1 + admin/ill_batch_statuses.pl | 44 ++- api/v1/swagger/definitions/illbatch.yaml | 8 +- .../swagger/definitions/illbatchstatus.yaml | 2 +- .../atomicupdate/bug_30719_add_ill_batches.pl | 189 ++++++++++--- .../mysql/en/mandatory/illbatch_statuses.yml | 42 +++ installer/data/mysql/kohastructure.sql | 20 +- .../mysql/mandatory/illbatch_statuses.sql | 5 - .../prog/en/includes/admin-menu.inc | 2 +- .../en/includes/ill-batch-modal-strings.inc | 2 +- .../prog/en/includes/ill-batch.inc | 8 +- .../prog/en/includes/ill-toolbar.inc | 2 +- .../prog/en/modules/admin/admin-home.tt | 4 +- .../en/modules/admin/ill_batch_statuses.tt | 107 ++++---- .../prog/en/modules/ill/ill-requests.tt | 9 +- .../intranet-tmpl/prog/js/ill-batch-modal.js | 28 +- .../intranet-tmpl/prog/js/ill-batch-table.js | 21 +- t/db_dependent/IllbatchStatuses.t | 172 +++++++----- t/db_dependent/Illbatches.t | 54 ++-- t/db_dependent/api/v1/ill_requests.t | 67 +++-- t/db_dependent/api/v1/illbatches.t | 252 +++++++----------- t/db_dependent/api/v1/illbatchstatuses.t | 169 +++++------- 29 files changed, 850 insertions(+), 703 deletions(-) create mode 100644 installer/data/mysql/en/mandatory/illbatch_statuses.yml delete mode 100644 installer/data/mysql/mandatory/illbatch_statuses.sql diff --git a/Koha/Illbatch.pm b/Koha/Illbatch.pm index f0e427eec9..e5f65dbab7 100644 --- a/Koha/Illbatch.pm +++ b/Koha/Illbatch.pm @@ -39,10 +39,8 @@ Return the status object associated with this batch =cut sub status { - my ( $self ) = @_; - return Koha::IllbatchStatus->_new_from_dbic( - scalar $self->_result->statuscode - ); + my ($self) = @_; + return Koha::IllbatchStatus->_new_from_dbic( scalar $self->_result->statuscode ); } =head3 patron @@ -54,10 +52,8 @@ Return the patron object associated with this batch =cut sub patron { - my ( $self ) = @_; - return Koha::Patron->_new_from_dbic( - scalar $self->_result->borrowernumber - ); + my ($self) = @_; + return Koha::Patron->_new_from_dbic( scalar $self->_result->borrowernumber ); } =head3 branch @@ -69,10 +65,8 @@ Return the branch object associated with this batch =cut sub branch { - my ( $self ) = @_; - return Koha::Library->_new_from_dbic( - scalar $self->_result->branchcode - ); + my ($self) = @_; + return Koha::Library->_new_from_dbic( scalar $self->_result->branchcode ); } =head3 requests_count @@ -84,10 +78,8 @@ Return the number of requests associated with this batch =cut sub requests_count { - my ( $self ) = @_; - return Koha::Illrequests->search({ - batch_id => $self->id - })->count; + my ($self) = @_; + return Koha::Illrequests->search( { batch_id => $self->id } )->count; } =head3 create_and_log @@ -99,18 +91,20 @@ Log batch creation following storage =cut sub create_and_log { - my ( $self ) = @_; + my ($self) = @_; $self->store; my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_create', - objectnumber => $self->id, - infos => to_json({}) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_create', + objectnumber => $self->id, + infos => to_json( {} ) + } + ); } =head3 update_and_log @@ -129,7 +123,7 @@ sub update_and_log { branchcode => $self->branchcode }; - $self->set( $params ); + $self->set($params); my $update = $self->store; my $after = { @@ -139,15 +133,19 @@ sub update_and_log { my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_update', - objectnumber => $self->id, - infos => to_json({ - before => $before, - after => $after - }) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_update', + objectnumber => $self->id, + infos => to_json( + { + before => $before, + after => $after + } + ) + } + ); } =head3 delete_and_log @@ -159,16 +157,18 @@ Log batch delete =cut sub delete_and_log { - my ( $self ) = @_; + my ($self) = @_; my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_delete', - objectnumber => $self->id, - infos => to_json({}) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_delete', + objectnumber => $self->id, + infos => to_json( {} ) + } + ); $self->delete; } diff --git a/Koha/IllbatchStatus.pm b/Koha/IllbatchStatus.pm index c49ec48703..acd2a7ed64 100644 --- a/Koha/IllbatchStatus.pm +++ b/Koha/IllbatchStatus.pm @@ -39,34 +39,36 @@ Log batch status creation following storage =cut sub create_and_log { - my ( $self ) = @_; + my ($self) = @_; # Ensure code is uppercase and contains only word characters my $fixed_code = uc $self->code; $fixed_code =~ s/\W/_/; # Ensure this status doesn't already exist - my $status = Koha::IllbatchStatuses->find({ code => $fixed_code }); + my $status = Koha::IllbatchStatuses->find( { code => $fixed_code } ); if ($status) { - return { - error => "Duplicate status found" - }; + return { error => "Duplicate status found" }; } # Ensure system statuses can't be created - $self->set({ - code => $fixed_code, - is_system => 0 - })->store; + $self->set( + { + code => $fixed_code, + is_system => 0 + } + )->store; my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_status_create', - objectnumber => $self->id, - infos => to_json({}) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_status_create', + objectnumber => $self->id, + infos => to_json( {} ) + } + ); } =head3 update_and_log @@ -80,31 +82,29 @@ Log batch status update following storage sub update_and_log { my ( $self, $params ) = @_; - my $before = { - name => $self->name - }; + my $before = { name => $self->name }; # Ensure only the name can be changed - $self->set({ - name => $params->{name} - }); + $self->set( { name => $params->{name} } ); my $update = $self->store; - my $after = { - name => $self->name - }; + my $after = { name => $self->name }; my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_status_update', - objectnumber => $self->id, - infos => to_json({ - before => $before, - after => $after - }) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_status_update', + objectnumber => $self->id, + infos => to_json( + { + before => $before, + after => $after + } + ) + } + ); } =head3 delete_and_log @@ -116,25 +116,27 @@ Log batch status delete =cut sub delete_and_log { - my ( $self ) = @_; + my ($self) = @_; # Don't permit deletion of system statuses - if ($self->is_system) { + if ( $self->is_system ) { return; } # Update all batches that use this status to have status UNKNOWN - my $affected = Koha::Illbatches->search({ statuscode => $self->code }); - $affected->update({ statuscode => 'UNKNOWN'}); + my $affected = Koha::Illbatches->search( { statuscode => $self->code } ); + $affected->update( { statuscode => 'UNKNOWN' } ); my $logger = Koha::Illrequest::Logger->new; - $logger->log_something({ - modulename => 'ILL', - actionname => 'batch_status_delete', - objectnumber => $self->id, - infos => to_json({}) - }); + $logger->log_something( + { + modulename => 'ILL', + actionname => 'batch_status_delete', + objectnumber => $self->id, + infos => to_json( {} ) + } + ); $self->delete; } diff --git a/Koha/REST/V1/IllbatchStatuses.pm b/Koha/REST/V1/IllbatchStatuses.pm index 28f398efb1..28d19da073 100644 --- a/Koha/REST/V1/IllbatchStatuses.pm +++ b/Koha/REST/V1/IllbatchStatuses.pm @@ -52,20 +52,18 @@ sub get { my $status_code = $c->validation->param('illbatchstatus_code'); - my $status = Koha::IllbatchStatuses->find({ code => $status_code }); + my $status = Koha::IllbatchStatuses->find( { code => $status_code } ); - if (not defined $status) { + if ( not defined $status ) { return $c->render( - status => 404, + status => 404, openapi => { error => "ILL batch status not found" } ); } return $c->render( - status => 200, - openapi => { - %{$status->unblessed} - } + status => 200, + openapi => { %{ $status->unblessed } } ); } @@ -80,11 +78,11 @@ sub add { my $body = $c->validation->param('body'); - my $status = Koha::IllbatchStatus->new( $body ); + my $status = Koha::IllbatchStatus->new($body); return try { my $return = $status->create_and_log; - if ($return && $return->{error}) { + if ( $return && $return->{error} ) { return $c->render( status => 500, openapi => $return @@ -95,8 +93,7 @@ sub add { openapi => $status ); } - } - catch { + } catch { $c->unhandled_exception($_); }; } @@ -110,7 +107,7 @@ Update a batch status sub update { my $c = shift->openapi->valid_input or return; - my $status = Koha::IllbatchStatuses->find({ code => $c->validation->param('illbatchstatus_code') }); + my $status = Koha::IllbatchStatuses->find( { code => $c->validation->param('illbatchstatus_code') } ); if ( not defined $status ) { return $c->render( @@ -122,15 +119,15 @@ sub update { my $params = $c->req->json; return try { + # Only permit updating of name - $status->update_and_log({ name => $params->{name} }); + $status->update_and_log( { name => $params->{name} } ); return $c->render( status => 200, openapi => $status ); - } - catch { + } catch { $c->unhandled_exception($_); }; } @@ -145,21 +142,23 @@ sub delete { my $c = shift->openapi->valid_input or return; - my $status = Koha::IllbatchStatuses->find({ code => $c->validation->param( 'illbatchstatus_code' ) }); + my $status = Koha::IllbatchStatuses->find( { code => $c->validation->param('illbatchstatus_code') } ); if ( not defined $status ) { return $c->render( status => 404, openapi => { errors => [ { message => "ILL batch status not found" } ] } ); } - if ( $status->is_system) { - return $c->render( status => 400, openapi => { errors => [ { message => "ILL batch status cannot be deleted" } ] } ); + if ( $status->is_system ) { + return $c->render( + status => 400, + openapi => { errors => [ { message => "ILL batch status cannot be deleted" } ] } + ); } return try { $status->delete_and_log; - return $c->render( status => 204, openapi => ''); - } - catch { + return $c->render( status => 204, openapi => '' ); + } catch { $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Illbatches.pm b/Koha/REST/V1/Illbatches.pm index fbadb6ecd6..6c23a3036e 100644 --- a/Koha/REST/V1/Illbatches.pm +++ b/Koha/REST/V1/Illbatches.pm @@ -47,47 +47,46 @@ sub list { # Get all patrons associated with all our batches # in one go my $patrons = {}; - foreach my $batch(@batches) { + foreach my $batch (@batches) { my $patron_id = $batch->borrowernumber; - $patrons->{$patron_id} = 1 - }; - my @patron_ids = keys %{$patrons}; - my $patron_results = Koha::Patrons->search({ - borrowernumber => { -in => \@patron_ids } - }); + $patrons->{$patron_id} = 1; + } + my @patron_ids = keys %{$patrons}; + my $patron_results = Koha::Patrons->search( { borrowernumber => { -in => \@patron_ids } } ); # Get all branches associated with all our batches # in one go my $branches = {}; - foreach my $batch(@batches) { + foreach my $batch (@batches) { my $branch_id = $batch->branchcode; - $branches->{$branch_id} = 1 - }; - my @branchcodes = keys %{$branches}; - my $branch_results = Koha::Libraries->search({ - branchcode => { -in => \@branchcodes } - }); + $branches->{$branch_id} = 1; + } + my @branchcodes = keys %{$branches}; + my $branch_results = Koha::Libraries->search( { branchcode => { -in => \@branchcodes } } ); # Get all batch statuses associated with all our batches # in one go my $statuses = {}; - foreach my $batch(@batches) { + foreach my $batch (@batches) { my $code = $batch->statuscode; - $statuses->{$code} = 1 - }; - my @statuscodes = keys %{$statuses}; - my $status_results = Koha::IllbatchStatuses->search({ - code => { -in => \@statuscodes } - }); + $statuses->{$code} = 1; + } + my @statuscodes = keys %{$statuses}; + my $status_results = Koha::IllbatchStatuses->search( { code => { -in => \@statuscodes } } ); # Populate the response my @to_return = (); - foreach my $it_batch(@batches) { - my $patron = $patron_results->find({ borrowernumber => $it_batch->borrowernumber}); - my $branch = $branch_results->find({ branchcode => $it_batch->branchcode }); - my $status = $status_results->find({ code => $it_batch->statuscode }); + foreach my $it_batch (@batches) { + my $patron = $patron_results->find( { borrowernumber => $it_batch->borrowernumber } ); + my $branch = $branch_results->find( { branchcode => $it_batch->branchcode } ); + my $status = $status_results->find( { code => $it_batch->statuscode } ); push @to_return, { - %{$it_batch->unblessed}, + batch_id => $it_batch->id, + backend => $it_batch->backend, + library_id => $it_batch->branchcode, + name => $it_batch->name, + statuscode => $it_batch->statuscode, + patron_id => $it_batch->borrowernumber, patron => $patron, branch => $branch, status => $status, @@ -111,17 +110,22 @@ sub get { my $batch = Koha::Illbatches->find($batchid); - if (not defined $batch) { + if ( not defined $batch ) { return $c->render( - status => 404, + status => 404, openapi => { error => "ILL batch not found" } ); } return $c->render( - status => 200, + status => 200, openapi => { - %{$batch->unblessed}, + batch_id => $batch->id, + backend => $batch->backend, + library_id => $batch->branchcode, + name => $batch->name, + statuscode => $batch->statuscode, + patron_id => $batch->borrowernumber, patron => $batch->patron->unblessed, branch => $batch->branch->unblessed, status => $batch->status->unblessed, @@ -143,7 +147,7 @@ sub add { # We receive cardnumber, so we need to look up the corresponding # borrowernumber - my $patron = Koha::Patrons->find({ cardnumber => $body->{cardnumber} }); + my $patron = Koha::Patrons->find( { cardnumber => $body->{cardnumber} } ); if ( not defined $patron ) { return $c->render( @@ -154,26 +158,31 @@ sub add { delete $body->{cardnumber}; $body->{borrowernumber} = $patron->borrowernumber; + $body->{branchcode} = delete $body->{library_id}; return try { - my $batch = Koha::Illbatch->new( $body ); + my $batch = Koha::Illbatch->new($body); $batch->create_and_log; $c->res->headers->location( $c->req->url->to_string . '/' . $batch->id ); my $ret = { - %{$batch->unblessed}, - patron => $batch->patron->unblessed, - branch => $batch->branch->unblessed, - status => $batch->status->unblessed, - requests_count => 0 + batch_id => $batch->id, + backend => $batch->backend, + library_id => $batch->branchcode, + name => $batch->name, + statuscode => $batch->statuscode, + patron_id => $batch->borrowernumber, + patron => $batch->patron->unblessed, + branch => $batch->branch->unblessed, + status => $batch->status->unblessed, + requests_count => 0 }; return $c->render( status => 201, openapi => $ret ); - } - catch { + } catch { if ( blessed $_ ) { if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) { return $c->render( @@ -206,12 +215,19 @@ sub update { my $params = $c->req->json; delete $params->{cardnumber}; + $params->{borrowernumber} = delete $params->{patron_id} if $params->{patron_id}; + $params->{branchcode} = delete $params->{library_id} if $params->{library_id}; return try { - $batch->update_and_log( $params ); + $batch->update_and_log($params); my $ret = { - %{$batch->unblessed}, + batch_id => $batch->id, + backend => $batch->backend, + library_id => $batch->branchcode, + name => $batch->name, + statuscode => $batch->statuscode, + patron_id => $batch->borrowernumber, patron => $batch->patron->unblessed, branch => $batch->branch->unblessed, status => $batch->status->unblessed, @@ -222,8 +238,7 @@ sub update { status => 200, openapi => $ret ); - } - catch { + } catch { $c->unhandled_exception($_); }; } @@ -238,7 +253,7 @@ sub delete { my $c = shift->openapi->valid_input or return; - my $batch = Koha::Illbatches->find( $c->validation->param( 'illbatch_id' ) ); + my $batch = Koha::Illbatches->find( $c->validation->param('illbatch_id') ); if ( not defined $batch ) { return $c->render( status => 404, openapi => { error => "ILL batch not found" } ); @@ -246,9 +261,8 @@ sub delete { return try { $batch->delete_and_log; - return $c->render( status => 204, openapi => ''); - } - catch { + return $c->render( status => 204, openapi => '' ); + } catch { $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Illrequests.pm b/Koha/REST/V1/Illrequests.pm index a647df5894..ffa512d802 100644 --- a/Koha/REST/V1/Illrequests.pm +++ b/Koha/REST/V1/Illrequests.pm @@ -93,14 +93,12 @@ sub add { my $create_result = &{$create_api}($body, $request); my $new_id = $create_result->illrequest_id; - my @new_req = Koha::Illrequests->search({ - illrequest_id => $new_id - })->as_list; + my $new_req = Koha::Illrequests->find($new_id); - $c->res->headers->location($c->req->url->to_string . '/' . $new_req[0]->illrequest_id); + $c->res->headers->location($c->req->url->to_string . '/' . $new_req->illrequest_id); return $c->render( status => 201, - openapi => $new_req[0]->to_api + openapi => $new_req->to_api ); } ); diff --git a/Koha/Schema/Result/Illbatch.pm b/Koha/Schema/Result/Illbatch.pm index 0f5b414494..22c402b254 100644 --- a/Koha/Schema/Result/Illbatch.pm +++ b/Koha/Schema/Result/Illbatch.pm @@ -197,4 +197,12 @@ __PACKAGE__->belongs_to( # Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-09-08 13:49:29 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:YKxQxJMKxdBP9X4+i0Rfzw +sub koha_object_class { + 'Koha::Illbatch'; +} + +sub koha_objects_class { + 'Koha::Illbatches'; +} + 1; diff --git a/Koha/Schema/Result/IllbatchStatus.pm b/Koha/Schema/Result/IllbatchStatus.pm index 1f0b86025e..64b6fd867e 100644 --- a/Koha/Schema/Result/IllbatchStatus.pm +++ b/Koha/Schema/Result/IllbatchStatus.pm @@ -114,4 +114,16 @@ __PACKAGE__->has_many( # Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-09-08 13:49:29 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:yo60FJ+kyRj8QuEMac8CFA +__PACKAGE__->add_columns( + '+is_system' => { is_boolean => 1 }, +); + +sub koha_object_class { + 'Koha::IllbatchStatus'; +} + +sub koha_objects_class { + 'Koha::IllbatchStatuses'; +} + 1; diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml index 892d3c3610..56d6012a5e 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -827,6 +827,7 @@ modules: columnname: ill_request_id - columnname: batch + is_hidden: 1 - columnname: metadata_author - diff --git a/admin/ill_batch_statuses.pl b/admin/ill_batch_statuses.pl index bf16b06ded..b6b9721fc2 100755 --- a/admin/ill_batch_statuses.pl +++ b/admin/ill_batch_statuses.pl @@ -18,11 +18,11 @@ # along with Koha; if not, see . use Modern::Perl; -use CGI qw ( -utf8 ); +use CGI qw ( -utf8 ); use Try::Tiny qw( catch try ); use C4::Context; -use C4::Auth qw( get_template_and_user ); +use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_html_with_http_headers ); use Koha::IllbatchStatus; @@ -35,55 +35,51 @@ my @messages; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { - template_name => "admin/ill_batch_statuses.tt", - query => $input, - type => "intranet", - flagsrequired => { parameters => 'ill' }, + template_name => "admin/ill_batch_statuses.tt", + query => $input, + type => "intranet", + flagsrequired => { parameters => 'ill' }, } ); my $status; if ($code) { - $status = Koha::IllbatchStatuses->find({ code => $code }); + $status = Koha::IllbatchStatuses->find( { code => $code } ); } if ( $op eq 'add_form' ) { if ($status) { - $template->param( - status => $status - ); + $template->param( status => $status ); } -} -elsif ( $op eq 'add_validate' ) { +} elsif ( $op eq 'add_validate' ) { my $name = $input->param('name'); my $code = $input->param('code'); if ( not defined $status ) { - $status = Koha::IllbatchStatus->new( { - name => $name, - code => $code - } ); + $status = Koha::IllbatchStatus->new( + { + name => $name, + code => $code + } + ); } try { - if ($status->id) { - $status->update_and_log({ name => $name }); + if ( $status->id ) { + $status->update_and_log( { name => $name } ); } else { $status->create_and_log; } push @messages, { type => 'message', code => 'success_on_saving' }; - } - catch { + } catch { push @messages, { type => 'error', code => 'error_on_saving' }; }; $op = 'list'; -} -elsif ( $op eq 'delete' ) { +} elsif ( $op eq 'delete' ) { try { $status->delete_and_log; push @messages, { code => 'success_on_delete', type => 'message' }; - } - catch { + } catch { push @messages, { code => 'error_on_delete', type => 'alert' }; }; diff --git a/api/v1/swagger/definitions/illbatch.yaml b/api/v1/swagger/definitions/illbatch.yaml index d3146a3303..177c06c35b 100644 --- a/api/v1/swagger/definitions/illbatch.yaml +++ b/api/v1/swagger/definitions/illbatch.yaml @@ -1,7 +1,7 @@ --- type: object properties: - id: + batch_id: type: string description: Internal ILL batch identifier name: @@ -13,10 +13,10 @@ properties: cardnumber: type: string description: Card number of the patron of the ILL batch - borrowernumber: + patron_id: type: string description: Borrower number of the patron of the ILL batch - branchcode: + library_id: type: string description: Branch code of the branch of the ILL batch patron: @@ -44,5 +44,5 @@ additionalProperties: false required: - name - backend - - branchcode + - library_id - statuscode \ No newline at end of file diff --git a/api/v1/swagger/definitions/illbatchstatus.yaml b/api/v1/swagger/definitions/illbatchstatus.yaml index 42b2452551..f133154669 100644 --- a/api/v1/swagger/definitions/illbatchstatus.yaml +++ b/api/v1/swagger/definitions/illbatchstatus.yaml @@ -11,7 +11,7 @@ properties: type: string description: Unique, immutable status code is_system: - type: string + type: boolean description: Is this status required for system operation additionalProperties: false required: diff --git a/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl b/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl index fa6dc47e43..c3a378b69c 100755 --- a/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl +++ b/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl @@ -1,54 +1,157 @@ use Modern::Perl; return { - bug_number => "30719", + bug_number => "30719", description => "Add ILL batches", - up => sub { + up => sub { my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; - $dbh->do(q{ - CREATE TABLE IF NOT EXISTS `illbatches` ( - `id` int(11) NOT NULL auto_increment, -- Batch ID - `name` varchar(100) NOT NULL, -- Unique name of batch - `backend` varchar(20) NOT NULL, -- Name of batch backend - `borrowernumber` int(11), -- Patron associated with batch - `branchcode` varchar(50), -- Branch associated with batch - `statuscode` varchar(20), -- Status of batch - PRIMARY KEY (`id`), - UNIQUE KEY `u_illbatches__name` (`name`) - ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci - }); - $dbh->do(q{ + my ( $dbh, $out ) = @$args{qw(dbh out)}; + $dbh->do( + q{ CREATE TABLE IF NOT EXISTS `illbatch_statuses` ( - `id` int(11) NOT NULL auto_increment, -- Status ID - `name` varchar(100) NOT NULL, -- Name of status - `code` varchar(20) NOT NULL, -- Unique, immutable code for status - `is_system` int(1), -- Is this status required for system operation + `id` int(11) NOT NULL auto_increment COMMENT "Status ID", + `name` varchar(100) NOT NULL COMMENT "Name of status", + `code` varchar(20) NOT NULL COMMENT "Unique, immutable code for status", + `is_system` tinyint(1) COMMENT "Is this status required for system operation", PRIMARY KEY (`id`), UNIQUE KEY `u_illbatchstatuses__code` (`code`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci - }); - $dbh->do(q{ - ALTER TABLE `illrequests` - ADD COLUMN `batch_id` int(11) AFTER backend -- Optional ID of batch that this request belongs to - }); - $dbh->do(q{ - ALTER TABLE `illrequests` - ADD CONSTRAINT `illrequests_ibfk` FOREIGN KEY (`batch_id`) REFERENCES `illbatches` (`id`) ON DELETE SET NULL ON UPDATE CASCADE - }); - $dbh->do(q{ - ALTER TABLE `illbatches` - ADD CONSTRAINT `illbatches_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE - }); - $dbh->do(q{ - ALTER TABLE `illbatches` - ADD CONSTRAINT `illbatches_bcfk` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE - }); - $dbh->do(q{ - ALTER TABLE `illbatches` - ADD CONSTRAINT `illbatches_sfk` FOREIGN KEY (`statuscode`) REFERENCES `illbatch_statuses` (`code`) ON DELETE SET NULL ON UPDATE CASCADE - }); - - say $out "Bug 30719: Add ILL batches completed" + } + ); + $dbh->do( + q{ + CREATE TABLE IF NOT EXISTS `illbatches` ( + `id` int(11) NOT NULL auto_increment COMMENT "Batch ID", + `name` varchar(100) NOT NULL COMMENT "Unique name of batch", + `backend` varchar(20) NOT NULL COMMENT "Name of batch backend", + `borrowernumber` int(11) COMMENT "Patron associated with batch", + `branchcode` varchar(50) COMMENT "Branch associated with batch", + `statuscode` varchar(20) COMMENT "Status of batch", + PRIMARY KEY (`id`), + UNIQUE KEY `u_illbatches__name` (`name`), + CONSTRAINT `illbatches_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, + CONSTRAINT `illbatches_bcfk` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE, + CONSTRAINT `illbatches_sfk` FOREIGN KEY (`statuscode`) REFERENCES `illbatch_statuses` (`code`) ON DELETE SET NULL ON UPDATE CASCADE + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci + } + ); + unless ( column_exists( 'illrequests', 'batch_id' ) ) { + $dbh->do( + q{ + ALTER TABLE `illrequests` + ADD COLUMN `batch_id` int(11) AFTER backend -- Optional ID of batch that this request belongs to + } + ); + } + + unless ( foreign_key_exists( 'illrequests', 'illrequests_ibfk' ) ) { + $dbh->do( + q{ + ALTER TABLE `illrequests` + ADD CONSTRAINT `illrequests_ibfk` FOREIGN KEY (`batch_id`) REFERENCES `illbatches` (`id`) ON DELETE SET NULL ON UPDATE CASCADE + } + ); + } + + unless ( foreign_key_exists( 'illbatches', 'illbatches_bnfk' ) ) { + $dbh->do( + q{ + ALTER TABLE `illbatches` + ADD CONSTRAINT `illbatches_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE + } + ); + } + + unless ( foreign_key_exists( 'illbatches', 'illbatches_bcfk' ) ) { + $dbh->do( + q{ + ALTER TABLE `illbatches` + ADD CONSTRAINT `illbatches_bcfk` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE + } + ); + } + + unless ( foreign_key_exists( 'illbatches', 'illbatches_sfk' ) ) { + $dbh->do( + q{ + ALTER TABLE `illbatches` + ADD CONSTRAINT `illbatches_sfk` FOREIGN KEY (`statuscode`) REFERENCES `illbatch_statuses` (`code`) ON DELETE SET NULL ON UPDATE CASCADE + } + ); + } + + # Get any existing NEW batch status + my ($new_status) = $dbh->selectrow_array( + q| + SELECT name FROM illbatch_statuses WHERE code='NEW'; + | + ); + + if ($new_status) { + say $out "Bug 30719: NEW ILL batch status found. Update has already been run."; + } else { + $dbh->do( + qq{ + INSERT INTO illbatch_statuses ( name, code, is_system ) VALUES ('New', 'NEW', '1') + } + ); + say $out "Bug 30719: Added NEW ILL batch status"; + } + + # Get any existing IN_PROGRESS batch status + my ($in_progress_status) = $dbh->selectrow_array( + q| + SELECT name FROM illbatch_statuses WHERE code='IN_PROGRESS'; + | + ); + + if ($in_progress_status) { + say $out "Bug 30719: IN_PROGRESS ILL batch status found. Update has already been run."; + } else { + $dbh->do( + qq{ + INSERT INTO illbatch_statuses( name, code, is_system ) VALUES( 'In progress', 'IN_PROGRESS', '1' ) + } + ); + say $out "Bug 30719: Added IN_PROGRESS ILL batch status"; + } + + # Get any existing COMPLETED batch status + my ($completed_status) = $dbh->selectrow_array( + q| + SELECT name FROM illbatch_statuses WHERE code='COMPLETED'; + | + ); + + if ($completed_status) { + say $out "Bug 30719: COMPLETED ILL batch status found. Update has already been run."; + } else { + $dbh->do( + qq{ + INSERT INTO illbatch_statuses( name, code, is_system ) VALUES( 'Completed', 'COMPLETED', '1' ) + } + ); + say $out "Bug 30719: Added COMPLETED ILL batch status"; + } + + # Get any existing UNKNOWN batch status + my ($unknown_status) = $dbh->selectrow_array( + q| + SELECT name FROM illbatch_statuses WHERE code='UNKNOWN'; + | + ); + + if ($unknown_status) { + say $out "Bug 30719: UNKNOWN ILL batch status found. Update has already been run."; + } else { + $dbh->do( + qq{ + INSERT INTO illbatch_statuses( name, code, is_system ) VALUES( 'Unknown', 'UNKNOWN', '1' ) + } + ); + say $out "Bug 30719: Added UNKNOWN ILL batch status"; + } + + say $out "Bug 30719: Add ILL batches completed"; }, }; diff --git a/installer/data/mysql/en/mandatory/illbatch_statuses.yml b/installer/data/mysql/en/mandatory/illbatch_statuses.yml new file mode 100644 index 0000000000..0d6c2c4187 --- /dev/null +++ b/installer/data/mysql/en/mandatory/illbatch_statuses.yml @@ -0,0 +1,42 @@ +--- +# +# Copyright 2023 Koha Development Team +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +description: + - "ILL batch statuses used in Koha" + +tables: + - illbatch_statuses: + translatable: [ description ] + multiline: [] + rows: + - code: "NEW" + name: "New" + is_system: "1" + + - code: "IN_PROGRESS" + name: "In progress" + is_system: "1" + + - code: "COMPLETED" + name: "Completed" + is_system: "1" + + - code: "UNKNOWN" + name: "Unknown" + is_system: "1" \ No newline at end of file diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index aabba79017..a40b2e949d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -3306,10 +3306,10 @@ CREATE TABLE `illrequestattributes` ( -- DROP TABLE IF EXISTS `illbatch_statuses`; CREATE TABLE `illbatch_statuses` ( - `id` int(11) NOT NULL auto_increment, -- Status ID - `name` varchar(100) NOT NULL, -- Name of status - `code` varchar(20) NOT NULL, -- Unique, immutable code for status - `is_system` int(1), -- Is this status required for system operation + `id` int(11) NOT NULL auto_increment COMMENT "Status ID", + `name` varchar(100) NOT NULL COMMENT "Name of status", + `code` varchar(20) NOT NULL COMMENT "Unique, immutable code for status", + `is_system` tinyint(1) COMMENT "Is this status required for system operation", PRIMARY KEY (`id`), UNIQUE KEY `u_illbatchstatuses__code` (`code`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; @@ -3319,12 +3319,12 @@ CREATE TABLE `illbatch_statuses` ( -- DROP TABLE IF EXISTS `illbatches`; CREATE TABLE `illbatches` ( - `id` int(11) NOT NULL auto_increment, -- Batch ID - `name` varchar(100) NOT NULL, -- Unique name of batch - `backend` varchar(20) NOT NULL, -- Name of batch backend - `borrowernumber` int(11), -- Patron associated with batch - `branchcode` varchar(50), -- Branch associated with batch - `statuscode` varchar(20), -- Status of batch + `id` int(11) NOT NULL auto_increment COMMENT "Batch ID", + `name` varchar(100) NOT NULL COMMENT "Unique name of batch", + `backend` varchar(20) NOT NULL COMMENT "Name of batch backend", + `borrowernumber` int(11) COMMENT "Patron associated with batch", + `branchcode` varchar(50) COMMENT "Branch associated with batch", + `statuscode` varchar(20) COMMENT "Status of batch", PRIMARY KEY (`id`), UNIQUE KEY `u_illbatches__name` (`name`), CONSTRAINT `illbatches_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, diff --git a/installer/data/mysql/mandatory/illbatch_statuses.sql b/installer/data/mysql/mandatory/illbatch_statuses.sql deleted file mode 100644 index 8c6288eb8b..0000000000 --- a/installer/data/mysql/mandatory/illbatch_statuses.sql +++ /dev/null @@ -1,5 +0,0 @@ -INSERT INTO illbatch_statuses ( name, code, is_system ) VALUES -('New', 'NEW', 1), -('In progress', 'IN_PROGRESS', 1), -('Completed', 'COMPLETED', 1), -('Unknown', 'UNKNOWN', 1); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc index 3be6058a87..c7caffc014 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc @@ -178,7 +178,7 @@
  • Keyboard shortcuts
    • [% END %] [% IF Koha.Preference('ILLModule ') && CAN_user_ill %] -
  • Interlibrary Loan batch statuses
    • +
  • Interlibrary loan batch statuses
    • [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch-modal-strings.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch-modal-strings.inc index 69737a6259..5286f77b5e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch-modal-strings.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch-modal-strings.inc @@ -16,7 +16,7 @@ var ill_button_remove = _("Remove"); var ill_batch_create_api_fail = _("Unable to create batch request"); var ill_batch_update_api_fail = _("Unable to updatecreate batch request"); - var ill_batch_item_remove = _("Are you sure you want to remove this item from the batch"); + var ill_batch_item_remove = _("Are you sure you want to remove this item from the batch?"); var ill_batch_create_cancel_button = _("Close"); var ill_batch_metadata_more = _("More"); var ill_batch_metadata_less = _("Less"); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch.inc index 8c0ca21e74..ebe1a4cf41 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch.inc @@ -1,5 +1,9 @@ [% IF query_type == "batch_list" %] -
    +

    + View ILL requests batches +

    +
    +

    Details for all batches

    @@ -8,7 +12,7 @@ - + diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-toolbar.inc index 4febc377ac..9312ae0525 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/ill-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/ill-toolbar.inc @@ -37,7 +37,7 @@
      [% FOREACH backend IN have_batch %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt index f14be05474..28b17808e1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt @@ -286,8 +286,8 @@
      Define which keys trigger actions in the advanced cataloging editor
      [% END %] [% IF Koha.Preference('ILLModule') && CAN_user_ill %] -
      Interlibrary Loan batch statuses
      -
      Manage the statuses that can be assigned to Interlibrary Loan batches
      +
      Interlibrary loan batch statuses
      +
      Manage the statuses that can be assigned to Interlibrary loan batches
      [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/ill_batch_statuses.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/ill_batch_statuses.tt index c47cc27e2c..66f15e0d0b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/ill_batch_statuses.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/ill_batch_statuses.tt @@ -11,7 +11,7 @@ [% ELSE %] New batch status [% END %] › [% END %] - Interlibrary Loan batch statuses › Administration › Koha + Interlibrary loan batch statuses › Administration › Koha [% INCLUDE 'doc-head-close.inc' %] @@ -20,38 +20,29 @@ [% INCLUDE 'header.inc' %] [% INCLUDE 'prefs-admin-search.inc' %] - + [% END #/ WRAPPER breadcrumbs %] +[% END #/ WRAPPER sub-header.inc %]
      @@ -101,14 +92,18 @@
    • - [% status.is_system ? "Yes" : "No" | html %] + [% IF status.is_system %] + Yes + [% ELSE %] + No + [% END %]
      • - + Cancel
      @@ -119,31 +114,33 @@ New batch status
      -

      Interlibrary Loan batch statuses

      +

      Interlibrary loan batch statuses

      [% IF statuses.count %] -
    Number of requests Status PatronBranchLibrary
    - - - - - - - - [% FOREACH status IN statuses %] - - - - - - - [% END %] - -
    NameCodeIs systemActions
    [% status.name | html %][% status.code | html %][% status.is_system ? "Yes" : "No" | html %] - Edit - [% IF !status.is_system %] - Delete - [% END %] -
    +
    + + + + + + + + + [% FOREACH status IN statuses %] + + + + + + + [% END %] + +
    NameCodeIs systemActions
    [% status.name | html %][% status.code | html %][% status.is_system ? "Yes" : "No" | html %] + Edit + [% IF !status.is_system %] + Delete + [% END %] +
    + [% ELSE %]
    There are no batch statuses defined. Create new batch status diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt index d5c1d894eb..18de16803f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ -669,7 +669,7 @@ [% IF request.batch > 0 %]
  • Batch: - + [% request.batch.name | html %]
    • @@ -806,9 +806,10 @@ [% ELSIF query_type == 'illlist' %]

    - View ILL requests - [% IF batch %] - for batch "[% batch.name | html %]" + [% IF !batch %] + View ILL requests + [% ELSIF batch %] + View ILL requests for batch "[% batch.name | html %]" [% END %]

    diff --git a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js index 9a913a9ee2..607e5582f6 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js +++ b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js @@ -310,7 +310,7 @@ batch_id: batchId, ill_backend_id: batch.data.backend, patron_id: batch.data.patron.borrowernumber, - library_id: batch.data.branchcode, + library_id: batch.data.library_id, extended_attributes: extended_attributes }; window.doCreateSubmission(payload) @@ -378,7 +378,7 @@ var option = document.createElement('option') option.value = status.code; option.text = status.name; - if (batch.data.id && batch.data.statuscode === status.code) { + if (batch.data.batch_id && batch.data.statuscode === status.code) { option.selected = true; } statusesSelect.add(option); @@ -479,7 +479,7 @@ updateBatch() .then(function () { $('#ill-batch-modal').modal({ show: false }); - location.href = '/cgi-bin/koha/ill/ill-requests.pl?batch_id=' + batch.data.id; + location.href = '/cgi-bin/koha/ill/ill-requests.pl?batch_id=' + batch.data.batch_id; }); }; @@ -505,11 +505,11 @@ }) .then(function (jsoned) { batch.data = { - id: jsoned.id, + batch_id: jsoned.batch_id, name: jsoned.name, backend: jsoned.backend, cardnumber: jsoned.cardnumber, - branchcode: jsoned.branchcode, + library_id: jsoned.library_id, statuscode: jsoned.statuscode } return jsoned; @@ -534,7 +534,7 @@ name: nameInput.value, backend: backend, cardnumber: cardnumberInput.value, - branchcode: selectedBranchcode, + library_id: selectedBranchcode, statuscode: selectedStatuscode }) }) @@ -545,13 +545,13 @@ return Promise.reject(response); }) .then(function (body) { - batchId = body.id; + batchId = body.batch_id; batch.data = { - id: body.id, + batch_id: body.batch_id, name: body.name, backend: body.backend, cardnumber: body.patron.cardnumber, - branchcode: body.branchcode, + library_id: body.library_id, statuscode: body.statuscode, patron: body.patron, status: body.status @@ -572,7 +572,7 @@ function updateBatch() { var selectedBranchcode = branchcodeSelect.selectedOptions[0].value; var selectedStatuscode = statusesSelect.selectedOptions[0].value; - return doBatchApiRequest('/' + batch.data.id, { + return doBatchApiRequest('/' + batch.data.batch_id, { method: 'PUT', headers: { 'Content-type': 'application/json' @@ -581,7 +581,7 @@ name: nameInput.value, backend: batch.data.backend, cardnumber: batch.data.patron.cardnumber, - branchcode: selectedBranchcode, + library_id: selectedBranchcode, statuscode: selectedStatuscode }) }) @@ -966,7 +966,7 @@ { width: '18%', render: createActions, - className: 'action-column' + className: 'action-column noExport' } ], createdRow: function (row, data) { @@ -1039,13 +1039,13 @@ } function manageBatchItemsDisplay() { - batchItemsDisplay.style.display = batch.data.id ? 'block' : 'none' + batchItemsDisplay.style.display = batch.data.batch_id ? 'block' : 'none' }; function updateBatchInputs() { nameInput.value = batch.data.name || ''; cardnumberInput.value = batch.data.cardnumber || ''; - branchcodeSelect.value = batch.data.branchcode || ''; + branchcodeSelect.value = batch.data.library_id || ''; } function debounce(func) { diff --git a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js index 605efba436..011d192841 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js +++ b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js @@ -46,7 +46,7 @@ data: batchesProxy.data, columns: [ { - data: 'id', + data: 'batch_id', width: '10%' }, { @@ -76,7 +76,8 @@ { render: createActions, width: '10%', - orderable: false + orderable: false, + className: 'noExport' } ], processing: true, @@ -93,7 +94,7 @@ // A render function for batch name var createName = function (x, y, data) { var a = document.createElement('a'); - a.setAttribute('href', '/cgi-bin/koha/ill/ill-requests.pl?batch_id=' + data.id); + a.setAttribute('href', '/cgi-bin/koha/ill/ill-requests.pl?batch_id=' + data.batch_id); a.setAttribute('title', data.name); a.textContent = data.name; return a.outerHTML; @@ -106,13 +107,7 @@ // A render function for our patron link var createPatronLink = function (data) { - var link = document.createElement('a'); - link.setAttribute('title', ill_batch_borrower_details); - link.setAttribute('href', '/cgi-bin/koha/members/moremember.pl?borrowernumber=' + data.borrowernumber); - var displayText = [data.firstname, data.surname].join(' ') + ' ( ' + data.cardnumber + ' )'; - link.appendChild(document.createTextNode(displayText)); - - return link.outerHTML; + return data ? $patron_to_html(data, { display_cardnumber: true, url: true }) : ''; }; // A render function for our row action buttons @@ -123,13 +118,13 @@ var editButton = document.createElement('button'); editButton.setAttribute('type', 'button'); editButton.setAttribute('class', 'editButton btn btn-xs btn-default'); - editButton.setAttribute('data-batch-id', row.id); + editButton.setAttribute('data-batch-id', row.batch_id); editButton.appendChild(document.createTextNode(ill_batch_edit)); var deleteButton = document.createElement('button'); deleteButton.setAttribute('type', 'button'); deleteButton.setAttribute('class', 'deleteButton btn btn-xs btn-danger'); - deleteButton.setAttribute('data-batch-id', row.id); + deleteButton.setAttribute('data-batch-id', row.batch_id); deleteButton.appendChild(document.createTextNode(ill_batch_delete)); div.appendChild(editButton); @@ -201,7 +196,7 @@ // Remove a batch from our proxy data var removeBatch = function(id) { batchesProxy.data = batchesProxy.data.filter(function (batch) { - return batch.id != id; + return batch.batch_id != id; }); }; diff --git a/t/db_dependent/IllbatchStatuses.t b/t/db_dependent/IllbatchStatuses.t index 487b20d7d4..45fd7b1a03 100755 --- a/t/db_dependent/IllbatchStatuses.t +++ b/t/db_dependent/IllbatchStatuses.t @@ -30,7 +30,7 @@ use Test::MockModule; use Test::More tests => 13; -my $schema = Koha::Database->new->schema; +my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; use_ok('Koha::IllbatchStatus'); use_ok('Koha::IllbatchStatuses'); @@ -48,37 +48,44 @@ my $effects = { # Mock a logger so we can check it is called my $logger = Test::MockModule->new('Koha::Illrequest::Logger'); -$logger->mock('log_something', sub { - my ($self, $to_log ) = @_; - $effects->{$to_log->{actionname}} ++; -}); +$logger->mock( + 'log_something', + sub { + my ( $self, $to_log ) = @_; + $effects->{ $to_log->{actionname} }++; + } +); # Create a batch status -my $status = $builder->build({ - source => 'IllbatchStatus', - value => { - name => "Feeling the call to the Dark Side", - code => "OH_NO", - is_system => 1 +my $status = $builder->build( + { + source => 'IllbatchStatus', + value => { + name => "Feeling the call to the Dark Side", + code => "OH_NO", + is_system => 1 + } } -}); +); -my $status_obj = Koha::IllbatchStatuses->find({ code => $status->{code} }); +my $status_obj = Koha::IllbatchStatuses->find( { code => $status->{code} } ); isa_ok( $status_obj, 'Koha::IllbatchStatus' ); # Try to delete the status, it's a system status, so this should fail $status_obj->delete_and_log; -my $status_obj_del = Koha::IllbatchStatuses->find({ code => $status->{code} }); +my $status_obj_del = Koha::IllbatchStatuses->find( { code => $status->{code} } ); isa_ok( $status_obj_del, 'Koha::IllbatchStatus' ); ## Status create # Try creating a duplicate status -my $status2 = Koha::IllbatchStatus->new({ - name => "Obi-wan", - code => $status->{code}, - is_system => 0 -}); +my $status2 = Koha::IllbatchStatus->new( + { + name => "Obi-wan", + code => $status->{code}, + is_system => 0 + } +); is_deeply( $status2->create_and_log, { error => "Duplicate status found" }, @@ -86,11 +93,13 @@ is_deeply( ); # Create a non-duplicate status and ensure that the logger is called -my $status3 = Koha::IllbatchStatus->new({ - name => "Kylo", - code => "DARK_SIDE", - is_system => 0 -}); +my $status3 = Koha::IllbatchStatus->new( + { + name => "Kylo", + code => "DARK_SIDE", + is_system => 0 + } +); $status3->create_and_log; is( $effects->{'batch_status_create'}, @@ -99,27 +108,33 @@ is( ); # Try creating a system status and ensure it's not created -my $cannot_create_system = Koha::IllbatchStatus->new({ - name => "Jar Jar Binks", - code => "GUNGAN", - is_system => 1 -}); +my $cannot_create_system = Koha::IllbatchStatus->new( + { + name => "Jar Jar Binks", + code => "GUNGAN", + is_system => 1 + } +); $cannot_create_system->create_and_log; -my $created_but_not_system = Koha::IllbatchStatuses->find({ code => "GUNGAN" }); -is($created_but_not_system->{is_system}, undef, "is_system statuses cannot be created"); +my $created_but_not_system = Koha::IllbatchStatuses->find( { code => "GUNGAN" } ); +is( $created_but_not_system->{is_system}, undef, "is_system statuses cannot be created" ); ## Status update # Ensure only name can be updated -$status3->update_and_log({ - name => "Rey", - code => "LIGHT_SIDE", - is_system => 1 -}); +$status3->update_and_log( + { + name => "Rey", + code => "LIGHT_SIDE", + is_system => 1 + } +); + # Get our updated status, if we can get it by it's code, we know that hasn't changed -my $not_updated = Koha::IllbatchStatuses->find({ code => "DARK_SIDE" })->unblessed; -is($not_updated->{is_system}, 0, "is_system cannot be changed"); -is($not_updated->{name}, "Rey", "name can be changed"); +my $not_updated = Koha::IllbatchStatuses->find( { code => "DARK_SIDE" } )->unblessed; +is( $not_updated->{is_system}, 0, "is_system cannot be changed" ); +is( $not_updated->{name}, "Rey", "name can be changed" ); + # Ensure the logger is called is( $effects->{'batch_status_update'}, @@ -128,21 +143,26 @@ is( ); ## Status delete -my $cannot_delete = Koha::IllbatchStatus->new({ - name => "Palapatine", - code => "SITH", - is_system => 1 -})->store; -my $can_delete = Koha::IllbatchStatus->new({ - name => "Windu", - code => "JEDI", - is_system => 0 -}); +my $cannot_delete = Koha::IllbatchStatus->new( + { + name => "Palapatine", + code => "SITH", + is_system => 1 + } +)->store; +my $can_delete = Koha::IllbatchStatus->new( + { + name => "Windu", + code => "JEDI", + is_system => 0 + } +); $cannot_delete->delete_and_log; -my $not_deleted = Koha::IllbatchStatuses->find({ code => "SITH" }); +my $not_deleted = Koha::IllbatchStatuses->find( { code => "SITH" } ); isa_ok( $not_deleted, 'Koha::IllbatchStatus', "is_system statuses cannot be deleted" ); $can_delete->create_and_log; $can_delete->delete_and_log; + # Ensure the logger is called following a successful delete is( $effects->{'batch_status_delete'}, @@ -151,33 +171,41 @@ is( ); # Create a system "UNKNOWN" status -my $status_unknown = Koha::IllbatchStatus->new({ - name => "Unknown", - code => "UNKNOWN", - is_system => 1 -}); +my $status_unknown = Koha::IllbatchStatus->new( + { + name => "Unknown", + code => "UNKNOWN", + is_system => 1 + } +); $status_unknown->create_and_log; + # Create a batch and assign it a status -my $patron = $builder->build_object({ class => 'Koha::Patrons' }); -my $library = $builder->build_object({ class => 'Koha::Libraries' }); -my $status5 = Koha::IllbatchStatus->new({ - name => "Plagueis", - code => "DEAD_SITH", - is_system => 0 -}); +my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); +my $library = $builder->build_object( { class => 'Koha::Libraries' } ); +my $status5 = Koha::IllbatchStatus->new( + { + name => "Plagueis", + code => "DEAD_SITH", + is_system => 0 + } +); $status5->create_and_log; -my $batch = Koha::Illbatch->new({ - name => "My test batch", - borrowernumber => $patron->borrowernumber, - branchcode => $library->branchcode, - backend => "TEST", - statuscode => $status5->code -}); +my $batch = Koha::Illbatch->new( + { + name => "My test batch", + borrowernumber => $patron->borrowernumber, + branchcode => $library->branchcode, + backend => "TEST", + statuscode => $status5->code + } +); $batch->create_and_log; + # Delete the batch status and ensure the batch's status has been changed # to UNKNOWN $status5->delete_and_log; -my $updated_code = Koha::Illbatches->find({ statuscode => "UNKNOWN" }); -is($updated_code->statuscode, "UNKNOWN", "batches attached to deleted status have status changed to UNKNOWN"); +my $updated_code = Koha::Illbatches->find( { statuscode => "UNKNOWN" } ); +is( $updated_code->statuscode, "UNKNOWN", "batches attached to deleted status have status changed to UNKNOWN" ); -$schema->storage->txn_rollback; \ No newline at end of file +$schema->storage->txn_rollback; diff --git a/t/db_dependent/Illbatches.t b/t/db_dependent/Illbatches.t index 0d28a8f44b..4f2b215bca 100755 --- a/t/db_dependent/Illbatches.t +++ b/t/db_dependent/Illbatches.t @@ -30,7 +30,7 @@ use Test::MockModule; use Test::More tests => 8; -my $schema = Koha::Database->new->schema; +my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; use_ok('Koha::Illbatch'); use_ok('Koha::Illbatches'); @@ -40,43 +40,45 @@ $schema->storage->txn_begin; Koha::Illrequests->search->delete; # Create a patron -my $patron = $builder->build({ source => 'Borrower' }); +my $patron = $builder->build( { source => 'Borrower' } ); # Create a librarian -my $librarian = $builder->build({ - source => 'Borrower', - value => { - firstname => "Grogu" +my $librarian = $builder->build( + { + source => 'Borrower', + value => { firstname => "Grogu" } } -}); +); # Create a branch -my $branch = $builder->build({ - source => 'Branch' -}); +my $branch = $builder->build( { source => 'Branch' } ); # Create a batch -my $illbatch = $builder->build({ - source => 'Illbatch', - value => { - name => "My test batch", - backend => "Mock", - borrowernumber => $librarian->{borrowernumber}, - branchcode => $branch->{branchcode} +my $illbatch = $builder->build( + { + source => 'Illbatch', + value => { + name => "My test batch", + backend => "Mock", + borrowernumber => $librarian->{borrowernumber}, + branchcode => $branch->{branchcode} + } } -}); -my $batch_obj = Koha::Illbatches->find($illbatch->{id}); +); +my $batch_obj = Koha::Illbatches->find( $illbatch->{id} ); isa_ok( $batch_obj, 'Koha::Illbatch' ); # Create an ILL request in the batch -my $illrq = $builder->build({ - source => 'Illrequest', - value => { - borrowernumber => $patron->{borrowernumber}, - batch_id => $illbatch->{id} +my $illrq = $builder->build( + { + source => 'Illrequest', + value => { + borrowernumber => $patron->{borrowernumber}, + batch_id => $illbatch->{id} + } } -}); -my $illrq_obj = Koha::Illrequests->find($illrq->{illrequest_id}); +); +my $illrq_obj = Koha::Illrequests->find( $illrq->{illrequest_id} ); # Check requests_count my $requests_count = $batch_obj->requests_count; diff --git a/t/db_dependent/api/v1/ill_requests.t b/t/db_dependent/api/v1/ill_requests.t index 680b0d11f2..0769766778 100755 --- a/t/db_dependent/api/v1/ill_requests.t +++ b/t/db_dependent/api/v1/ill_requests.t @@ -144,6 +144,7 @@ subtest 'list() tests' => sub { class => 'Koha::Illrequests', value => { borrowernumber => $patron->borrowernumber, + batch_id => undef, status => $request_status->{code}, backend => $backend->name, notesstaff => '1' @@ -154,6 +155,7 @@ subtest 'list() tests' => sub { { class => 'Koha::Illrequests', value => { + batch_id => undef, status => $request_status->{code}, backend => $backend->name, status_alias => $av->authorised_value, @@ -291,9 +293,7 @@ subtest 'add() tests' => sub { my $backend = Test::MockObject->new; $backend->set_isa('Koha::Illbackends::Mock'); $backend->set_always('name', 'Mock'); - $backend->set_always('capabilities', sub { - return $illrequest; - } ); + $backend->mock( 'metadata', sub { @@ -310,32 +310,63 @@ subtest 'add() tests' => sub { # Mock Koha::Illrequest::load_backend (to load Mocked Backend) my $illreqmodule = Test::MockModule->new('Koha::Illrequest'); - $illreqmodule->mock( 'load_backend', + $illreqmodule->mock( + 'load_backend', sub { my $self = shift; $self->{_my_backend} = $backend; return $self } ); + $illreqmodule->mock( + '_backend', + sub { + my $self = shift; + $self->{_my_backend} = $backend if ($backend); + + return $self; + } + ); + + $illreqmodule->mock( + 'capabilities', + sub { + my ( $self, $name ) = @_; + + my $capabilities = { + + create_api => sub { + my ($body, $request ) = @_; + + my $api_req = $builder->build_object( + { + class => 'Koha::Illrequests', + value => { + borrowernumber => $patron->borrowernumber, + batch_id => undef, + status => 'NEW', + backend => $backend->name, + } + } + ); + + return $api_req; + } + }; + + return $capabilities->{$name}; + } + ); + $schema->storage->txn_begin; Koha::Illrequests->search->delete; my $body = { - backend => 'Mock', - borrowernumber => $patron->borrowernumber, - branchcode => $library->branchcode, - metadata => { - article_author => "Jessop, E. G.", - article_title => "Sleep", - issn => "0957-4832", - issue => "2", - pages => "89-90", - publisher => "OXFORD UNIVERSITY PRESS", - title => "Journal of public health medicine.", - year => "2001" - } + ill_backend_id => 'Mock', + patron_id => $patron->borrowernumber, + library_id => $library->branchcode }; ## Authorized user test - $t->post_ok( "//$userid:$password@/api/v1/illrequests" => json => $body) + $t->post_ok( "//$userid:$password@/api/v1/ill/requests" => json => $body) ->status_is(201); $schema->storage->txn_rollback; diff --git a/t/db_dependent/api/v1/illbatches.t b/t/db_dependent/api/v1/illbatches.t index 088f7d296f..9ed668ca4b 100755 --- a/t/db_dependent/api/v1/illbatches.t +++ b/t/db_dependent/api/v1/illbatches.t @@ -47,16 +47,12 @@ subtest 'list() tests' => sub { { class => 'Koha::Patrons', value => { - flags => 2 ** 22 # 22 => ill + flags => 2**22 # 22 => ill } } ); - my $branch = $builder->build_object( - { - class => 'Koha::Libraries' - } - ); + my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); my $password = 'sheev_is_da_boss!'; $librarian->set_password( { password => $password, skip_validation => 1 } ); @@ -64,59 +60,53 @@ subtest 'list() tests' => sub { ## Authorized user tests # No batches, so empty array should be returned - $t->get_ok("//$userid:$password@/api/v1/illbatches") - ->status_is(200) - ->json_is( [] ); - - my $batch = $builder->build_object({ - class => 'Koha::Illbatches', - value => { - name => "PapaPalpatine", - backend => "Mock", - borrowernumber => $librarian->borrowernumber, - branchcode => $branch->branchcode + $t->get_ok("//$userid:$password@/api/v1/illbatches")->status_is(200)->json_is( [] ); + + my $batch = $builder->build_object( + { + class => 'Koha::Illbatches', + value => { + name => "PapaPalpatine", + backend => "Mock", + borrowernumber => $librarian->borrowernumber, + branchcode => $branch->branchcode + } } - }); + ); - my $illrq = $builder->build({ - source => 'Illrequest', - value => { - borrowernumber => $librarian->borrowernumber, - batch_id => $batch->id + my $illrq = $builder->build( + { + source => 'Illrequest', + value => { + borrowernumber => $librarian->borrowernumber, + batch_id => $batch->id + } } - }); + ); # One batch created, should get returned - $t->get_ok("//$userid:$password@/api/v1/illbatches") - ->status_is(200) - ->json_has( '/0/id', 'Batch ID' ) - ->json_has( '/0/name', 'Batch name' ) - ->json_has( '/0/backend', 'Backend name' ) - ->json_has( '/0/borrowernumber', 'Borrowernumber' ) - ->json_has( '/0/branchcode', 'Branchcode' ) - ->json_has( '/0/patron', 'patron embedded' ) - ->json_has( '/0/branch', 'branch embedded' ) - ->json_has( '/0/requests_count', 'request count' ); + $t->get_ok("//$userid:$password@/api/v1/illbatches")->status_is(200)->json_has( '/0/batch_id', 'Batch ID' ) + ->json_has( '/0/name', 'Batch name' )->json_has( '/0/backend', 'Backend name' ) + ->json_has( '/0/patron_id', 'Borrowernumber' )->json_has( '/0/library_id', 'Branchcode' ) + ->json_has( '/0/patron', 'patron embedded' )->json_has( '/0/branch', 'branch embedded' ) + ->json_has( '/0/requests_count', 'request count' ); # Try to create a second batch with the same name, this should fail - my $another_batch = $builder->build_object({ class => 'Koha::Illbatches', value => { - name => $batch->name - } }); + my $another_batch = $builder->build_object( { class => 'Koha::Illbatches', value => { name => $batch->name } } ); + # Create a second batch with a different name - my $batch_with_another_name = $builder->build_object({ class => 'Koha::Illbatches' }); + my $batch_with_another_name = $builder->build_object( { class => 'Koha::Illbatches' } ); # Two batches created, they should both be returned - $t->get_ok("//$userid:$password@/api/v1/illbatches") - ->status_is(200) - ->json_has('/0', 'has first batch') - ->json_has('/1', 'has second batch'); + $t->get_ok("//$userid:$password@/api/v1/illbatches")->status_is(200)->json_has( '/0', 'has first batch' ) + ->json_has( '/1', 'has second batch' ); my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { cardnumber => 999, - flags => 0 + flags => 0 } } ); @@ -125,8 +115,7 @@ subtest 'list() tests' => sub { my $unauth_userid = $patron->userid; # Unauthorized access - $t->get_ok("//$unauth_userid:$password@/api/v1/illbatches") - ->status_is(403); + $t->get_ok("//$unauth_userid:$password@/api/v1/illbatches")->status_is(403); $schema->storage->txn_rollback; }; @@ -154,54 +143,44 @@ subtest 'get() tests' => sub { } ); - my $branch = $builder->build_object( + my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); + + my $batch = $builder->build_object( { - class => 'Koha::Libraries' + class => 'Koha::Illbatches', + value => { + name => "LeiaOrgana", + backend => "Mock", + borrowernumber => $librarian->borrowernumber, + branchcode => $branch->branchcode + } } ); - my $batch = $builder->build_object({ - class => 'Koha::Illbatches', - value => { - name => "LeiaOrgana", - backend => "Mock", - borrowernumber => $librarian->borrowernumber, - branchcode => $branch->branchcode - } - }); - - $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - $t->get_ok( "//$userid:$password@/api/v1/illbatches/" . $batch->id ) - ->status_is(200) - ->json_has( '/id', 'Batch ID' ) - ->json_has( '/name', 'Batch name' ) - ->json_has( '/backend', 'Backend name' ) - ->json_has( '/borrowernumber', 'Borrowernumber' ) - ->json_has( '/branchcode', 'Branchcode' ) - ->json_has( '/patron', 'patron embedded' ) - ->json_has( '/branch', 'branch embedded' ) - ->json_has( '/requests_count', 'request count' ); - - $t->get_ok( "//$unauth_userid:$password@/api/v1/illbatches/" . $batch->id ) - ->status_is(403); - - my $batch_to_delete = $builder->build_object({ class => 'Koha::Illbatches' }); + $t->get_ok( "//$userid:$password@/api/v1/illbatches/" . $batch->id )->status_is(200) + ->json_has( '/batch_id', 'Batch ID' )->json_has( '/name', 'Batch name' ) + ->json_has( '/backend', 'Backend name' )->json_has( '/patron_id', 'Borrowernumber' ) + ->json_has( '/library_id', 'Branchcode' )->json_has( '/patron', 'patron embedded' ) + ->json_has( '/branch', 'branch embedded' )->json_has( '/requests_count', 'request count' ); + + $t->get_ok( "//$unauth_userid:$password@/api/v1/illbatches/" . $batch->id )->status_is(403); + + my $batch_to_delete = $builder->build_object( { class => 'Koha::Illbatches' } ); my $non_existent_id = $batch_to_delete->id; $batch_to_delete->delete; - $t->get_ok( "//$userid:$password@/api/v1/illbatches/$non_existent_id" ) - ->status_is(404) - ->json_is( '/error' => 'ILL batch not found' ); + $t->get_ok("//$userid:$password@/api/v1/illbatches/$non_existent_id")->status_is(404) + ->json_is( '/error' => 'ILL batch not found' ); $schema->storage->txn_rollback; }; subtest 'add() tests' => sub { - plan tests =>19; + plan tests => 19; $schema->storage->txn_begin; @@ -225,29 +204,20 @@ subtest 'add() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - my $branch = $builder->build_object( - { - class => 'Koha::Libraries' - } - ); + my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); - my $batch_status = $builder->build_object( - { - class => 'Koha::IllbatchStatuses' - } - ); + my $batch_status = $builder->build_object( { class => 'Koha::IllbatchStatuses' } ); my $batch_metadata = { - name => "Anakin's requests", - backend => "Mock", - cardnumber => $librarian->cardnumber, - branchcode => $branch->branchcode, - statuscode => $batch_status->code + name => "Anakin's requests", + backend => "Mock", + cardnumber => $librarian->cardnumber, + library_id => $branch->branchcode, + statuscode => $batch_status->code }; # Unauthorized attempt to write - $t->post_ok("//$unauth_userid:$password@/api/v1/illbatches" => json => $batch_metadata) - ->status_is(403); + $t->post_ok( "//$unauth_userid:$password@/api/v1/illbatches" => json => $batch_metadata )->status_is(403); # Authorized attempt to write invalid data my $batch_with_invalid_field = { @@ -255,36 +225,28 @@ subtest 'add() tests' => sub { doh => 1 }; - $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_with_invalid_field ) - ->status_is(400) - ->json_is( + $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_with_invalid_field )->status_is(400) + ->json_is( "/errors" => [ { message => "Properties not allowed: doh.", path => "/body" } ] - ); + ); # Authorized attempt to write my $batch_id = - $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_metadata ) - ->status_is( 201 ) - ->json_is( '/name' => $batch_metadata->{name} ) - ->json_is( '/backend' => $batch_metadata->{backend} ) - ->json_is( '/borrowernumber' => $librarian->borrowernumber ) - ->json_is( '/branchcode' => $batch_metadata->{branchcode} ) - ->json_is( '/statuscode' => $batch_status->code ) - ->json_has( '/patron' ) - ->json_has( '/status' ) - ->json_has( '/requests_count' ) - ->json_has( '/branch' ); + $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_metadata )->status_is(201) + ->json_is( '/name' => $batch_metadata->{name} )->json_is( '/backend' => $batch_metadata->{backend} ) + ->json_is( '/patron_id' => $librarian->borrowernumber ) + ->json_is( '/library_id' => $batch_metadata->{library_id} )->json_is( '/statuscode' => $batch_status->code ) + ->json_has('/patron')->json_has('/status')->json_has('/requests_count')->json_has('/branch'); # Authorized attempt to create with null id $batch_metadata->{id} = undef; - $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_metadata ) - ->status_is(400) - ->json_has('/errors'); + $t->post_ok( "//$userid:$password@/api/v1/illbatches" => json => $batch_metadata )->status_is(400) + ->json_has('/errors'); $schema->storage->txn_rollback; }; @@ -315,81 +277,68 @@ subtest 'update() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - my $branch = $builder->build_object( - { - class => 'Koha::Libraries' - } - ); + my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); - my $batch_id = $builder->build_object({ class => 'Koha::Illbatches' } )->id; + my $batch_id = $builder->build_object( { class => 'Koha::Illbatches' } )->id; # Unauthorized attempt to update - $t->put_ok( "//$unauth_userid:$password@/api/v1/illbatches/$batch_id" => json => { name => 'These are not the droids you are looking for' } ) - ->status_is(403); + $t->put_ok( "//$unauth_userid:$password@/api/v1/illbatches/$batch_id" => json => + { name => 'These are not the droids you are looking for' } )->status_is(403); - my $batch_status = $builder->build_object( - { - class => 'Koha::IllbatchStatuses' - } - ); + my $batch_status = $builder->build_object( { class => 'Koha::IllbatchStatuses' } ); # Attempt partial update on a PUT my $batch_with_missing_field = { - backend => "Mock", - borrowernumber => $librarian->borrowernumber, - branchcode => $branch->branchcode, + backend => "Mock", + patron_id => $librarian->borrowernumber, + library_id => $branch->branchcode, statuscode => $batch_status->code }; $t->put_ok( "//$userid:$password@/api/v1/illbatches/$batch_id" => json => $batch_with_missing_field ) - ->status_is(400) - ->json_is( "/errors" => - [ { message => "Missing property.", path => "/body/name" } ] - ); + ->status_is(400)->json_is( "/errors" => [ { message => "Missing property.", path => "/body/name" } ] ); # Full object update on PUT my $batch_with_updated_field = { - name => "Master Ploo Koon", - backend => "Mock", - borrowernumber => $librarian->borrowernumber, - branchcode => $branch->branchcode, + name => "Master Ploo Koon", + backend => "Mock", + patron_id => $librarian->borrowernumber, + library_id => $branch->branchcode, statuscode => $batch_status->code }; $t->put_ok( "//$userid:$password@/api/v1/illbatches/$batch_id" => json => $batch_with_updated_field ) - ->status_is(200) - ->json_is( '/name' => 'Master Ploo Koon' ); + ->status_is(200)->json_is( '/name' => 'Master Ploo Koon' ); # Authorized attempt to write invalid data my $batch_with_invalid_field = { - doh => 1, - name => "Master Mace Windu", + doh => 1, + name => "Master Mace Windu", backend => "Mock" }; $t->put_ok( "//$userid:$password@/api/v1/illbatches/$batch_id" => json => $batch_with_invalid_field ) - ->status_is(400) - ->json_is( + ->status_is(400)->json_is( "/errors" => [ { message => "Properties not allowed: doh.", path => "/body" } ] - ); + ); - my $batch_to_delete = $builder->build_object({ class => 'Koha::Cities' }); + my $batch_to_delete = $builder->build_object( { class => 'Koha::Cities' } ); my $non_existent_id = $batch_to_delete->id; $batch_to_delete->delete; $t->put_ok( "//$userid:$password@/api/v1/illbatches/$non_existent_id" => json => $batch_with_updated_field ) - ->status_is(404); + ->status_is(404); # Wrong method (POST) $batch_with_updated_field->{id} = 2; $t->post_ok( "//$userid:$password@/api/v1/illbatches/$batch_id" => json => $batch_with_updated_field ) - ->status_is(404); + ->status_is(404); $schema->storage->txn_rollback; }; @@ -420,17 +369,14 @@ subtest 'delete() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - my $batch_id = $builder->build_object({ class => 'Koha::Illbatches' })->id; + my $batch_id = $builder->build_object( { class => 'Koha::Illbatches' } )->id; # Unauthorized attempt to delete - $t->delete_ok( "//$unauth_userid:$password@/api/v1/illbatches/$batch_id" ) - ->status_is(403); + $t->delete_ok("//$unauth_userid:$password@/api/v1/illbatches/$batch_id")->status_is(403); - $t->delete_ok("//$userid:$password@/api/v1/illbatches/$batch_id") - ->status_is(204); + $t->delete_ok("//$userid:$password@/api/v1/illbatches/$batch_id")->status_is(204); - $t->delete_ok("//$userid:$password@/api/v1/illbatches/$batch_id") - ->status_is(404); + $t->delete_ok("//$userid:$password@/api/v1/illbatches/$batch_id")->status_is(404); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/api/v1/illbatchstatuses.t b/t/db_dependent/api/v1/illbatchstatuses.t index 335b4365a3..2bfce7f898 100755 --- a/t/db_dependent/api/v1/illbatchstatuses.t +++ b/t/db_dependent/api/v1/illbatchstatuses.t @@ -46,7 +46,7 @@ subtest 'list() tests' => sub { { class => 'Koha::Patrons', value => { - flags => 2 ** 22 # 22 => ill + flags => 2**22 # 22 => ill } } ); @@ -56,26 +56,22 @@ subtest 'list() tests' => sub { ## Authorized user tests # No statuses, so empty array should be returned - $t->get_ok("//$userid:$password@/api/v1/illbatchstatuses") - ->status_is(200) - ->json_is( [] ); - - my $status = $builder->build_object({ - class => 'Koha::IllbatchStatuses', - value => { - name => "Han Solo", - code => "SOLO", - is_system => 0 + $t->get_ok("//$userid:$password@/api/v1/illbatchstatuses")->status_is(200)->json_is( [] ); + + my $status = $builder->build_object( + { + class => 'Koha::IllbatchStatuses', + value => { + name => "Han Solo", + code => "SOLO", + is_system => 0 + } } - }); + ); # One batch created, should get returned - $t->get_ok("//$userid:$password@/api/v1/illbatchstatuses") - ->status_is(200) - ->json_has( '/0/id', 'ID' ) - ->json_has( '/0/name', 'Name' ) - ->json_has( '/0/code', 'Code' ) - ->json_has( '/0/is_system', 'is_system' ); + $t->get_ok("//$userid:$password@/api/v1/illbatchstatuses")->status_is(200)->json_has( '/0/id', 'ID' ) + ->json_has( '/0/name', 'Name' )->json_has( '/0/code', 'Code' )->json_has( '/0/is_system', 'is_system' ); $schema->storage->txn_rollback; }; @@ -96,14 +92,16 @@ subtest 'get() tests' => sub { $librarian->set_password( { password => $password, skip_validation => 1 } ); my $userid = $librarian->userid; - my $status = $builder->build_object({ - class => 'Koha::IllbatchStatuses', - value => { - name => "Han Solo", - code => "SOLO", - is_system => 0 + my $status = $builder->build_object( + { + class => 'Koha::IllbatchStatuses', + value => { + name => "Han Solo", + code => "SOLO", + is_system => 0 + } } - }); + ); # Unauthorised user my $patron = $builder->build_object( @@ -115,30 +113,25 @@ subtest 'get() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - $t->get_ok( "//$userid:$password@/api/v1/illbatchstatuses/" . $status->code ) - ->status_is(200) - ->json_has( '/id', 'ID' ) - ->json_has( '/name', 'Name' ) - ->json_has( '/code', 'Code' ) - ->json_has( '/is_system', 'is_system' ); + $t->get_ok( "//$userid:$password@/api/v1/illbatchstatuses/" . $status->code )->status_is(200) + ->json_has( '/id', 'ID' )->json_has( '/name', 'Name' )->json_has( '/code', 'Code' ) + ->json_has( '/is_system', 'is_system' ); - $t->get_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/" . $status->id ) - ->status_is(403); + $t->get_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/" . $status->id )->status_is(403); - my $status_to_delete = $builder->build_object({ class => 'Koha::IllbatchStatuses' }); + my $status_to_delete = $builder->build_object( { class => 'Koha::IllbatchStatuses' } ); my $non_existent_code = $status_to_delete->code; $status_to_delete->delete; - $t->get_ok( "//$userid:$password@/api/v1/illbatchstatuses/$non_existent_code" ) - ->status_is(404) - ->json_is( '/error' => 'ILL batch status not found' ); + $t->get_ok("//$userid:$password@/api/v1/illbatchstatuses/$non_existent_code")->status_is(404) + ->json_is( '/error' => 'ILL batch status not found' ); $schema->storage->txn_rollback; }; subtest 'add() tests' => sub { - plan tests =>14; + plan tests => 14; $schema->storage->txn_begin; @@ -162,14 +155,13 @@ subtest 'add() tests' => sub { my $unauth_userid = $patron->userid; my $status_metadata = { - name => "In a bacta tank", - code => "BACTA", - is_system => 0 + name => "In a bacta tank", + code => "BACTA", + is_system => 0 }; # Unauthorized attempt to write - $t->post_ok("//$unauth_userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata) - ->status_is(403); + $t->post_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata )->status_is(403); # Authorized attempt to write invalid data my $status_with_invalid_field = { @@ -177,31 +169,26 @@ subtest 'add() tests' => sub { doh => 1 }; - $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_with_invalid_field ) - ->status_is(400) - ->json_is( + $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_with_invalid_field )->status_is(400) + ->json_is( "/errors" => [ { message => "Properties not allowed: doh.", path => "/body" } ] - ); + ); # Authorized attempt to write my $status_id = - $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata ) - ->status_is( 201 ) - ->json_has( '/id', 'ID' ) - ->json_has( '/name', 'Name' ) - ->json_has( '/code', 'Code' ) + $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata )->status_is(201) + ->json_has( '/id', 'ID' )->json_has( '/name', 'Name' )->json_has( '/code', 'Code' ) ->json_has( '/is_system', 'is_system' ); # Authorized attempt to create with null id $status_metadata->{id} = undef; - $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata ) - ->status_is(400) - ->json_has('/errors'); + $t->post_ok( "//$userid:$password@/api/v1/illbatchstatuses" => json => $status_metadata )->status_is(400) + ->json_has('/errors'); $schema->storage->txn_rollback; }; @@ -231,11 +218,11 @@ subtest 'update() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - my $status_code = $builder->build_object({ class => 'Koha::IllbatchStatuses' } )->code; + my $status_code = $builder->build_object( { class => 'Koha::IllbatchStatuses' } )->code; # Unauthorized attempt to update - $t->put_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/$status_code" => json => { name => 'These are not the droids you are looking for' } ) - ->status_is(403); + $t->put_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/$status_code" => json => + { name => 'These are not the droids you are looking for' } )->status_is(403); # Attempt partial update on a PUT my $status_with_missing_field = { @@ -244,21 +231,17 @@ subtest 'update() tests' => sub { }; $t->put_ok( "//$userid:$password@/api/v1/illbatchstatuses/$status_code" => json => $status_with_missing_field ) - ->status_is(400) - ->json_is( "/errors" => - [ { message => "Missing property.", path => "/body/name" } ] - ); + ->status_is(400)->json_is( "/errors" => [ { message => "Missing property.", path => "/body/name" } ] ); # Full object update on PUT my $status_with_updated_field = { - name => "Master Ploo Koon", - code => $status_code, - is_system => 0 + name => "Master Ploo Koon", + code => $status_code, + is_system => 0 }; $t->put_ok( "//$userid:$password@/api/v1/illbatchstatuses/$status_code" => json => $status_with_updated_field ) - ->status_is(200) - ->json_is( '/name' => 'Master Ploo Koon' ); + ->status_is(200)->json_is( '/name' => 'Master Ploo Koon' ); # Authorized attempt to write invalid data my $status_with_invalid_field = { @@ -268,22 +251,22 @@ subtest 'update() tests' => sub { }; $t->put_ok( "//$userid:$password@/api/v1/illbatchstatuses/$status_code" => json => $status_with_invalid_field ) - ->status_is(400) - ->json_is( + ->status_is(400)->json_is( "/errors" => [ { message => "Properties not allowed: doh.", path => "/body" } ] - ); + ); - my $status_to_delete = $builder->build_object({ class => 'Koha::IllbatchStatuses' }); + my $status_to_delete = $builder->build_object( { class => 'Koha::IllbatchStatuses' } ); my $non_existent_code = $status_to_delete->code; $status_to_delete->delete; - $t->put_ok( "//$userid:$password@/api/v1/illbatchstatuses/$non_existent_code" => json => $status_with_updated_field ) - ->status_is(404); + $t->put_ok( + "//$userid:$password@/api/v1/illbatchstatuses/$non_existent_code" => json => $status_with_updated_field ) + ->status_is(404); $schema->storage->txn_rollback; }; @@ -314,39 +297,29 @@ subtest 'delete() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - my $non_system_status = $builder->build_object({ - class => 'Koha::IllbatchStatuses', - value => { - is_system => 0 + my $non_system_status = $builder->build_object( + { + class => 'Koha::IllbatchStatuses', + value => { is_system => 0 } } - }); + ); - my $system_status = $builder->build_object({ - class => 'Koha::IllbatchStatuses', - value => { - is_system => 1 + my $system_status = $builder->build_object( + { + class => 'Koha::IllbatchStatuses', + value => { is_system => 1 } } - }); + ); # Unauthorized attempt to delete - $t->delete_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code ) - ->status_is(403); + $t->delete_ok( "//$unauth_userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code )->status_is(403); - $t->delete_ok("//$userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code ) - ->status_is(204); + $t->delete_ok( "//$userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code )->status_is(204); - $t->delete_ok("//$userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code ) - ->status_is(404); + $t->delete_ok( "//$userid:$password@/api/v1/illbatchstatuses/" . $non_system_status->code )->status_is(404); - $t->delete_ok("//$userid:$password@/api/v1/illbatchstatuses/" . $system_status->code ) - ->status_is(400) - ->json_is( - "/errors" => [ - { - message => "ILL batch status cannot be deleted" - } - ] - ); + $t->delete_ok( "//$userid:$password@/api/v1/illbatchstatuses/" . $system_status->code )->status_is(400) + ->json_is( "/errors" => [ { message => "ILL batch status cannot be deleted" } ] ); $schema->storage->txn_rollback; }; -- 2.30.2