Summary: | Make issue and old_issue prefetchable from Koha::Account::Line | ||
---|---|---|---|
Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
Component: | Architecture, internals, and plumbing | Assignee: | Bugs List <koha-bugs> |
Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | bin.wen, jonathan.druart, josef.moravec, kyle |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 22407 | ||
Attachments: |
Bug 22683: Allow prefetch with Koha::Account::Line->checkouts
Bug 22683: Allow prefetch with Koha::Account::Line->checkouts Bug 22683: (follow-up) Rename DBIC relationship accessor Bug 22683: (follow-up) Rename issue to checkout when call the relation |
Description
Martin Renvoize (ashimema)
2019-04-11 08:31:01 UTC
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. |