Bugzilla – Attachment 123260 Details for
Bug 25260
Merge 'reserves' and 'old_reserves' into a new 'holds' table
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25260: (QA follow-up) Rename id => hold_id
Bug-25260-QA-follow-up-Rename-id--holdid.patch (text/plain), 36.71 KB, created by
Tomás Cohen Arazi (tcohen)
on 2021-07-28 15:21:44 UTC
(
hide
)
Description:
Bug 25260: (QA follow-up) Rename id => hold_id
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2021-07-28 15:21:44 UTC
Size:
36.71 KB
patch
obsolete
>From 06c42b961f4c54b3538307111b5a9842feb94e97 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <tomascohen@theke.io> >--- > 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<Koha::Schema::Result::Hold> > __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</id> >+=item * L</hold_id> > > =back > > =cut > >-__PACKAGE__->set_primary_key("id"); >+__PACKAGE__->set_primary_key("hold_id"); > > =head1 RELATIONS > >@@ -319,7 +319,7 @@ Related object: L<Koha::Schema::Result::ClubHoldsToPatronHold> > __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 > <li><<holds.waiting_date>></li> > </ul> > <p>Notes: >-<pre><<holds.id>>=<<holds.notes>></pre> >+<pre><<holds.hold_id>>=<<holds.notes>></pre> > </p> > EOF > >@@ -609,7 +609,7 @@ EOF > <li>[% hold.waiting_date | \$KohaDates %]</li> > </ul> > <p>Notes: >-<pre>[% hold.id %]=[% hold.notes %]</pre> >+<pre>[% hold.hold_id %]=[% hold.notes %]</pre> > </p> > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25260
:
103984
|
103985
|
103986
|
103987
|
103988
|
103989
|
103990
|
103991
|
103992
|
103993
|
104053
|
104054
|
104055
|
104056
|
104057
|
117745
|
117746
|
117747
|
117748
|
117749
|
117750
|
117751
|
122761
|
122762
|
122763
|
122764
|
122765
|
122766
|
122767
|
122787
|
122788
|
122789
|
122790
|
122791
|
122792
|
122793
|
122976
|
122977
|
122978
|
122979
|
122980
|
122981
|
122982
|
122983
|
123071
|
123072
|
123073
|
123074
|
123075
|
123076
|
123077
|
123078
|
123079
|
123080
|
123257
|
123258
|
123259
| 123260 |
123261
|
123396