See omnibus bug for more details description.
Created attachment 87751 [details] [review] Bug 22683: Allow prefetch with Koha::Account::Line->checkouts This patch adds the required DBIC side relationships for the Koha::Account::Line checkouts relationship accessor to support prefetching and also updates the method to enable the capability. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment on attachment 87751 [details] [review] Bug 22683: Allow prefetch with Koha::Account::Line->checkouts >+__PACKAGE__->belongs_to( >+ "issue", >+ "Koha::Schema::Result::Issue", >+ { "foriegn.issue_id" => "self.issue_id" }, "foriegn"
Comment on attachment 87751 [details] [review] Bug 22683: Allow prefetch with Koha::Account::Line->checkouts Review of attachment 87751 [details] [review]: ----------------------------------------------------------------- ::: Koha/Account/Line.pm @@ +68,4 @@ > > + my $old_issue_rs = $self->_result->old_issue; > + return Koha::Old::Checkout->_new_from_dbic( $old_issue_rs ) if $old_issue_rs; > + return undef; should be only 'return;' I think...
(In reply to Tomás Cohen Arazi from comment #2) > Comment on attachment 87751 [details] [review] [review] > Bug 22683: Allow prefetch with Koha::Account::Line->checkouts > > >+__PACKAGE__->belongs_to( > >+ "issue", > >+ "Koha::Schema::Result::Issue", > >+ { "foriegn.issue_id" => "self.issue_id" }, > > "foriegn" oops, good catch
(In reply to Josef Moravec from comment #3) > Comment on attachment 87751 [details] [review] [review] > Bug 22683: Allow prefetch with Koha::Account::Line->checkouts > > Review of attachment 87751 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/Account/Line.pm > @@ +68,4 @@ > > > > + my $old_issue_rs = $self->_result->old_issue; > > + return Koha::Old::Checkout->_new_from_dbic( $old_issue_rs ) if $old_issue_rs; > > + return undef; > > should be only 'return;' I think... It should amount to the same thing.. but I think you're right that our linter would complain about returning explicit undef.
Created attachment 88061 [details] [review] Bug 22683: Allow prefetch with Koha::Account::Line->checkouts This patch adds the required DBIC side relationships for the Koha::Account::Line checkouts relationship accessor to support prefetching and also updates the method to enable the capability. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88062 [details] [review] Bug 22683: (follow-up) Rename DBIC relationship accessor Rename the DBIC side accessors to match the Koha::Object side one to make the 'prefetch' simpler to guess. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 88147 [details] [review] Bug 22683: (follow-up) Rename issue to checkout when call the relation
There are some failing tests: t/db_dependent/Accounts.t .............. 24/34 Value not allowed for auto_incr issue_id in Issue at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 372. # Failed test 'Accountline created' # at t/db_dependent/Accounts.t line 404. # got: '0' # expected: '1' # Looks like you planned 8 tests but ran 1. # Looks like you failed 1 test of 1 run. t/db_dependent/Accounts.t .............. 26/34 # Failed test 'More Koha::Account::pay tests' # at t/db_dependent/Accounts.t line 430. Can't call method "amountoutstanding" on an undefined value at /home/vagrant/kohaclone/Koha/Account.pm line 101. # Looks like your test exited with 255 just after 26. t/db_dependent/Accounts.t .............. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 9/34 subtests t/db_dependent/Koha/Account/Offsets.t .. Value not allowed for auto_incr issue_id in Issue at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 372. # No tests run! t/db_dependent/Koha/Account/Offsets.t .. 1/1 # Failed test 'No tests run for subtest "total_outstanding() tests"' # at t/db_dependent/Koha/Account/Offsets.t line 70. Can't call method "id" on an undefined value at t/db_dependent/Koha/Account/Offsets.t line 46. # Looks like your test exited with 255 just after 1. t/db_dependent/Koha/Account/Offsets.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 1/1 subtests t/db_dependent/Members.t ............... 35/47 Value not allowed for auto_incr issue_id in Issue at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 372. # Failed test 'DeleteExpiredOpacRegistrations doesn't delete borrower with fine and no checkout' # at t/db_dependent/Members.t line 382. # got: '1' # expected: '0' # Looks like you planned 5 tests but ran 4. # Looks like you failed 1 test of 4 run. t/db_dependent/Members.t ............... 46/47 # Failed test 'purgeSelfRegistration' # at t/db_dependent/Members.t line 387. Can't call method "delete" on an undefined value at t/db_dependent/Members.t line 384. # Looks like your test exited with 255 just after 46. t/db_dependent/Members.t ............... Dubious, test returned 255 (wstat 65280, 0xff00) Failed 2/47 subtests Test Summary Report ------------------- t/db_dependent/Accounts.t (Wstat: 65280 Tests: 26 Failed: 1) Failed test: 26 Non-zero exit status: 255 Parse errors: Bad plan. You planned 34 tests but ran 26. t/db_dependent/Koha/Account/Offsets.t (Wstat: 65280 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 255 t/db_dependent/Members.t (Wstat: 65280 Tests: 46 Failed: 1) Failed test: 46 Non-zero exit status: 255 Parse errors: Bad plan. You planned 47 tests but ran 46.
You are adding 2 FK on the same column? I do not think this is possible. Moreover you add a note for the 2 constraints: +Note: This DBIC only relationship should be removed (as it will get automatically recreated) +if we add a proper foreign key for accountlines.issue_id But we will not be able to create this constraint at DB level.