Bugzilla – Attachment 156811 Details for
Bug 30719
ILL should provide the ability to create batch requests
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30719: (QA follow-up) Pick better column names and cleanup
Bug-30719-QA-follow-up-Pick-better-column-names-an.patch (text/plain), 35.77 KB, created by
Pedro Amorim
on 2023-10-11 09:22:42 UTC
(
hide
)
Description:
Bug 30719: (QA follow-up) Pick better column names and cleanup
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2023-10-11 09:22:42 UTC
Size:
35.77 KB
patch
obsolete
>From 83219fd5c06ab92f2f2745a88368f32bfcc6660e Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 10 Oct 2023 16:02:38 -0300 >Subject: [PATCH] Bug 30719: (QA follow-up) Pick better column names and > cleanup > >This patch takes on normalizing the attribute names, embeds, and also >makes the whole API more kosher, in terms of using accessors for related >objects, using the standard structure for strings_map, etc. >--- > Koha/Illbatch.pm | 87 ++++++++------- > Koha/REST/V1/Illbatches.pm | 19 ++-- > Koha/Schema/Result/Borrower.pm | 6 +- > Koha/Schema/Result/Branch.pm | 6 +- > Koha/Schema/Result/Illbatch.pm | 85 +++++++------- > Koha/Schema/Result/IllbatchStatus.pm | 6 +- > Koha/Schema/Result/Illrequest.pm | 6 +- > api/v1/swagger/definitions/ill_batch.yaml | 37 ++++--- > api/v1/swagger/paths/ill_batches.yaml | 9 +- > installer/data/mysql/kohastructure.sql | 18 +-- > t/db_dependent/IllbatchStatuses.t | 10 +- > t/db_dependent/Illbatches.t | 25 ++--- > t/db_dependent/api/v1/ill_batches.t | 129 +++++++++++----------- > 13 files changed, 235 insertions(+), 208 deletions(-) > >diff --git a/Koha/Illbatch.pm b/Koha/Illbatch.pm >index f4163a8ed0..eea37a3d07 100644 >--- a/Koha/Illbatch.pm >+++ b/Koha/Illbatch.pm >@@ -18,9 +18,15 @@ package Koha::Illbatch; > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >+ > use Koha::Database; >+ >+use Koha::Illrequests; > use Koha::Illrequest::Logger; >-use Koha::IllbatchStatus; >+use Koha::IllbatchStatuses; >+use Koha::Libraries; >+use Koha::Patrons; >+ > use JSON qw( to_json ); > use base qw(Koha::Object); > >@@ -40,35 +46,37 @@ Return the status object associated with this batch > > sub status { > my ($self) = @_; >- return Koha::IllbatchStatus->_new_from_dbic( scalar $self->_result->statuscode ); >+ return Koha::IllbatchStatus->_new_from_dbic( scalar $self->_result->status_code ); > } > > =head3 patron > > my $patron = Koha::Illbatch->patron; > >-Return the patron object associated with this batch >+Return the I<Koha::Patron> object associated with this batch > > =cut > > sub patron { > my ($self) = @_; >- my $patron = return Koha::Patrons->find( { borrowernumber => $self->borrowernumber } ); >+ my $patron = $self->_result->patron; > return unless $patron; >+ return Koha::Patron->_new_from_dbic($patron); > } > >-=head3 branch >+=head3 library > >- my $branch = Koha::Illbatch->branch; >+ my $library = Koha::Illbatch->library; > >-Return the branch object associated with this batch >+Return the I<Koha::Library> object associated with this batch > > =cut > >-sub branch { >+sub library { > my ($self) = @_; >- my $library = return Koha::Libraries->find( { branchcode => $self->branchcode } ); >+ my $library = $self->_result->library; > return unless $library; >+ return Koha::Library->_new_from_dbic($library); > } > > =head3 requests_count >@@ -86,13 +94,13 @@ sub requests_count { > > =head3 requests > >-Return the requests for this batch >+Return the I<Koha::Illrequests> for this batch > > =cut > > sub requests { > my ($self) = @_; >- my $requests = $self->_result->illrequests; >+ my $requests = $self->_result->requests; > return Koha::Illrequests->_new_from_dbic($requests); > } > >@@ -134,7 +142,7 @@ sub update_and_log { > > my $before = { > name => $self->name, >- branchcode => $self->branchcode >+ library_id => $self->library_id, > }; > > $self->set($params); >@@ -142,7 +150,7 @@ sub update_and_log { > > my $after = { > name => $self->name, >- branchcode => $self->branchcode >+ library_id => $self->library_id, > }; > > my $logger = Koha::Illrequest::Logger->new; >@@ -192,6 +200,17 @@ sub delete_and_log { > > =head3 strings_map > >+Returns a map of column name to string representations including the string, >+the mapping type and the mapping category where appropriate. >+ >+Currently handles library and ILL batch status expansions. >+expansions. >+ >+Accepts a param hashref where the I<public> key denotes whether we want the public >+or staff client strings. >+ >+Note: the I<public> parameter is not currently used. >+ > =cut > > sub strings_map { >@@ -199,41 +218,31 @@ sub strings_map { > > my $strings = {}; > >- if ( defined $self->statuscode ) { >- my $ill_batch_status = Koha::IllbatchStatuses->search( { code => $self->statuscode } ); >- my $ill_batch_status_str = >- $ill_batch_status->count >- ? $ill_batch_status->next->name >- : $self->statuscode; >+ if ( defined $self->status_code ) { >+ my $status = $self->status; > >- $strings->{status} = { >- name => $ill_batch_status_str, >- }; >+ if ( $status ) { >+ $strings->{status_code} = { >+ str => $status->name, >+ type => 'ill_batch_status', >+ }; >+ } > } > >- if ( defined $self->branchcode ) { >- my $ill_batch_branch = Koha::Libraries->find( $self->branchcode ); >- my $ill_batch_branchname_str = $ill_batch_branch ? $ill_batch_branch->branchname : $self->branchcode; >+ if ( defined $self->library_id ) { >+ my $library = $self->library; > >- $strings->{branchname} = $ill_batch_branchname_str; >+ if ($library) { >+ $strings->{library_id} = { >+ str => $library->branchname, >+ type => 'library', >+ }; >+ } > } > > return $strings; > } > >-=head3 to_api_mapping >- >-=cut >- >-sub to_api_mapping { >- return { >- id => 'batch_id', >- branchcode => 'library_id', >- borrowernumber => 'patron_id', >- status => 'batch_status', >- }; >-} >- > =head3 _type > > my $type = Koha::Illbatch->_type; >diff --git a/Koha/REST/V1/Illbatches.pm b/Koha/REST/V1/Illbatches.pm >index 1b25c7956c..b4e7d6aa02 100644 >--- a/Koha/REST/V1/Illbatches.pm >+++ b/Koha/REST/V1/Illbatches.pm >@@ -41,12 +41,14 @@ sub list { > my $c = shift->openapi->valid_input or return; > > return try { >- my $illbatches = $c->objects->search( Koha::Illbatches->new ); >- return $c->render( status => 200, openapi => $illbatches ); >+ return $c->render( >+ status => 200, >+ openapi => $c->objects->search( Koha::Illbatches->new ) >+ ); > } catch { >+ warn "$_"; > $c->unhandled_exception($_); > }; >- > } > > =head3 get >@@ -59,7 +61,7 @@ sub get { > my $c = shift->openapi->valid_input or return; > > return try { >- my $ill_batch = $c->objects->find( Koha::Illbatches->search, $c->param('ill_batch_id') ); >+ my $ill_batch = $c->objects->find( Koha::Illbatches->new, $c->param('ill_batch_id') ); > > unless ($ill_batch) { > return $c->render( >@@ -98,8 +100,7 @@ sub add { > } > > delete $body->{cardnumber}; >- $body->{borrowernumber} = $patron->borrowernumber; >- $body->{branchcode} = delete $body->{library_id}; >+ $body->{patron_id} = $patron->id; > > return try { > my $batch = Koha::Illbatch->new_from_api($body); >@@ -107,7 +108,7 @@ sub add { > > $c->res->headers->location( $c->req->url->to_string . '/' . $batch->id ); > >- my $ill_batch = $c->objects->find( Koha::Illbatches->search, $batch->id ); >+ my $ill_batch = $c->objects->find( Koha::Illbatches->new, $batch->id ); > > return $c->render( > status => 201, >@@ -146,15 +147,13 @@ 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); > > return $c->render( > status => 200, >- openapi => $batch->to_api >+ openapi => $c->objects->to_api($batch), > ); > } catch { > $c->unhandled_exception($_); >diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm >index 075cef0c7f..ff0c1ca9b1 100644 >--- a/Koha/Schema/Result/Borrower.pm >+++ b/Koha/Schema/Result/Borrower.pm >@@ -1413,7 +1413,7 @@ Related object: L<Koha::Schema::Result::Illbatch> > __PACKAGE__->has_many( > "illbatches", > "Koha::Schema::Result::Illbatch", >- { "foreign.borrowernumber" => "self.borrowernumber" }, >+ { "foreign.patron_id" => "self.borrowernumber" }, > { cascade_copy => 0, cascade_delete => 0 }, > ); > >@@ -2118,8 +2118,8 @@ Composing rels: L</user_permissions> -> permission > __PACKAGE__->many_to_many("permissions", "user_permissions", "permission"); > > >-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-04-28 11:24:21 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:RUWvcq9kgQvACo14H/u9jQ >+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-10 14:16:46 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:nIMnqyBVBam7NvIx4aDfHw > > __PACKAGE__->has_many( > "restrictions", >diff --git a/Koha/Schema/Result/Branch.pm b/Koha/Schema/Result/Branch.pm >index 7d0cbaffaf..31039bda60 100644 >--- a/Koha/Schema/Result/Branch.pm >+++ b/Koha/Schema/Result/Branch.pm >@@ -703,7 +703,7 @@ Related object: L<Koha::Schema::Result::Illbatch> > __PACKAGE__->has_many( > "illbatches", > "Koha::Schema::Result::Illbatch", >- { "foreign.branchcode" => "self.branchcode" }, >+ { "foreign.library_id" => "self.branchcode" }, > { cascade_copy => 0, cascade_delete => 0 }, > ); > >@@ -933,8 +933,8 @@ __PACKAGE__->has_many( > ); > > >-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-04-28 11:24:21 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:GGYhqyTVMmaFo9hAY4Jy5w >+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-10 14:16:46 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:LjdauIP6C1I1/1liCIddJQ > > __PACKAGE__->add_columns( > '+pickup_location' => { is_boolean => 1 }, >diff --git a/Koha/Schema/Result/Illbatch.pm b/Koha/Schema/Result/Illbatch.pm >index 22c402b254..d7e758cae7 100644 >--- a/Koha/Schema/Result/Illbatch.pm >+++ b/Koha/Schema/Result/Illbatch.pm >@@ -23,7 +23,7 @@ __PACKAGE__->table("illbatches"); > > =head1 ACCESSORS > >-=head2 id >+=head2 ill_batch_id > > data_type: 'integer' > is_auto_increment: 1 >@@ -47,7 +47,7 @@ Unique name of batch > > Name of batch backend > >-=head2 borrowernumber >+=head2 patron_id > > data_type: 'integer' > is_foreign_key: 1 >@@ -55,7 +55,7 @@ Name of batch backend > > Patron associated with batch > >-=head2 branchcode >+=head2 library_id > > data_type: 'varchar' > is_foreign_key: 1 >@@ -64,7 +64,7 @@ Patron associated with batch > > Branch associated with batch > >-=head2 statuscode >+=head2 status_code > > data_type: 'varchar' > is_foreign_key: 1 >@@ -76,17 +76,17 @@ Status of batch > =cut > > __PACKAGE__->add_columns( >- "id", >+ "ill_batch_id", > { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, > "name", > { data_type => "varchar", is_nullable => 0, size => 100 }, > "backend", > { data_type => "varchar", is_nullable => 0, size => 20 }, >- "borrowernumber", >+ "patron_id", > { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, >- "branchcode", >+ "library_id", > { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 50 }, >- "statuscode", >+ "status_code", > { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 20 }, > ); > >@@ -94,13 +94,13 @@ __PACKAGE__->add_columns( > > =over 4 > >-=item * L</id> >+=item * L</ill_batch_id> > > =back > > =cut > >-__PACKAGE__->set_primary_key("id"); >+__PACKAGE__->set_primary_key("ill_batch_id"); > > =head1 UNIQUE CONSTRAINTS > >@@ -118,27 +118,22 @@ __PACKAGE__->add_unique_constraint("u_illbatches__name", ["name"]); > > =head1 RELATIONS > >-=head2 borrowernumber >+=head2 illrequests > >-Type: belongs_to >+Type: has_many > >-Related object: L<Koha::Schema::Result::Borrower> >+Related object: L<Koha::Schema::Result::Illrequest> > > =cut > >-__PACKAGE__->belongs_to( >- "borrowernumber", >- "Koha::Schema::Result::Borrower", >- { borrowernumber => "borrowernumber" }, >- { >- is_deferrable => 1, >- join_type => "LEFT", >- on_delete => "SET NULL", >- on_update => "CASCADE", >- }, >+__PACKAGE__->has_many( >+ "illrequests", >+ "Koha::Schema::Result::Illrequest", >+ { "foreign.batch_id" => "self.ill_batch_id" }, >+ { cascade_copy => 0, cascade_delete => 0 }, > ); > >-=head2 branchcode >+=head2 library > > Type: belongs_to > >@@ -147,9 +142,9 @@ Related object: L<Koha::Schema::Result::Branch> > =cut > > __PACKAGE__->belongs_to( >- "branchcode", >+ "library", > "Koha::Schema::Result::Branch", >- { branchcode => "branchcode" }, >+ { branchcode => "library_id" }, > { > is_deferrable => 1, > join_type => "LEFT", >@@ -158,22 +153,27 @@ __PACKAGE__->belongs_to( > }, > ); > >-=head2 illrequests >+=head2 patron > >-Type: has_many >+Type: belongs_to > >-Related object: L<Koha::Schema::Result::Illrequest> >+Related object: L<Koha::Schema::Result::Borrower> > > =cut > >-__PACKAGE__->has_many( >- "illrequests", >- "Koha::Schema::Result::Illrequest", >- { "foreign.batch_id" => "self.id" }, >- { cascade_copy => 0, cascade_delete => 0 }, >+__PACKAGE__->belongs_to( >+ "patron", >+ "Koha::Schema::Result::Borrower", >+ { borrowernumber => "patron_id" }, >+ { >+ is_deferrable => 1, >+ join_type => "LEFT", >+ on_delete => "SET NULL", >+ on_update => "CASCADE", >+ }, > ); > >-=head2 statuscode >+=head2 status_code > > Type: belongs_to > >@@ -182,9 +182,9 @@ Related object: L<Koha::Schema::Result::IllbatchStatus> > =cut > > __PACKAGE__->belongs_to( >- "statuscode", >+ "status_code", > "Koha::Schema::Result::IllbatchStatus", >- { code => "statuscode" }, >+ { code => "status_code" }, > { > is_deferrable => 1, > join_type => "LEFT", >@@ -194,8 +194,15 @@ __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 >+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-10 18:12:30 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:81afdstzxyW2hhIx6mu4KA >+ >+__PACKAGE__->has_many( >+ "requests", >+ "Koha::Schema::Result::Illrequest", >+ { "foreign.batch_id" => "self.ill_batch_id" }, >+ { cascade_copy => 0, cascade_delete => 0 }, >+); > > sub koha_object_class { > 'Koha::Illbatch'; >diff --git a/Koha/Schema/Result/IllbatchStatus.pm b/Koha/Schema/Result/IllbatchStatus.pm >index 64b6fd867e..f0b9e983ca 100644 >--- a/Koha/Schema/Result/IllbatchStatus.pm >+++ b/Koha/Schema/Result/IllbatchStatus.pm >@@ -106,13 +106,13 @@ Related object: L<Koha::Schema::Result::Illbatch> > __PACKAGE__->has_many( > "illbatches", > "Koha::Schema::Result::Illbatch", >- { "foreign.statuscode" => "self.code" }, >+ { "foreign.status_code" => "self.code" }, > { cascade_copy => 0, cascade_delete => 0 }, > ); > > >-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-09-08 13:49:29 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:yo60FJ+kyRj8QuEMac8CFA >+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-10 18:12:30 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:sRgblQWtTH/cdtdMT3KP+w > > __PACKAGE__->add_columns( > '+is_system' => { is_boolean => 1 }, >diff --git a/Koha/Schema/Result/Illrequest.pm b/Koha/Schema/Result/Illrequest.pm >index d7c646b975..e1c29012bc 100644 >--- a/Koha/Schema/Result/Illrequest.pm >+++ b/Koha/Schema/Result/Illrequest.pm >@@ -276,7 +276,7 @@ Related object: L<Koha::Schema::Result::Illbatch> > __PACKAGE__->belongs_to( > "batch", > "Koha::Schema::Result::Illbatch", >- { id => "batch_id" }, >+ { ill_batch_id => "batch_id" }, > { > is_deferrable => 1, > join_type => "LEFT", >@@ -391,8 +391,8 @@ __PACKAGE__->belongs_to( > ); > > >-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-09-08 13:51:09 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:QS3E1jO/d797B0ADcjT0yQ >+# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-10 14:49:40 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:DQl+7uwLCz5GeqU2hBFmMA > > __PACKAGE__->has_many( > "comments", >diff --git a/api/v1/swagger/definitions/ill_batch.yaml b/api/v1/swagger/definitions/ill_batch.yaml >index eadbe244c5..a142e88a32 100644 >--- a/api/v1/swagger/definitions/ill_batch.yaml >+++ b/api/v1/swagger/definitions/ill_batch.yaml >@@ -1,8 +1,8 @@ > --- > type: object > properties: >- batch_id: >- type: string >+ ill_batch_id: >+ type: integer > description: Internal ILL batch identifier > name: > type: string >@@ -12,34 +12,41 @@ properties: > description: Backend name > cardnumber: > type: string >- description: Card number of the patron of the ILL batch >+ description: Library assigned user identifier of the ILL batch > patron_id: > type: string >- description: Borrower number of the patron of the ILL batch >+ description: Internal identifier the patron of the ILL batch > library_id: > type: string >- description: Branch code of the branch of the ILL batch >+ description: Internal identifier for the ILL batch's library >+ status_code: >+ type: string >+ description: Code of the status of the ILL batch > patron: > type: > - object > - "null" > description: The patron associated with the batch >- branch: >+ library: > type: > - object > - "null" >- description: The branch associated with the batch >- statuscode: >- type: string >- description: Code of the status of the ILL batch >+ description: The library associated with the batch >+ requests: >+ type: >+ - array >+ - "null" >+ description: The requests in this batch (x-koha-embed) >+ requests_count: >+ type: >+ - integer >+ - "null" >+ description: The number of requests in this batch (x-koha-embed) > status: > type: > - object > - "null" >- description: The status associated with the batch >- requests_count: >- type: string >- description: The number of requests in this batch >+ description: The status associated with the batch (x-koha-embed) > _strings: > type: > - object >@@ -50,4 +57,4 @@ required: > - name > - backend > - library_id >- - statuscode >\ No newline at end of file >+ - status_code >diff --git a/api/v1/swagger/paths/ill_batches.yaml b/api/v1/swagger/paths/ill_batches.yaml >index dd3173cdd6..3d747be2a5 100644 >--- a/api/v1/swagger/paths/ill_batches.yaml >+++ b/api/v1/swagger/paths/ill_batches.yaml >@@ -23,9 +23,10 @@ > type: string > enum: > - +strings >+ - requests > - requests+count > - patron >- - branch >+ - library > collectionFormat: csv > produces: > - application/json >@@ -82,9 +83,10 @@ > type: string > enum: > - +strings >+ - requests > - requests+count > - patron >- - branch >+ - library > collectionFormat: csv > produces: > - application/json >@@ -156,9 +158,10 @@ > type: string > enum: > - +strings >+ - requests > - requests+count > - patron >- - branch >+ - library > collectionFormat: csv > produces: > - application/json >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 0ee9aa3e53..caaacd3fe6 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -3319,17 +3319,17 @@ CREATE TABLE `illbatch_statuses` ( > -- > DROP TABLE IF EXISTS `illbatches`; > CREATE TABLE `illbatches` ( >- `id` int(11) NOT NULL auto_increment COMMENT "Batch ID", >+ `ill_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`), >+ `patron_id` int(11) NULL DEFAULT NULL COMMENT "Patron associated with batch", >+ `library_id` varchar(50) NULL DEFAULT NULL COMMENT "Branch associated with batch", >+ `status_code` varchar(20) NULL DEFAULT NULL COMMENT "Status of batch", >+ PRIMARY KEY (`ill_batch_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 >+ CONSTRAINT `illbatches_bnfk` FOREIGN KEY (`patron_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, >+ CONSTRAINT `illbatches_bcfk` FOREIGN KEY (`library_id`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE, >+ CONSTRAINT `illbatches_sfk` FOREIGN KEY (`status_code`) REFERENCES `illbatch_statuses` (`code`) ON DELETE SET NULL ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > > -- >@@ -3370,7 +3370,7 @@ CREATE TABLE `illrequests` ( > CONSTRAINT `illrequests_bibfk` FOREIGN KEY (`biblio_id`) REFERENCES `biblio` (`biblionumber`) ON DELETE SET NULL ON UPDATE CASCADE, > CONSTRAINT `illrequests_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, > CONSTRAINT `illrequests_safk` FOREIGN KEY (`status_alias`) REFERENCES `authorised_values` (`authorised_value`) ON DELETE SET NULL ON UPDATE CASCADE, >- CONSTRAINT `illrequests_ibfk` FOREIGN KEY (`batch_id`) REFERENCES `illbatches` (`id`) ON DELETE SET NULL ON UPDATE CASCADE >+ CONSTRAINT `illrequests_ibfk` FOREIGN KEY (`batch_id`) REFERENCES `illbatches` (`ill_batch_id`) ON DELETE SET NULL ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > /*!40101 SET character_set_client = @saved_cs_client */; > >diff --git a/t/db_dependent/IllbatchStatuses.t b/t/db_dependent/IllbatchStatuses.t >index 45fd7b1a03..c03e81a426 100755 >--- a/t/db_dependent/IllbatchStatuses.t >+++ b/t/db_dependent/IllbatchStatuses.t >@@ -193,11 +193,11 @@ my $status5 = Koha::IllbatchStatus->new( > $status5->create_and_log; > my $batch = Koha::Illbatch->new( > { >- name => "My test batch", >- borrowernumber => $patron->borrowernumber, >- branchcode => $library->branchcode, >- backend => "TEST", >- statuscode => $status5->code >+ name => "My test batch", >+ patron_id => $patron->borrowernumber, >+ library_id => $library->branchcode, >+ backend => "TEST", >+ statuscode => $status5->code > } > ); > $batch->create_and_log; >diff --git a/t/db_dependent/Illbatches.t b/t/db_dependent/Illbatches.t >index 4f2b215bca..18f79bab9b 100755 >--- a/t/db_dependent/Illbatches.t >+++ b/t/db_dependent/Illbatches.t >@@ -17,7 +17,6 @@ > > use Modern::Perl; > >-use File::Basename qw/basename/; > use Koha::Database; > use Koha::Illbatch; > use Koha::Illbatches; >@@ -28,7 +27,7 @@ use t::lib::TestBuilder; > use Test::MockObject; > use Test::MockModule; > >-use Test::More tests => 8; >+use Test::More tests => 7; > > my $schema = Koha::Database->new->schema; > my $builder = t::lib::TestBuilder->new; >@@ -54,19 +53,17 @@ my $librarian = $builder->build( > my $branch = $builder->build( { source => 'Branch' } ); > > # Create a batch >-my $illbatch = $builder->build( >+my $illbatch = $builder->build_object( > { >- source => 'Illbatch', >+ class => 'Koha::Illbatches', > value => { >- name => "My test batch", >- backend => "Mock", >- borrowernumber => $librarian->{borrowernumber}, >- branchcode => $branch->{branchcode} >+ name => "My test batch", >+ backend => "Mock", >+ patron_id => $librarian->{borrowernumber}, >+ library_id => $branch->{branchcode} > } > } > ); >-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( >@@ -74,23 +71,23 @@ my $illrq = $builder->build( > source => 'Illrequest', > value => { > borrowernumber => $patron->{borrowernumber}, >- batch_id => $illbatch->{id} >+ batch_id => $illbatch->id > } > } > ); > my $illrq_obj = Koha::Illrequests->find( $illrq->{illrequest_id} ); > > # Check requests_count >-my $requests_count = $batch_obj->requests_count; >+my $requests_count = $illbatch->requests_count; > is( $requests_count, 1, 'requests_count returns correctly' ); > > # Check patron >-my $batch_patron = $batch_obj->patron; >+my $batch_patron = $illbatch->patron; > isa_ok( $batch_patron, 'Koha::Patron' ); > is( $batch_patron->firstname, "Grogu", "patron returns correctly" ); > > # Check branch >-my $batch_branch = $batch_obj->branch; >+my $batch_branch = $illbatch->library; > isa_ok( $batch_branch, 'Koha::Library' ); > is( $batch_branch->branchcode, $branch->{branchcode}, "branch returns correctly" ); > >diff --git a/t/db_dependent/api/v1/ill_batches.t b/t/db_dependent/api/v1/ill_batches.t >index 3b2b5c57fa..aae636d829 100755 >--- a/t/db_dependent/api/v1/ill_batches.t >+++ b/t/db_dependent/api/v1/ill_batches.t >@@ -23,6 +23,8 @@ use Test::Mojo; > use t::lib::TestBuilder; > use t::lib::Mocks; > >+use JSON qw(encode_json); >+ > use Koha::Illbatch; > use Koha::Illbatches; > use Koha::Illrequests; >@@ -37,12 +39,10 @@ t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); > > subtest 'list() tests' => sub { > >- plan tests => 19; >+ plan tests => 21; > > $schema->storage->txn_begin; > >- Koha::Illbatches->search->delete; >- > my $librarian = $builder->build_object( > { > class => 'Koha::Patrons', >@@ -52,55 +52,62 @@ subtest 'list() tests' => sub { > } > ); > >- my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > > my $password = 'sheev_is_da_boss!'; > $librarian->set_password( { password => $password, skip_validation => 1 } ); > my $userid = $librarian->userid; > >+ my $batch_to_delete = $builder->build_object( { class => 'Koha::Illbatches' } ); >+ my $deleted_batch_id = $batch_to_delete->id; >+ $batch_to_delete->delete; >+ >+ my $query = { ill_batch_id => [$deleted_batch_id] }; >+ > ## Authorized user tests > # No batches, so empty array should be returned >- $t->get_ok("//$userid:$password@/api/v1/ill/batches")->status_is(200)->json_is( [] ); >+ $t->get_ok( "//$userid:$password@/api/v1/ill/batches?q=" . encode_json($query) )->status_is(200)->json_is( [] ); > >- my $batch = $builder->build_object( >+ my $batch_1 = $builder->build_object( > { > class => 'Koha::Illbatches', > value => { >- name => "PapaPalpatine", >- backend => "Mock", >- borrowernumber => $librarian->borrowernumber, >- branchcode => $branch->branchcode >+ backend => "Mock", >+ patron_id => $librarian->id, >+ library_id => $library->id, > } > } > ); > >- my $illrq = $builder->build( >+ my $illrq = $builder->build_object( > { >- source => 'Illrequest', >- value => { >- borrowernumber => $librarian->borrowernumber, >- batch_id => $batch->id >+ class => 'Koha::Illrequests', >+ value => { >+ batch_id => $batch_1->id, >+ borrowernumber => $librarian->id, > } > } > ); > >- # One batch created, should get returned >- $t->get_ok( >- "//$userid:$password@/api/v1/ill/batches" => { 'x-koha-embed' => '+strings,requests+count,patron,branch' } ) >- ->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' ); >+ $query = { ill_batch_id => [ $batch_1->id ] }; > >- # 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 } } ); >+ # One batch created, should get returned >+ $t->get_ok( "//$userid:$password@/api/v1/ill/batches?q=" >+ . encode_json($query) => { 'x-koha-embed' => '+strings,requests+count,patron,library' } )->status_is(200) >+ ->json_has( '/0/ill_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/library', 'branch embedded' )->json_has( '/0/requests_count', 'request count' ); > > # Create a second batch with a different name >- my $batch_with_another_name = $builder->build_object( { class => 'Koha::Illbatches' } ); >+ my $batch_2 = $builder->build_object( { class => 'Koha::Illbatches' } ); >+ >+ $query = { ill_batch_id => [ $batch_1->id, $batch_2->id ] }; > > # Two batches created, they should both be returned >- $t->get_ok("//$userid:$password@/api/v1/ill/batches")->status_is(200)->json_has( '/0', 'has first batch' ) >- ->json_has( '/1', 'has second batch' ); >+ $t->get_ok( "//$userid:$password@/api/v1/ill/batches?q=" . encode_json($query) )->status_is(200) >+ ->json_has( '/0', 'has first batch' )->json_is( '/0/ill_batch_id', $batch_1->id ) >+ ->json_has( '/1', 'has second batch' )->json_is( '/1/ill_batch_id', $batch_2->id ); > > my $patron = $builder->build_object( > { >@@ -144,16 +151,15 @@ subtest 'get() tests' => sub { > } > ); > >- my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > > my $batch = $builder->build_object( > { > class => 'Koha::Illbatches', > value => { >- name => "LeiaOrgana", >- backend => "Mock", >- borrowernumber => $librarian->borrowernumber, >- branchcode => $branch->branchcode >+ backend => "Mock", >+ patron_id => $librarian->id, >+ library_id => $library->id, > } > } > ); >@@ -162,11 +168,11 @@ subtest 'get() tests' => sub { > my $unauth_userid = $patron->userid; > > $t->get_ok( "//$userid:$password@/api/v1/ill/batches/" >- . $batch->id => { 'x-koha-embed' => '+strings,requests+count,patron,branch' } )->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' ); >+ . $batch->id => { 'x-koha-embed' => '+strings,requests+count,patron,library' } )->status_is(200) >+ ->json_has( '/ill_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( '/library', 'library embedded' )->json_has( '/requests_count', 'request count' ); > > $t->get_ok( "//$unauth_userid:$password@/api/v1/ill/batches/" . $batch->id )->status_is(403); > >@@ -182,7 +188,7 @@ subtest 'get() tests' => sub { > > subtest 'add() tests' => sub { > >- plan tests => 19; >+ plan tests => 20; > > $schema->storage->txn_begin; > >@@ -206,16 +212,16 @@ 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 $library = $builder->build_object( { class => 'Koha::Libraries' } ); > > my $batch_status = $builder->build_object( { class => 'Koha::IllbatchStatuses' } ); > > my $batch_metadata = { >- name => "Anakin's requests", >- backend => "Mock", >- cardnumber => $librarian->cardnumber, >- library_id => $branch->branchcode, >- statuscode => $batch_status->code >+ name => "Anakin's requests", >+ backend => "Mock", >+ cardnumber => $librarian->cardnumber, >+ library_id => $library->branchcode, >+ status_code => $batch_status->code > }; > > # Unauthorized attempt to write >@@ -239,13 +245,13 @@ subtest 'add() tests' => sub { > > # Authorized attempt to write > my $batch_id = >- $t->post_ok( "//$userid:$password@/api/v1/ill/batches" => >- { 'x-koha-embed' => '+strings,requests+count,patron,branch' } => 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('/_strings/status')->json_has('/requests_count')->json_has('/branch'); >+ $t->post_ok( >+ "//$userid:$password@/api/v1/ill/batches" => { 'x-koha-embed' => '+strings,requests+count,patron,library' } => >+ 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( '/status_code' => $batch_status->code ) >+ ->json_has('/patron')->json_has('/_strings/status_code')->json_has('/_strings/library_id') >+ ->json_has('/requests_count')->json_has('/library'); > > # Authorized attempt to create with null id > $batch_metadata->{id} = undef; >@@ -281,8 +287,7 @@ 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 $library = $builder->build_object( { class => 'Koha::Libraries' } ); > my $batch_id = $builder->build_object( { class => 'Koha::Illbatches' } )->id; > > # Unauthorized attempt to update >@@ -293,10 +298,10 @@ subtest 'update() tests' => sub { > > # Attempt partial update on a PUT > my $batch_with_missing_field = { >- backend => "Mock", >- patron_id => $librarian->borrowernumber, >- library_id => $branch->branchcode, >- statuscode => $batch_status->code >+ backend => "Mock", >+ patron_id => $librarian->borrowernumber, >+ library_id => $library->branchcode, >+ status_code => $batch_status->code > }; > > $t->put_ok( "//$userid:$password@/api/v1/ill/batches/$batch_id" => json => $batch_with_missing_field ) >@@ -304,11 +309,11 @@ subtest 'update() tests' => sub { > > # Full object update on PUT > my $batch_with_updated_field = { >- name => "Master Ploo Koon", >- backend => "Mock", >- patron_id => $librarian->borrowernumber, >- library_id => $branch->branchcode, >- statuscode => $batch_status->code >+ name => "Master Ploo Koon", >+ backend => "Mock", >+ patron_id => $librarian->borrowernumber, >+ library_id => $library->branchcode, >+ status_code => $batch_status->code > }; > > $t->put_ok( "//$userid:$password@/api/v1/ill/batches/$batch_id" => json => $batch_with_updated_field ) >-- >2.30.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=156811">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30719
:
144281
|
144285
|
144286
|
144287
|
144288
|
144289
|
144290
|
144291
|
144292
|
144293
|
144294
|
144295
|
144296
|
144297
|
144298
|
144299
|
144300
|
144301
|
144302
|
145760
|
145869
|
145870
|
145876
|
145934
|
146152
|
147658
|
147929
|
147930
|
147931
|
147932
|
147933
|
147934
|
147952
|
147953
|
147954
|
147955
|
147956
|
151152
|
151153
|
151154
|
151155
|
151156
|
151284
|
151285
|
151286
|
151287
|
151288
|
151303
|
151304
|
151305
|
151306
|
151307
|
151308
|
151309
|
151310
|
151311
|
151312
|
151513
|
151514
|
151515
|
151516
|
151517
|
151519
|
151520
|
151521
|
151522
|
151523
|
151571
|
151572
|
151573
|
151574
|
151575
|
151611
|
151612
|
151613
|
151614
|
151615
|
151804
|
151805
|
151806
|
151807
|
151808
|
151809
|
154046
|
154047
|
154048
|
154049
|
154050
|
154051
|
154052
|
155421
|
155422
|
155423
|
155424
|
155425
|
155426
|
155427
|
155428
|
155429
|
155430
|
155431
|
155432
|
155433
|
155434
|
155435
|
156556
|
156748
|
156749
|
156752
|
156783
|
156785
| 156811 |
156812
|
156827
|
156828