Bug 18862 - Allow relations in Koha::Object's AUTOLOAD
Summary: Allow relations in Koha::Object's AUTOLOAD
Status: CLOSED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-27 15:07 UTC by Marcel de Rooy
Modified: 2017-07-06 06:31 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-06-27 15:07:49 UTC
From report 17698:
my @notes = Koha::Checkouts->search({ 'me.note' => { '!=', undef } }, { prefetch => [ 'borrower', { item => 'biblionumber' } ] });

I am having problems on the template side. I can access the item and
biblio information about the issue, but not the borrower information.
[Internal server error triggered by AUTOLOAD in Koha::Object.]

Additionally adding 'branch' will trigger a similar error.

What is different with the item relation ??
Comment 1 Aleisha Amohia 2017-06-28 03:22:29 UTC
I think I have found another place where the DBIx relationships don't work as expected. See Bug 18870. I'm trying to use a prefetch to get data that the DBIx relationship should make easy to access, but for some reason it fails.
I'm thinking this is a big issue that we need to get fixed, because otherwise the Koha Objects are essentially useless to us.
Comment 2 Marcel de Rooy 2017-06-28 10:05:34 UTC
Found it:

Issue.pm contains:

sub item {
    my ( $self ) = @_;
    my $item_rs = $self->_result->item;
    return Koha::Item->_new_from_dbic( $item_rs );
}

and 

sub patron {
    my ( $self ) = @_;
    my $patron_rs = $self->_result->borrower;
    return Koha::Patron->_new_from_dbic( $patron_rs );
}

So we need to use patron instead of borrower.
Instead of adding item, patron, branch etc. subs all over the place, we probably do better in checking these relations in the AUTOLOAD too.
Will try something here.
Comment 3 Marcel de Rooy 2017-06-28 12:15:06 UTC
Well, this would probably work in AUTOLOAD:

    my @relations = $self->_result->relationships;
    if( grep { /^$method$/ } @relations ) {
        return _new_from_dbic( _guess_koha_class($method), $self->_result->$method );
    }

together with something like (maybe add check_install on the module):

sub _guess_koha_class {
    my ( $self, $relation ) = @_;
    my $rel_startcase = uc(substr($relation,0,1)) . substr($relation,1);
    my $class_dbix = "Koha::Schema::Result::" . $rel_startcase;
    my $koha_class;
    if( $class_dbix->can('koha_objects_class') ) {
        $koha_class = $class_dbix->koha_objects_class;
        $koha_class =~ s/s$//;   # singular
        $koha_class =~ s/ie$/y/; # Librarie => Library etc.
    } else {
        $koha_class = "Koha::" . $rel_startcase;
    }
    return $koha_class;
}

But I am afraid that it is not that optimal performance-wise..
Comment 4 Marcel de Rooy 2017-06-28 12:15:41 UTC
Any thoughts before closing this report? Jonathan?
Comment 5 Jonathan Druart 2017-07-05 18:04:24 UTC
What you pasted in comment 2 is the current way to go. I am not in favour of adding more magic for now.
We clearly see/show what we are using and the methods are covered by tests.