[Bug 24965] New: Koha::Object->to_api should handle undef counts
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Bug ID: 24965 Summary: Koha::Object->to_api should handle undef counts Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org If to_api is requested to embed the count on a relation that might be undef (for example, if the relation is not just a FK on another table but linking table: sub current_item_level_holds { my ($self) = @_; my $items_rs = $self->_result->aqorders_items; my @item_numbers = $items_rs->get_column('itemnumber')->all; return unless @item_numbers; my $biblio = $self->biblio; return unless $biblio; return $biblio->current_holds->search( { itemnumber => { -in => \@item_numbers } } ); } it is possible to get exceptions for calling ->count on an undefined object. We should catch this situation and return zero. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Blocks| |20212 Depends on| |24528 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20212 [Bug 20212] Slowness in Receiving in Acquisitions https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24528 [Bug 24528] Add a syntax for specifying counts on x-koha-embed -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 101560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101560&action=edit Bug 24965: Regression tests To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t => FAIL: Tests fail with "Can't call method "count" on an undefined value..." Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 101561 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101561&action=edit Bug 24965: Make Koha::Object->to_api handle undef counts In some cases where we write a class method that is not directly tied to a DBIC relationship (maybe it should) we are following the following pattern: my $related_object = Koha::RelatedObjects->find($params,$attributes); return unless $related_object; return $related_object->some_iterator_on_other_class; While this is not highly consistent (design-wise, because it would be great if we could return an empty iterator instead of undef) we use this broadly and consistently. The to_api method +count handling is not failsafe to those undef cases. And we should make it return 0 instead of trying to get ->count on undefined objects. This patch makes to_api fallback to 0 when it is requested to embed a count. To test: 1. Apply the regression test patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t => FAIL: Test fails with an error about ->count on undefined variable. 3. Apply this patch 4. Repeat 2 => SUCCESS: The patch adds the simple fallback to 0 on counts 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101560|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 102154 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102154&action=edit Bug 24965: Regression tests To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t => FAIL: Tests fail with "Can't call method "count" on an undefined value..." Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101561|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 102155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102155&action=edit Bug 24965: Make Koha::Object->to_api handle undef counts In some cases where we write a class method that is not directly tied to a DBIC relationship (maybe it should) we are following the following pattern: my $related_object = Koha::RelatedObjects->find($params,$attributes); return unless $related_object; return $related_object->some_iterator_on_other_class; While this is not highly consistent (design-wise, because it would be great if we could return an empty iterator instead of undef) we use this broadly and consistently. The to_api method +count handling is not failsafe to those undef cases. And we should make it return 0 instead of trying to get ->count on undefined objects. This patch makes to_api fallback to 0 when it is requested to embed a count. To test: 1. Apply the regression test patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t => FAIL: Test fails with an error about ->count on undefined variable. 3. Apply this patch 4. Repeat 2 => SUCCESS: The patch adds the simple fallback to 0 on counts 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- What is the problematic situation in current_item_level_holds? no @item_numbers, or no $biblio? The 'return unless @item_numbers' can be removed, you will get an empty iterator in that case. Do you have other examples? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Waiting for an answer. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
Waiting for an answer.
In my opinion it feels dirty. We should have a way to effectively build an 'empty iterator' instead of actually performing a search to the DB, knowing it needs to return 0 results. Damn you make me work extra all the time -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Failed QA |RESOLVED --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** This bug has been marked as a duplicate of bug 25297 *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24965 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|20212 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20212 [Bug 20212] Slowness in Receiving in Acquisitions -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org