From 06c42b961f4c54b3538307111b5a9842feb94e97 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 22 Jul 2021 12:14:11 -0300 Subject: [PATCH] Bug 25260: (QA follow-up) Rename id => hold_id The guidelines clearly specify this is how the primary key needs to be named. https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL7:_Primary_keys Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 60 +++++++++--------- C4/Letters.pm | 2 +- C4/Reserves.pm | 61 ++++++++++--------- Koha/Hold.pm | 13 ---- Koha/REST/V1/Holds.pm | 10 +-- Koha/Schema/Result/ClubHoldsToPatronHold.pm | 6 +- Koha/Schema/Result/Hold.pm | 14 ++--- .../atomicupdate/bug_25260_holds_table.perl | 16 ++--- installer/data/mysql/kohastructure.sql | 10 +-- t/db_dependent/Circulation.t | 4 +- t/db_dependent/Hold.t | 4 +- t/db_dependent/Items/MoveItemFromBiblio.t | 6 +- t/db_dependent/Koha/Holds.t | 2 +- t/db_dependent/Koha/Patrons.t | 2 +- t/db_dependent/Letters/TemplateToolkit.t | 4 +- t/db_dependent/Reserves.t | 22 +++---- 16 files changed, 112 insertions(+), 124 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 08fd28aba3..98d1f1408e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1104,47 +1104,47 @@ sub CanBookBeIssued { { # The item is on reserve and waiting, but has been # reserved by some other patron. - $needsconfirmation{RESERVE_WAITING} = 1; - $needsconfirmation{'resfirstname'} = $patron->firstname; - $needsconfirmation{'ressurname'} = $patron->surname; - $needsconfirmation{'rescardnumber'} = $patron->cardnumber; + $needsconfirmation{RESERVE_WAITING} = 1; + $needsconfirmation{'resfirstname'} = $patron->firstname; + $needsconfirmation{'ressurname'} = $patron->surname; + $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id}; - $needsconfirmation{'reswaitingdate'} = $res->{waiting_date}; - $needsconfirmation{'reserve_id'} = $res->{id}; + $needsconfirmation{'resbranchcode'} = $res->{pickup_library_id}; + $needsconfirmation{'reswaitingdate'} = $res->{waiting_date}; + $needsconfirmation{'reserve_id'} = $res->{hold_id}; } elsif ( $restype eq "Reserved" ) { # The item is on reserve for someone else. - $needsconfirmation{RESERVED} = 1; - $needsconfirmation{'resfirstname'} = $patron->firstname; - $needsconfirmation{'ressurname'} = $patron->surname; - $needsconfirmation{'rescardnumber'} = $patron->cardnumber; + $needsconfirmation{RESERVED} = 1; + $needsconfirmation{'resfirstname'} = $patron->firstname; + $needsconfirmation{'ressurname'} = $patron->surname; + $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{created_date}; - $needsconfirmation{'reserve_id'} = $res->{id}; + $needsconfirmation{'resbranchcode'} = $patron->branchcode; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{hold_id}; } elsif ( $restype eq "Transferred" ) { # The item is determined hold being transferred for someone else. - $needsconfirmation{TRANSFERRED} = 1; - $needsconfirmation{'resfirstname'} = $patron->firstname; - $needsconfirmation{'ressurname'} = $patron->surname; - $needsconfirmation{'rescardnumber'} = $patron->cardnumber; + $needsconfirmation{TRANSFERRED} = 1; + $needsconfirmation{'resfirstname'} = $patron->firstname; + $needsconfirmation{'ressurname'} = $patron->surname; + $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{created_date}; - $needsconfirmation{'reserve_id'} = $res->{id}; + $needsconfirmation{'resbranchcode'} = $patron->branchcode; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{hold_id}; } elsif ( $restype eq "Processing" ) { # The item is determined hold being processed for someone else. - $needsconfirmation{PROCESSING} = 1; - $needsconfirmation{'resfirstname'} = $patron->firstname; - $needsconfirmation{'ressurname'} = $patron->surname; - $needsconfirmation{'rescardnumber'} = $patron->cardnumber; + $needsconfirmation{PROCESSING} = 1; + $needsconfirmation{'resfirstname'} = $patron->firstname; + $needsconfirmation{'ressurname'} = $patron->surname; + $needsconfirmation{'rescardnumber'} = $patron->cardnumber; $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber; - $needsconfirmation{'resbranchcode'} = $patron->branchcode; - $needsconfirmation{'resreservedate'} = $res->{created_date}; - $needsconfirmation{'reserve_id'} = $res->{id}; + $needsconfirmation{'resbranchcode'} = $patron->branchcode; + $needsconfirmation{'resreservedate'} = $res->{created_date}; + $needsconfirmation{'reserve_id'} = $res->{hold_id}; } } } @@ -2864,10 +2864,6 @@ sub CanBookBeRenewed { } my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber); - use Data::Printer colored => 1; - p( $resfound ); - p( $resrec ); - p( $possible_reserves ); # If next hold is non priority, then check if any hold with priority (non_priority = 0) exists for the same biblionumber. if ( $resfound && $resrec->{non_priority} ) { diff --git a/C4/Letters.pm b/C4/Letters.pm index 55e1f48ef6..06dad858fc 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1669,7 +1669,7 @@ sub _get_tt_params { module => 'Koha::Holds', singular => 'hold', plural => 'holds', - pk => 'id', + pk => 'hold_id', }, serial => { module => 'Koha::Serials', diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 35d28bdd9e..25a5a08d21 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -633,7 +633,7 @@ sub GetOtherReserves { #minus priorities of others reservs ModReserveMinusPriority( $itemnumber, - $checkreserves->{id}, + $checkreserves->{hold_id}, ); #launch the subroutine dotransfer @@ -651,7 +651,7 @@ sub GetOtherReserves { $messages->{'waiting'} = 1; ModReserveMinusPriority( $itemnumber, - $checkreserves->{id}, + $checkreserves->{hold_id}, ); ModReserveStatus($itemnumber,'waiting'); } @@ -1094,7 +1094,7 @@ whose keys are fields from the reserves table in the Koha database. sub ModReserveFill { my ($res) = @_; - my $reserve_id = $res->{'id'}; + my $reserve_id = $res->{hold_id}; my $hold = Koha::Holds->find($reserve_id); # get the priority on this record.... @@ -1210,7 +1210,7 @@ sub ModReserveAffect { $hold->set_processing(); } else { $hold->set_waiting($desk_id); - _koha_notify_reserve( $hold->id ) unless $already_on_shelf; + _koha_notify_reserve( $hold->hold_id ) unless $already_on_shelf; # Complete transfer if one exists my $transfer = $hold->item->get_transfer; $transfer->receive if $transfer; @@ -1282,7 +1282,7 @@ sub ModReserveMinusPriority { my $query = " UPDATE holds SET priority = 0 , item_id = ? - WHERE id = ? + WHERE hold_id = ? "; my $sth_upd = $dbh->prepare($query); $sth_upd->execute( $item_id, $reserve_id ); @@ -1462,7 +1462,7 @@ sub ToggleLowestPriority { my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( "UPDATE holds SET lowest_priority = NOT lowest_priority WHERE id = ?"); + my $sth = $dbh->prepare( "UPDATE holds SET lowest_priority = NOT lowest_priority WHERE hold_id = ?"); $sth->execute( $reserve_id ); _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); @@ -1600,7 +1600,7 @@ sub _FixPriority { my $query = " UPDATE holds SET priority = 0 - WHERE id = ? + WHERE hold_id = ? AND status IN ('waiting', 'in_transit', 'processing') "; my $sth = $dbh->prepare($query); @@ -1610,7 +1610,7 @@ sub _FixPriority { # get whats left my $query = " - SELECT id + SELECT hold_id FROM holds WHERE biblio_id = ? AND completed = 0 @@ -1628,7 +1628,7 @@ sub _FixPriority { my $i; my $key = -1; # to allow for 0 to be a valid result for ( $i = 0 ; $i < @priority ; $i++ ) { - if ( $reserve_id && $reserve_id == $priority[$i]->{'id'} ) { + if ( $reserve_id && $reserve_id == $priority[$i]->{hold_id} ) { $key = $i; # save the index last; } @@ -1646,23 +1646,23 @@ sub _FixPriority { $query = " UPDATE holds SET priority = ? - WHERE id = ? + WHERE hold_id = ? "; $sth = $dbh->prepare($query); for ( my $j = 0 ; $j < @priority ; $j++ ) { $sth->execute( $j + 1, - $priority[$j]->{'id'} + $priority[$j]->{hold_id} ); } - $sth = $dbh->prepare( "SELECT id FROM holds WHERE lowest_priority = 1 ORDER BY priority" ); + $sth = $dbh->prepare( "SELECT hold_id FROM holds WHERE lowest_priority = 1 ORDER BY priority" ); $sth->execute(); unless ( $ignoreSetLowestRank ) { while ( my $res = $sth->fetchrow_hashref() ) { _FixPriority({ - reserve_id => $res->{'id'}, + reserve_id => $res->{hold_id}, rank => '999999', ignoreSetLowestRank => 1 }); @@ -1710,12 +1710,12 @@ sub _Findgroupreserve { holds.timestamp AS timestamp, biblioitems.biblioitemnumber AS biblioitemnumber, holds.item_id AS item_id, - holds.id AS id, + holds.hold_id AS hold_id, holds.item_type AS item_type, holds.non_priority AS non_priority FROM holds JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber) - JOIN hold_fill_targets ON (holds.id=hold_fill_targets.reserve_id) + JOIN hold_fill_targets ON (holds.hold_id=hold_fill_targets.reserve_id) WHERE status='placed' AND priority > 0 AND completed = 0 @@ -1740,19 +1740,19 @@ sub _Findgroupreserve { holds.patron_id AS patron_id, holds.created_date AS created_date, holds.pickup_library_id AS pickup_library_id, - holds.completed_date AS completed_date, + holds.completed_date AS completed_date, holds.status AS status, holds.notes AS notes, holds.priority AS priority, holds.timestamp AS timestamp, biblioitems.biblioitemnumber AS biblioitemnumber, holds.item_id AS item_id, - holds.id AS id, + holds.hold_id AS hold_id, holds.item_type AS item_type, holds.non_priority AS non_priority FROM holds JOIN biblioitems ON (holds.biblio_id=biblioitems.biblionumber) - JOIN hold_fill_targets ON (holds.id=hold_fill_targets.reserve_id) + JOIN hold_fill_targets ON (holds.hold_id=hold_fill_targets.reserve_id) WHERE status='placed' AND priority > 0 AND completed = 0 @@ -1783,7 +1783,7 @@ sub _Findgroupreserve { holds.priority AS priority, holds.timestamp AS timestamp, holds.item_id AS item_id, - holds.id AS id, + holds.hold_id AS hold_id, holds.item_type AS item_type, holds.non_priority AS non_priority FROM holds @@ -1996,7 +1996,7 @@ sub MoveReserve { RevertWaitingStatus({ itemnumber => $itemnumber }); } elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item - my $hold = Koha::Holds->find( $res->{id} ); + my $hold = Koha::Holds->find( $res->{hold_id} ); $hold->cancel; } } @@ -2026,18 +2026,23 @@ sub MergeHolds { # Reorder by date # don't reorder those already waiting - $sth = $dbh->prepare( - "SELECT id FROM holds WHERE biblio_id = ? AND status = 'placed' AND completed = 0 ORDER BY created_date ASC" - ); - my $upd_sth = $dbh->prepare( - "UPDATE holds SET priority = ? WHERE id = ? " - ); + $sth = $dbh->prepare(q{ + SELECT hold_id + FROM holds + WHERE biblio_id = ? AND status = 'placed' AND completed = 0 + ORDER BY created_date ASC + }); + my $upd_sth = $dbh->prepare(q{ + UPDATE holds + SET priority = ? + WHERE hold_id = ? + }); $sth->execute( $to_biblio ); my $priority = 1; while ( my $reserve = $sth->fetchrow_hashref() ) { $upd_sth->execute( $priority, - $reserve->{'id'} + $reserve->{hold_id} ); $priority++; } @@ -2068,7 +2073,7 @@ sub RevertWaitingStatus { my $hold = Koha::Holds->search( { item_id => $item_id, - status => 'waiting', + status => 'waiting', } )->next; diff --git a/Koha/Hold.pm b/Koha/Hold.pm index a03113b938..f2730ddefc 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -632,19 +632,6 @@ sub _set_default_expiration_date { dt_from_string( $self->created_date )->add( $timeunit => $period ) ); } -=head3 to_api_mapping - -This method returns the mapping for representing a Koha::Hold object -on the API. - -=cut - -sub to_api_mapping { - return { - id => 'hold_id' - }; -} - =head2 Internal methods =head3 _type diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 285cb70f73..27de38caab 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -248,7 +248,7 @@ sub edit { return try { my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; + my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( @@ -313,7 +313,7 @@ sub delete { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; + my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( status => 404, openapi => { error => "Hold not found." } ); @@ -342,7 +342,7 @@ sub suspend { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; + my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next; my $body = $c->req->json; my $end_date = ($body) ? $body->{end_date} : undef; @@ -390,7 +390,7 @@ sub resume { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; + my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next; my $body = $c->req->json; unless ($hold) { @@ -416,7 +416,7 @@ sub update_priority { my $c = shift->openapi->valid_input or return; my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->search({id=>$hold_id, completed=>0})->next; + my $hold = Koha::Holds->search({hold_id=>$hold_id, completed=>0})->next; unless ($hold) { return $c->render( diff --git a/Koha/Schema/Result/ClubHoldsToPatronHold.pm b/Koha/Schema/Result/ClubHoldsToPatronHold.pm index 96c5a75254..109507b017 100644 --- a/Koha/Schema/Result/ClubHoldsToPatronHold.pm +++ b/Koha/Schema/Result/ClubHoldsToPatronHold.pm @@ -135,7 +135,7 @@ Related object: L __PACKAGE__->belongs_to( "hold", "Koha::Schema::Result::Hold", - { id => "hold_id" }, + { hold_id => "hold_id" }, { is_deferrable => 1, join_type => "LEFT", @@ -160,8 +160,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-07-12 11:40:08 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:7BEtGibjktT8MUPfLlY9Lw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-07-22 15:29:24 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:DqyCOIzzWIX3X6KBkr72zQ sub koha_objects_class { 'Koha::Club::Hold::PatronHolds'; diff --git a/Koha/Schema/Result/Hold.pm b/Koha/Schema/Result/Hold.pm index f3e0ab6fdf..6bd621e575 100644 --- a/Koha/Schema/Result/Hold.pm +++ b/Koha/Schema/Result/Hold.pm @@ -23,7 +23,7 @@ __PACKAGE__->table("holds"); =head1 ACCESSORS -=head2 id +=head2 hold_id data_type: 'integer' is_auto_increment: 1 @@ -205,7 +205,7 @@ optional authorised value CANCELLATION_REASON =cut __PACKAGE__->add_columns( - "id", + "hold_id", { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, "patron_id", { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, @@ -278,13 +278,13 @@ __PACKAGE__->add_columns( =over 4 -=item * L +=item * L =back =cut -__PACKAGE__->set_primary_key("id"); +__PACKAGE__->set_primary_key("hold_id"); =head1 RELATIONS @@ -319,7 +319,7 @@ Related object: L __PACKAGE__->has_many( "club_holds_to_patron_holds", "Koha::Schema::Result::ClubHoldsToPatronHold", - { "foreign.hold_id" => "self.id" }, + { "foreign.hold_id" => "self.hold_id" }, { cascade_copy => 0, cascade_delete => 0 }, ); @@ -424,8 +424,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-01-27 19:43:42 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:EjBk5LQLG2761uSkk/35Sw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-07-22 15:29:24 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:5iq7d1ACqLGnEBAwWtbaTw __PACKAGE__->add_columns( '+completed' => { is_boolean => 1 }, diff --git a/installer/data/mysql/atomicupdate/bug_25260_holds_table.perl b/installer/data/mysql/atomicupdate/bug_25260_holds_table.perl index d5730b2f8b..9246bc3f92 100644 --- a/installer/data/mysql/atomicupdate/bug_25260_holds_table.perl +++ b/installer/data/mysql/atomicupdate/bug_25260_holds_table.perl @@ -4,7 +4,7 @@ if( CheckVersion( $DBversion ) ) { if( !TableExists( 'holds' ) ) { $dbh->do( q| CREATE TABLE `holds` ( - `id` INT(11) NOT NULL AUTO_INCREMENT COMMENT 'internal hold identifier', + `hold_id` INT(11) NOT NULL AUTO_INCREMENT COMMENT 'internal hold identifier', `patron_id` INT(11) DEFAULT NULL COMMENT 'foreign key from the borrowers table defining which patron this hold is for', `pickup_library_id` VARCHAR(10) DEFAULT NULL COMMENT 'foreign key from the branches table defining which branch the patron wishes to pick this hold up at', `desk_id` INT(11) DEFAULT NULL COMMENT 'foreign key from the desks table defining which desk the patron should pick this hold up at', @@ -26,7 +26,7 @@ if( CheckVersion( $DBversion ) ) { `suspended_until_date` DATETIME NULL DEFAULT NULL COMMENT 'Date until it is suspended', `completed` TINYINT(1) NOT NULL DEFAULT 0 COMMENT "If it has been completed (i.e. either 'fulfilled' or 'cancelled')", `cancellation_reason` VARCHAR(80) NULL DEFAULT NULL COMMENT 'optional authorised value CANCELLATION_REASON', - PRIMARY KEY (`id`), + PRIMARY KEY (`hold_id`), KEY `priority_status_idx` (`priority`, `status`), KEY `patron_id` (`patron_id`), KEY `biblio_id` (`biblio_id`), @@ -49,7 +49,7 @@ if( CheckVersion( $DBversion ) ) { # migrate 'reserves' and 'old_reserves' into 'holds' $dbh->do(q| INSERT INTO holds ( - id, + hold_id, patron_id, created_date, biblio_id, @@ -147,14 +147,14 @@ if( CheckVersion( $DBversion ) ) { $dbh->do(q| ALTER TABLE club_holds_to_patron_holds - ADD CONSTRAINT clubs_holds_paton_holds_ibfk_3 FOREIGN KEY (hold_id) REFERENCES holds (id) ON DELETE CASCADE ON UPDATE CASCADE + ADD CONSTRAINT clubs_holds_paton_holds_ibfk_3 FOREIGN KEY (hold_id) REFERENCES holds (hold_id) ON DELETE CASCADE ON UPDATE CASCADE |); } if ( TableExists('holds') && TableExists('reserves') && !table_is_view('reserves') ) { $dbh->do(q| - DELETE FROM res USING `reserves` as res INNER JOIN `holds` as h ON h.id = res.reserve_id + DELETE FROM res USING `reserves` as res INNER JOIN `holds` as h ON h.hold_id = res.reserve_id |); $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM reserves"); @@ -173,7 +173,7 @@ if( CheckVersion( $DBversion ) ) { CREATE VIEW `reserves` AS SELECT - `id` AS `reserve_id`, + `hold_id` AS `reserve_id`, `patron_id` AS `borrowernumber`, `created_date` AS `reservedate`, `biblio_id` AS `biblionumber`, @@ -207,7 +207,7 @@ if( CheckVersion( $DBversion ) ) { if ( TableExists('holds') && TableExists('old_reserves') && !table_is_view('old_reserves') ) { $dbh->do(q| - DELETE FROM res USING `old_reserves` as res INNER JOIN `holds` as h ON h.id = res.reserve_id + DELETE FROM res USING `old_reserves` as res INNER JOIN `holds` as h ON h.hold_id = res.reserve_id |); $sth = $dbh->prepare("SELECT COUNT(*) AS count FROM old_reserves"); @@ -226,7 +226,7 @@ if( CheckVersion( $DBversion ) ) { CREATE VIEW `old_reserves` AS SELECT - `id` AS `reserve_id`, + `hold_id` AS `reserve_id`, `patron_id` AS `borrowernumber`, `created_date` AS `reservedate`, `biblio_id` AS `biblionumber`, diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 9437352430..4dc4b485fd 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1865,7 +1865,7 @@ CREATE TABLE `club_holds_to_patron_holds` ( KEY `clubs_holds_paton_holds_ibfk_3` (`hold_id`), CONSTRAINT `clubs_holds_paton_holds_ibfk_1` FOREIGN KEY (`club_hold_id`) REFERENCES `club_holds` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `clubs_holds_paton_holds_ibfk_2` FOREIGN KEY (`patron_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `clubs_holds_paton_holds_ibfk_3` FOREIGN KEY (`hold_id`) REFERENCES `holds` (`id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `clubs_holds_paton_holds_ibfk_3` FOREIGN KEY (`hold_id`) REFERENCES `holds` (`hold_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; @@ -2653,7 +2653,7 @@ DROP TABLE IF EXISTS `holds`; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!40101 SET character_set_client = utf8 */; CREATE TABLE `holds` ( - `id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'internal hold identifier', + `hold_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'internal hold identifier', `patron_id` int(11) DEFAULT NULL COMMENT 'foreign key from the borrowers table defining which patron this hold is for', `pickup_library_id` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'foreign key from the branches table defining which branch the patron wishes to pick this hold up at', `desk_id` int(11) DEFAULT NULL COMMENT 'foreign key from the desks table defining which desk the patron should pick this hold up at', @@ -2675,7 +2675,7 @@ CREATE TABLE `holds` ( `suspended_until_date` datetime DEFAULT NULL COMMENT 'Date until it is suspended', `completed` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'If it has been completed (i.e. either ''fulfilled'' or ''cancelled'')', `cancellation_reason` varchar(80) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'optional authorised value CANCELLATION_REASON', - PRIMARY KEY (`id`), + PRIMARY KEY (`hold_id`), KEY `priority_status_idx` (`priority`,`status`), KEY `patron_id` (`patron_id`), KEY `biblio_id` (`biblio_id`), @@ -5368,7 +5368,7 @@ CREATE TABLE `zebraqueue` ( CREATE VIEW `reserves` AS SELECT - `id` AS `reserve_id`, + `hold_id` AS `reserve_id`, `patron_id` AS `borrowernumber`, `created_date` AS `reservedate`, `biblio_id` AS `biblionumber`, @@ -5406,7 +5406,7 @@ AS CREATE VIEW `old_reserves` AS SELECT - `id` AS `reserve_id`, + `hold_id` AS `reserve_id`, `patron_id` AS `borrowernumber`, `created_date` AS `reservedate`, `biblio_id` AS `biblionumber`, diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b690dafefa..32a3e3aa86 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -604,11 +604,11 @@ subtest "CanBookBeRenewed tests" => sub { is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); - my $reserveid = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $reserving_borrowernumber })->next->id; + my $reserveid = Koha::Holds->search({ biblio_id => $biblio->biblionumber, patron_id => $reserving_borrowernumber })->next->hold_id; my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber )->unblessed; AddIssue($reserving_borrower, $item_3->barcode); my $reserve = $dbh->selectrow_hashref( - 'SELECT * FROM holds WHERE id = ? AND completed = 1', + 'SELECT * FROM holds WHERE hold_id = ? AND completed = 1', { Slice => {} }, $reserveid ); diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index cee2a3c372..7703a70207 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -238,7 +238,7 @@ subtest "delete() tests" => sub { my $deleted = $hold->delete; is( ref($deleted), 'Koha::Hold', 'Koha::Hold->delete should return the Koha::Hold object if the hold has been correctly deleted' ); - is( Koha::Holds->search({ id => $hold_id })->count, 0, + is( Koha::Holds->search({ hold_id => $hold_id })->count, 0, "Koha::Hold->delete should have deleted the hold" ); my $number_of_logs = $schema->resultset('ActionLog')->search( @@ -253,7 +253,7 @@ subtest "delete() tests" => sub { $deleted = $hold->delete; is( ref($deleted), 'Koha::Hold', 'Koha::Hold->delete should return a Koha::Hold object if the hold has been correctly deleted' ); - is( Koha::Holds->search({ id => $hold_id })->count, 0, + is( Koha::Holds->search({ hold_id => $hold_id })->count, 0, "Koha::Hold->delete should have deleted the hold" ); $number_of_logs = $schema->resultset('ActionLog')->search( diff --git a/t/db_dependent/Items/MoveItemFromBiblio.t b/t/db_dependent/Items/MoveItemFromBiblio.t index 6e8e449059..707f86ee3e 100755 --- a/t/db_dependent/Items/MoveItemFromBiblio.t +++ b/t/db_dependent/Items/MoveItemFromBiblio.t @@ -73,9 +73,9 @@ is( $get_item2->biblionumber, $to_biblio->biblionumber, 'The item2 should have b my $get_item3 = Koha::Items->find( $item3->itemnumber ); is( $get_item3->biblionumber, $to_biblio->biblionumber, 'The item3 should not have been moved' ); -my $get_bib_level_hold = Koha::Holds->find( $bib_level_hold_not_to_move->{id} ); -my $get_item_level_hold_1 = Koha::Holds->find( $item_level_hold_not_to_move->{id} ); -my $get_item_level_hold_2 = Koha::Holds->find( $item_level_hold_to_move->{id} ); +my $get_bib_level_hold = Koha::Holds->find( $bib_level_hold_not_to_move->{hold_id} ); +my $get_item_level_hold_1 = Koha::Holds->find( $item_level_hold_not_to_move->{hold_id} ); +my $get_item_level_hold_2 = Koha::Holds->find( $item_level_hold_to_move->{hold_id} ); is( $get_bib_level_hold->biblio_id, $from_biblio->biblionumber, 'MoveItemFromBiblio should not have moved the biblio-level hold' ); is( $get_item_level_hold_1->biblio_id, $from_biblio->biblionumber, 'MoveItemFromBiblio should not have moved the item-level hold placed on item 1' ); diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t index 00de77cdd4..a00b1ef2c7 100755 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -440,7 +440,7 @@ subtest 'get_items_that_can_fill' => sub { my $holds = Koha::Holds->search( { - id => [ $reserve_id_1, $reserve_id_2, $waiting_reserve_id, ] + hold_id => [ $reserve_id_1, $reserve_id_2, $waiting_reserve_id, ] } ); diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index f084ceffa3..7c483d5ce1 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -485,7 +485,7 @@ subtest "delete" => sub { is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' ); - is (Koha::Holds->search( { id => $hold->{id}, status => 'cancelled' } )->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| ); + is (Koha::Holds->search( { hold_id => $hold->{hold_id}, status => 'cancelled' } )->count, 1, q|Koha::Patron->delete should have cancelled patron's holds| ); is( Koha::Holds->search( { patron_id => $patron->{borrowernumber}, completed => 0 } )->count, 0, q|Koha::Patron->delete should have cancelled patron's holds 2| ); diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index 0f206f9a55..6c1198c7a0 100755 --- a/t/db_dependent/Letters/TemplateToolkit.t +++ b/t/db_dependent/Letters/TemplateToolkit.t @@ -575,7 +575,7 @@ You have [% count %] items due
  • <>
  • Notes: -

    <>=<>
    +
    <>=<>

    EOF @@ -609,7 +609,7 @@ EOF
  • [% hold.waiting_date | \$KohaDates %]
  • Notes: -

    [% hold.id %]=[% hold.notes %]
    +
    [% hold.hold_id %]=[% hold.notes %]

    EOF diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index d7d666ab3e..97ebc80bcb 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -123,7 +123,7 @@ my ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber, $barcod is($status, "Reserved", "CheckReserves Test 1"); -ok(exists($reserve->{id}), 'CheckReserves() include reserve_id in its response'); +ok(exists($reserve->{hold_id}), 'CheckReserves() include reserve_id in its response'); ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber); is($status, "Reserved", "CheckReserves Test 2"); @@ -238,7 +238,7 @@ $itemnum_fpl = $builder->build_sample_item( # Ensure that priorities are numbered correcly when a hold is moved to waiting # (bug 11947) -$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); +$dbh->do("DELETE FROM holds WHERE biblio_id=?",undef,($bibnum2)); AddReserve( { branchcode => $branch_3, @@ -267,7 +267,7 @@ ModReserveAffect($itemnum_cpl, $requesters{$branch_3}, 0); # Now it should have different priorities. my $biblio = Koha::Biblios->find( $bibnum2 ); -my $holds = $biblio->holds({ completed => 0 }, { order_by => 'id' });; +my $holds = $biblio->holds({ completed => 0 }, { order_by => 'hold_id' });; is($holds->next->priority, 0, 'Item is correctly waiting'); is($holds->next->priority, 1, 'Item is correctly priority 1'); is($holds->next->priority, 2, 'Item is correctly priority 2'); @@ -277,7 +277,7 @@ is( @reserves, 1, 'GetWaiting got only the waiting reserve' ); is( $reserves[0]->patron_id(), $requesters{$branch_3}, 'GetWaiting got the reserve for the correct borrower' ); -$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); +$dbh->do("DELETE FROM holds WHERE biblio_id=?",undef,($bibnum2)); AddReserve( { branchcode => $branch_3, @@ -533,7 +533,7 @@ is( CanReserveBeCanceledFromOpac(), undef, 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' ); is( - CanReserveBeCanceledFromOpac( $canres->{id} ), + CanReserveBeCanceledFromOpac( $canres->{hold_id} ), undef, 'CanReserveBeCanceledFromOpac should return undef if called without the id' ); @@ -543,18 +543,18 @@ is( 'CanReserveBeCanceledFromOpac should return undef if called without borrowernumber' ); -my $cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +my $cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 1, 'Can user cancel its own reserve'); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_2}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_2}); is($cancancel, 0, 'Other user cant cancel reserve'); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 1); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in transfer status cant be canceled'); $dbh->do('DELETE FROM holds', undef, ($bibnum)); -is( CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}), undef, +is( CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}), undef, 'Cannot cancel a deleted hold' ); AddReserve( @@ -568,7 +568,7 @@ AddReserve( (undef, $canres, undef) = CheckReserves($item->itemnumber); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); -$cancancel = CanReserveBeCanceledFromOpac($canres->{id}, $requesters{$branch_1}); +$cancancel = CanReserveBeCanceledFromOpac($canres->{hold_id}, $requesters{$branch_1}); is($cancancel, 0, 'Reserve in waiting status cant be canceled'); # End of tests for bug 12876 @@ -1140,7 +1140,7 @@ subtest 'CheckReserves additional tests' => sub { my ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item->itemnumber ); is( $status, 'Transferred', "We found a reserve" ); - is( $matched_reserve->{id}, $reserve1->id, 'We got the Transit reserve' ); + is( $matched_reserve->{hold_id}, $reserve1->id, 'We got the Transit reserve' ); is( scalar @$possible_reserves, 2, 'We do get both reserves' ); my $patron_B = $builder->build_object({ class => "Koha::Patrons" }); -- 2.32.0