Attachment 57203 Details for Bug 17553 – Bug 17553: Koha::Patron->get_overdues

[patch] Bug 17553: Koha::Patron->get_overdues

Bug-17553-KohaPatron-getoverdues.patch (text/plain), 8.43 KB, created by Jonathan Druart on 2016-11-04 14:19:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-11-04 14:19:39 UTC

Size: 8.43 KB

patch

obsolete

>From a7757f22e692147fdc96f2fe4c1926887f2980ad Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 4 Nov 2016 14:04:58 +0000
>Subject: [PATCH] Bug 17553: Koha::Patron->get_overdues
>
>This patch is for discussion:
>As you can see it introduces a weird
>Koha::Object->unblessed_all_relateds subroutine.
>The idea is to fix a lack we will have soon: A lot of subroutines
>retrieves data from several tables and put them into a hashref. It's
>convenient for the use we have here: Send everything to a subroutine to
>generate a letter.
>The problem is that I did not find a good and elegant way to do that
>using DBIX::Class.
>This new method loops through all the "related resultsets" which are the
>prefetched tables.
>---
> C4/Members.pm                 | 21 --------------------
> Koha/Object.pm                | 45 +++++++++++++++++++++++++++++++++++++++++--
> Koha/Patron.pm                | 24 +++++++++++++++++++++++
> members/print_overdues.pl     |  5 +++--
> t/db_dependent/Koha/Patrons.t | 22 ++++++++++++++-------
> 5 files changed, 85 insertions(+), 32 deletions(-)
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index e334c61..6a3370f 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -85,8 +85,6 @@ BEGIN {
> 
>         &IssueSlip
>         GetBorrowersWithEmail
>-
>-        GetOverduesForPatron
>     );
> 
>     #Modify data
>@@ -1658,25 +1656,6 @@ WHERE borrowernumber = 0 AND DATEDIFF( NOW(), timestamp ) > ?|;
>     return $cnt eq '0E0'? 0: $cnt;
> }
> 
>-sub GetOverduesForPatron {
>-    my ( $borrowernumber ) = @_;
>-
>-    my $sql = "
>-        SELECT *
>-        FROM issues, items, biblio, biblioitems
>-        WHERE items.itemnumber=issues.itemnumber
>-          AND biblio.biblionumber   = items.biblionumber
>-          AND biblio.biblionumber   = biblioitems.biblionumber
>-          AND issues.borrowernumber = ?
>-          AND date_due < NOW()
>-    ";
>-
>-    my $sth = C4::Context->dbh->prepare( $sql );
>-    $sth->execute( $borrowernumber );
>-
>-    return $sth->fetchall_arrayref({});
>-}
>-
> END { }    # module clean-up code here (global destructor)
> 
> 1;
>diff --git a/Koha/Object.pm b/Koha/Object.pm
>index b5e11af..e99d1d6 100644
>--- a/Koha/Object.pm
>+++ b/Koha/Object.pm
>@@ -190,6 +190,37 @@ sub unblessed {
>     return { $self->_result->get_columns };
> }
> 
>+=head3 $object->unblessed_all_relateds
>+
>+my $everything_into_one_hashref = $object->unblessed_all_relateds
>+
>+The unblessed method only retrieves column' values for the column of the object.
>+In a *few* cases we want to retrieve the information of all the prefetched data.
>+
>+=cut
>+
>+sub unblessed_all_relateds {
>+    my ($self) = @_;
>+
>+    my %data;
>+    my $related_resultsets = $self->_result->{related_resultsets} || {};
>+    my $rs = $self;
>+    while ( $related_resultsets and %$related_resultsets ) {
>+        my @relations = keys %{ $related_resultsets };
>+        if ( @relations ) {
>+            my $relation = $relations[0];
>+            $rs = $rs->related_resultset($relation)->get_cache;
>+            $rs = $rs->[0]; # Does it makes sense to have several values here?
>+            my $object_class = Koha::Object::_get_object_class( $rs->result_class );
>+            my $koha_object = $object_class->_new_from_dbic( $rs );
>+            $related_resultsets = $rs->{related_resultsets};
>+            %data = ( %data, %{ $koha_object->unblessed } );
>+        }
>+    }
>+    %data = ( %data, %{ $self->unblessed } );
>+    return \%data;
>+}
>+
> =head3 $object->_result();
> 
> Returns the internal DBIC Row object
>@@ -221,6 +252,16 @@ sub _columns {
>     return $self->{_columns};
> }
> 
>+sub _get_object_class {
>+    my ( $type ) = @_;
>+    return unless $type;
>+
>+    if( $type->can('koha_object_class') ) {
>+        return $type->koha_object_class;
>+    }
>+    $type =~ s|Schema::Result::||;
>+    return ${type};
>+}
> 
> =head3 AUTOLOAD
> 
>@@ -246,13 +287,13 @@ sub AUTOLOAD {
>         }
>     }
> 
>-    my @known_methods = qw( is_changed id in_storage get_column );
>+    my @known_methods = qw( is_changed id in_storage get_column related_resultset );
> 
>     Koha::Exceptions::Object::MethodNotCoveredByTests->throw( "The method $method is not covered by tests!" ) unless grep {/^$method$/} @known_methods;
> 
>     my $r = eval { $self->_result->$method(@_) };
>     if ( $@ ) {
>-        Koha::Exceptions::Object::MethodNotFound->throw( "No method $method for " . ref($self) );
>+        Koha::Exceptions::Object::MethodNotFound->throw( "No method $method for " . ref($self) . " " . $@ );
>     }
>     return $r;
> }
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 9a2fd18..ded37a7 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -449,6 +449,30 @@ sub add_enrolment_fee_if_needed {
>     return $enrolment_fee || 0;
> }
> 
>+=head3 get_overdues
>+
>+my $overdue_items = $patron->get_overdues
>+
>+Return the overdued items
>+
>+=cut
>+
>+sub get_overdues {
>+    my ($self) = @_;
>+    # FIXME add date_due < now
>+    my $dtf = Koha::Database->new->schema->storage->datetime_parser;
>+    my $issues = Koha::Issues->search(
>+        {
>+            'me.borrowernumber' => $self->borrowernumber,
>+            'me.date_due' => { '<' => $dtf->format_datetime(dt_from_string) },
>+        },
>+        {
>+            prefetch => { item => { biblio => 'biblioitems' } },
>+        }
>+    );
>+    return $issues;
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/members/print_overdues.pl b/members/print_overdues.pl
>index b63dec9..4d3e607 100755
>--- a/members/print_overdues.pl
>+++ b/members/print_overdues.pl
>@@ -24,9 +24,10 @@ use CGI;
> use C4::Context;
> use C4::Auth;
> use C4::Output;
>-use C4::Members qw(GetOverduesForPatron);
> use C4::Overdues qw(parse_overdues_letter);
> 
>+use Koha::Patrons;
>+
> my $input = new CGI;
> 
> my $flagsrequired = { circulate => "circulate_remaining_permissions" };
>@@ -45,7 +46,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
> my $borrowernumber = $input->param('borrowernumber');
> my $branchcode     = C4::Context->userenv->{'branch'};
> 
>-my $overdues = GetOverduesForPatron($borrowernumber);
>+my $overdues = Koha::Patrons->find($borrowernumber)->get_overdues->unblessed_all_relateds;
> 
> my $letter = parse_overdues_letter(
>     {
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index a40d9c3..f014671 100644
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -516,7 +516,7 @@ subtest 'search_patrons_to_anonymise & anonymise_issue_history' => sub {
> };
> 
> subtest 'get_overdues' => sub {
>-    plan tests => 3;
>+    plan tests => 7;
> 
>     my $library = $builder->build( { source => 'Branch' } );
>     my ($biblionumber_1) = AddBiblio( MARC::Record->new, '' );
>@@ -568,14 +568,22 @@ subtest 'get_overdues' => sub {
>     AddIssue( $patron, $item_2->{barcode}, DateTime->now->subtract( days => 5 ) );
>     AddIssue( $patron, $item_3->{barcode} );
> 
>-    my $overdues = C4::Members::GetOverduesForPatron( $patron->{borrowernumber} );
>-    is( @$overdues, 2, 'GetOverduesForPatron should return the correct number of elements' );
>-    is( $overdues->[0]->{itemnumber}, $item_1->{itemnumber}, 'The issue should be returned in the same order as they have been done, first is correct' );
>-    is( $overdues->[1]->{itemnumber}, $item_2->{itemnumber}, 'The issue should be returned in the same order as they have been done, second is correct' );
>+    $patron = Koha::Patrons->find( $patron->{borrowernumber} );
>+    my $overdues = $patron->get_overdues;
>+    is( $overdues->count, 2, 'Patron should have 2 overdues');
>+    is( $overdues->next->itemnumber, $item_1->{itemnumber}, 'The issue should be returned in the same order as they have been done, first is correct' );
>+    is( $overdues->next->itemnumber, $item_2->{itemnumber}, 'The issue should be returned in the same order as they have been done, second is correct' );
>+
>+    my $o = $overdues->reset->next;
>+    my $unblessed_overdue = $o->unblessed_all_relateds;
>+    is( exists( $unblessed_overdue->{issuedate} ), 1, 'Fields from the issues table should be filled' );
>+    is( exists( $unblessed_overdue->{itemcallnumber} ), 1, 'Fields from the items table should be filled' );
>+    is( exists( $unblessed_overdue->{title} ), 1, 'Fields from the biblio table should be filled' );
>+    is( exists( $unblessed_overdue->{itemtype} ), 1, 'Fields from the biblioitems table should be filled' );
> 
>     # Clean stuffs
>-    Koha::Issues->search( { borrowernumber => $patron->{borrowernumber} } )->delete;
>-    Koha::Patrons->find( $patron->{borrowernumber} )->delete;
>+    Koha::Issues->search( { borrowernumber => $patron->borrowernumber } )->delete;
>+    $patron->delete;
> };
> 
> $retrieved_patron_1->delete;
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17553: 57202 | 57203 | 57312 | 57313 | 70330 | 70331 | 72769 | 72770 | 73554 | 73555 | 73556