Bugzilla – Attachment 37638 Details for
Bug 9809
Get rid of reserveconstraints
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9809: Get rid of reserveconstraints
Bug-9809-Get-rid-of-reserveconstraints.patch (text/plain), 10.76 KB, created by
Jonathan Druart
on 2015-04-10 10:06:02 UTC
(
hide
)
Description:
Bug 9809: Get rid of reserveconstraints
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-04-10 10:06:02 UTC
Size:
10.76 KB
patch
obsolete
>From 85c822c5c75cc48156f2899aeed3238d52bef811 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Fri, 10 Apr 2015 11:31:46 +0200 >Subject: [PATCH] Bug 9809: Get rid of reserveconstraints > >First, to be honest, I am not completely sure this patch won't >introduce regression. >We need several pairs of eyes, and if possible from someone in place for >a long time in the project :) > >Once again this looks vestige of Koha 2, and comments like "FIXME", >"This does not make sense", "probably never reached" appears all around >this code. >Confirmed, reserveconstraints was in > commit d0374d003716dfb40796caad6390a4d69bfb0376 > Author: rangi <rangi> > Date: Tue Dec 19 23:45:51 2000 +0000 > Initial revision > >This patch is not complete, the reserves.constrainttype should be removed >too (to confirm?), and all call to *Reserves using it be updated. > >What I understood: >This feature has never been finished, and has never worked. Developers try >not touch the code not to break something. >The only place something could be inserted in this table is in >C4::Reserves::AddReserve, if the parameter "constraint" starts with an >"o" or "e" (hum...). The only place (I found) where it is possible is in >the quite muddled serials/routing-preview.pl script (if >RoutingListAddReserves is set). This parameter is set to 'o' since the >beginning of the history and it's not clear why. >I suspect a simple wrong c/p. > >There are 2 places where something could be retrieved from this table: >- C4::Reserves::GetReservesFromBiblionumber. I did not find any calls > using something else than the number of reserves or data from the > reserves table. > Looking at the number of FIXME in this area, not sure it has already worked. >- _Findgroupreserve: I let you have a look at the sql query but to me > this cannot return anything if the reserveconstraints table is empty. > >Test plan: >0/ Don't apply this patch >1/ Try to add something in the reserveconstraints table ( :D ) >--- > C4/Reserves.pm | 95 +--------------------- > .../bug_9809-drop_table_reserveconstraints.sql | 1 + > installer/data/mysql/kohastructure.sql | 13 --- > reserve/placerequest.pl | 11 --- > serials/routing-preview.pl | 3 +- > 5 files changed, 3 insertions(+), 120 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_9809-drop_table_reserveconstraints.sql > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index b82387f..942a68c 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -239,21 +239,6 @@ sub AddReserve { > ); > } > } >- >- #} >- ($const eq "o" || $const eq "e") or return; # FIXME: why not have a useful return value? >- $query = qq{ >- INSERT INTO reserveconstraints >- (borrowernumber,biblionumber,reservedate,biblioitemnumber) >- VALUES >- (?,?,?,?) >- }; >- $sth = $dbh->prepare($query); # keep prepare outside the loop! >- foreach (@$bibitems) { >- $sth->execute($borrowernumber, $biblionumber, $resdate, $_); >- } >- >- return $reserve_id; > } > > =head2 GetReserve >@@ -331,50 +316,7 @@ sub GetReservesFromBiblionumber { > push( @params, $itemnumber ); > } > $query .= "ORDER BY priority"; >- my $sth = $dbh->prepare($query); >- $sth->execute( @params ); >- my @results; >- my $i = 0; >- while ( my $data = $sth->fetchrow_hashref ) { >- >- # FIXME - What is this doing? How do constraints work? >- if ($data->{constrainttype} eq 'o') { >- $query = ' >- SELECT biblioitemnumber >- FROM reserveconstraints >- WHERE biblionumber = ? >- AND borrowernumber = ? >- AND reservedate = ? >- '; >- my $csth = $dbh->prepare($query); >- $csth->execute($data->{biblionumber}, $data->{borrowernumber}, $data->{reservedate}); >- my @bibitemno; >- while ( my $bibitemnos = $csth->fetchrow_array ) { >- push( @bibitemno, $bibitemnos ); # FIXME: inefficient: use fetchall_arrayref >- } >- my $count = scalar @bibitemno; >- >- # if we have two or more different specific itemtypes >- # reserved by same person on same day >- my $bdata; >- if ( $count > 1 ) { >- $bdata = GetBiblioItemData( $bibitemno[$i] ); # FIXME: This doesn't make sense. >- $i++; # $i can increase each pass, but the next @bibitemno might be smaller? >- } >- else { >- # Look up the book we just found. >- $bdata = GetBiblioItemData( $bibitemno[0] ); >- } >- # Add the results of this latest search to the current >- # results. >- # FIXME - An 'each' would probably be more efficient. >- foreach my $key ( keys %$bdata ) { >- $data->{$key} = $bdata->{$key}; >- } >- } >- push @results, $data; >- } >- return \@results; >+ return $dbh->selectall_arrayref( $query, { Slice => {} }, @params ); > } > > =head2 GetReservesFromItemnumber >@@ -1970,41 +1912,6 @@ sub _Findgroupreserve { > push( @results, $data ) > unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; > } >- return @results if @results; >- >- my $query = qq{ >- SELECT reserves.biblionumber AS biblionumber, >- reserves.borrowernumber AS borrowernumber, >- reserves.reservedate AS reservedate, >- reserves.waitingdate AS waitingdate, >- reserves.branchcode AS branchcode, >- reserves.cancellationdate AS cancellationdate, >- reserves.found AS found, >- reserves.reservenotes AS reservenotes, >- reserves.priority AS priority, >- reserves.timestamp AS timestamp, >- reserveconstraints.biblioitemnumber AS biblioitemnumber, >- reserves.itemnumber AS itemnumber, >- reserves.reserve_id AS reserve_id >- FROM reserves >- LEFT JOIN reserveconstraints ON reserves.biblionumber = reserveconstraints.biblionumber >- WHERE reserves.biblionumber = ? >- AND ( ( reserveconstraints.biblioitemnumber = ? >- AND reserves.borrowernumber = reserveconstraints.borrowernumber >- AND reserves.reservedate = reserveconstraints.reservedate ) >- OR reserves.constrainttype='a' ) >- AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?) >- AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) >- AND suspend = 0 >- ORDER BY priority >- }; >- $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0); >- @results = (); >- while ( my $data = $sth->fetchrow_hashref ) { >- push( @results, $data ) >- unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; >- } > return @results; > } > >diff --git a/installer/data/mysql/atomicupdate/bug_9809-drop_table_reserveconstraints.sql b/installer/data/mysql/atomicupdate/bug_9809-drop_table_reserveconstraints.sql >new file mode 100644 >index 0000000..e73c88d >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_9809-drop_table_reserveconstraints.sql >@@ -0,0 +1 @@ >+DROP TABLE IF EXISTS reserveconstraints; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 6ddba43..3c61b41 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1829,19 +1829,6 @@ CREATE TABLE reports_dictionary ( -- definitions (or snippets of SQL) stored for > ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; > > -- >--- Table structure for table `reserveconstraints` >--- >- >-DROP TABLE IF EXISTS `reserveconstraints`; >-CREATE TABLE `reserveconstraints` ( >- `borrowernumber` int(11) NOT NULL default 0, >- `reservedate` date default NULL, >- `biblionumber` int(11) NOT NULL default 0, >- `biblioitemnumber` int(11) default NULL, >- `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP >-) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; >- >--- > -- Table structure for table `reserves` > -- > >diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl >index 1499ea5..af66edb 100755 >--- a/reserve/placerequest.pl >+++ b/reserve/placerequest.pl >@@ -38,11 +38,6 @@ my $input = CGI->new(); > checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); > > my @bibitems=$input->param('biblioitem'); >-# FIXME I think reqbib does not exist anymore, it's used in line 82, to AddReserve of contraint type 'o' >-# I bet it's a 2.x feature, reserving a given biblioitem, that is useless in Koha 3.0 >-# we can remove this line, the AddReserve of constrainttype 'o', >-# and probably remove the reserveconstraint table as well, I never could fill anything in this table. >-my @reqbib=$input->param('reqbib'); > my $biblionumber=$input->param('biblionumber'); > my $borrowernumber=$input->param('borrowernumber'); > my $notes=$input->param('notes'); >@@ -115,12 +110,6 @@ if ($type eq 'str8' && $borrower){ > if ($input->param('request') eq 'any'){ > # place a request on 1st available > AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); >- } elsif ($reqbib[0] ne ''){ >- # FIXME : elsif probably never reached, (see top of the script) >- # place a request on a given item >- AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); >- } else { >- AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); > } > } > } >diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl >index db0984c..4e23ed9 100755 >--- a/serials/routing-preview.pl >+++ b/serials/routing-preview.pl >@@ -79,7 +79,6 @@ if($ok){ > $count--; > } > } >- my $const = 'o'; > my $notes; > my $title = $subs->{'bibliotitle'}; > for my $routing ( @routinglist ) { >@@ -95,7 +94,7 @@ if($ok){ > branchcode => $branch > }); > } else { >- AddReserve($branch,$routing->{borrowernumber},$biblio,$const,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); >+ AddReserve($branch,$routing->{borrowernumber},$biblio,undef,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); > } > } > } >-- >2.1.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 9809
:
37638
|
40733
|
40734
|
40735
|
40736
|
40737
|
40738
|
41754
|
41755
|
41756
|
41757
|
41758
|
41759
|
41760
|
41761
|
41767
|
41768
|
41769
|
41770
|
41771
|
41772
|
41773
|
41774
|
41809
|
41810
|
41811
|
41812
|
41813
|
41814
|
41815
|
41816
|
41817
|
41845
|
41848
|
41849
|
41850
|
41851
|
41852
|
41892
|
41893
|
41894
|
41895
|
41896
|
41897
|
41898
|
41899
|
41900
|
41901
|
41902
|
41903