[Bug 19943] New: Koha::Biblio - Remove GetBiblioItemData
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Bug ID: 19943 Summary: Koha::Biblio - Remove GetBiblioItemData Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17628 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17628 [Bug 17628] Move bibliographic related code to Koha::Biblio -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 70384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70384&action=edit Bug 19943: Remove itemtype vs itype confusion in CanBookBeIssued Just a preliminary step to clean the code a bit in CanBookBeIssued. The effective item type is already set from GetItem and we do not need to deal with that again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 70385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70385&action=edit Bug 19943: Koha::GetBiblioItemData - Replace existing occurrences The biblioitem's info can be retrieved with Koha::Biblio->biblioitem Test plan: 1. Use the age restriction to restrict checkouts for a given patron 2. Check some items of a biblio out, go to "Items" tab, then "View item's checkout history" link. Compare views with and without patches -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 70386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70386&action=edit Bug 19943: Remove C4::Biblio::GetBiblioItemData It is no longer used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 70385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70385 Bug 19943: Koha::GetBiblioItemData - Replace existing occurrences Review of attachment 70385: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=19943&attachment=70385) ----------------------------------------------------------------- ::: circ/bookcount.pl @@ +39,5 @@
my $biblionumber = $input->param('biblionumber');
my $idata = itemdatanum($itm); +my $biblio = Koha::Biblios->find( $biblionumber ); +die "No valid biblionumber passed" unless $biblio; # FIXME A bit rude!
I believe if you purposefully call bookcount.pl directly so as to trigger this, GetBiblioItemData wouldn't die this way in the old code. Also, you can only call this from an existing catalogue/moredetail.tt page. Perhaps the uglier: $data = $biblio ? $biblio->unblessed : {}; ::: t/db_dependent/Biblio.t @@ +198,5 @@
is( $marc->subfield( $title_field, $title_subfield ), $title, );
+ my $biblioitem = Koha::Biblioitems->find( $biblioitemnumber ); + is( $biblioitem->_result->biblio->title, $title, # Should be $biblioitem->biblio instead, but not needed elsewhere for now + 'Do not know if this makes sense - compare result of previous two GetBiblioData tests.');
This test was proving that the GetBiblioItemData function worked by confirming that the title set in the Biblio was the one retrieved in this function. Given that this bug patch set removes the function, I'm not sure this test is needed at all. I'd recommend removing this one test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to M. Tompsett from comment #4)
Comment on attachment 70385 [details] [review] Bug 19943: Koha::GetBiblioItemData - Replace existing occurrences
Review of attachment 70385 [details] [review]: -----------------------------------------------------------------
::: circ/bookcount.pl @@ +39,5 @@
my $biblionumber = $input->param('biblionumber');
my $idata = itemdatanum($itm); +my $biblio = Koha::Biblios->find( $biblionumber ); +die "No valid biblionumber passed" unless $biblio; # FIXME A bit rude!
I believe if you purposefully call bookcount.pl directly so as to trigger this, GetBiblioItemData wouldn't die this way in the old code. Also, you can only call this from an existing catalogue/moredetail.tt page. Perhaps the uglier: $data = $biblio ? $biblio->unblessed : {};
Nope, I am waiting for output_and_exit_if_error from bug 18403 to deal with that.
::: t/db_dependent/Biblio.t @@ +198,5 @@
is( $marc->subfield( $title_field, $title_subfield ), $title, );
+ my $biblioitem = Koha::Biblioitems->find( $biblioitemnumber ); + is( $biblioitem->_result->biblio->title, $title, # Should be $biblioitem->biblio instead, but not needed elsewhere for now + 'Do not know if this makes sense - compare result of previous two GetBiblioData tests.');
This test was proving that the GetBiblioItemData function worked by confirming that the title set in the Biblio was the one retrieved in this function. Given that this bug patch set removes the function, I'm not sure this test is needed at all. I'd recommend removing this one test.
I added a comment to tell that, so I think it's ok. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Kyle M Hall <kyle@bywatersolutions.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=19943 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70384|0 |1 is obsolete| | Attachment #70385|0 |1 is obsolete| | Attachment #70386|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72568 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72568&action=edit Bug 19943: Remove itemtype vs itype confusion in CanBookBeIssued Just a preliminary step to clean the code a bit in CanBookBeIssued. The effective item type is already set from GetItem and we do not need to deal with that again. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72569&action=edit Bug 19943: Koha::GetBiblioItemData - Replace existing occurrences The biblioitem's info can be retrieved with Koha::Biblio->biblioitem Test plan: 1. Use the age restriction to restrict checkouts for a given patron 2. Check some items of a biblio out, go to "Items" tab, then "View item's checkout history" link. Compare views with and without patches Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72570 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72570&action=edit Bug 19943: Remove C4::Biblio::GetBiblioItemData It is no longer used. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72568|0 |1 is obsolete| | --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 72593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72593&action=edit Bug 19943: Remove itemtype vs itype confusion in CanBookBeIssued Just a preliminary step to clean the code a bit in CanBookBeIssued. The effective item type is already set from GetItem and we do not need to deal with that again. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72569|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 72594 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72594&action=edit Bug 19943: Koha::GetBiblioItemData - Replace existing occurrences The biblioitem's info can be retrieved with Koha::Biblio->biblioitem Test plan: 1. Use the age restriction to restrict checkouts for a given patron 2. Check some items of a biblio out, go to "Items" tab, then "View item's checkout history" link. Compare views with and without patches Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72570|0 |1 is obsolete| | --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 72595 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72595&action=edit Bug 19943: Remove C4::Biblio::GetBiblioItemData It is no longer used. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=19943 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 72596 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72596&action=edit Bug 19943: Gentle error handling for bookcount.pl This patch tests the required parameters and redirects with a 400 HTTP code if parameters are invalid. It also removes the need for the passed biblioitemnumber which is not used at all. A (now) useless sub is removed too. 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=19943 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72593|0 |1 is obsolete| | Attachment #72594|0 |1 is obsolete| | Attachment #72595|0 |1 is obsolete| | Attachment #72596|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73189&action=edit Bug 19943: Remove itemtype vs itype confusion in CanBookBeIssued Just a preliminary step to clean the code a bit in CanBookBeIssued. The effective item type is already set from GetItem and we do not need to deal with that again. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73190&action=edit Bug 19943: Koha::Biblio - Replace GetBiblioItemData with Koha::Biblio->biblioitem The biblioitem's info can be retrieved with Koha::Biblio->biblioitem Test plan: 1. Use the age restriction to restrict checkouts for a given patron 2. Check some items of a biblio out, go to "Items" tab, then "View item's checkout history" link. Compare views with and without patches Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73191 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73191&action=edit Bug 19943: Remove C4::Biblio::GetBiblioItemData It is no longer used. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73192 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73192&action=edit Bug 19943: Gentle error handling for bookcount.pl This patch tests the required parameters and redirects with a 400 HTTP code if parameters are invalid. It also removes the need for the passed biblioitemnumber which is not used at all. A (now) useless sub is removed too. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73194 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73194&action=edit Bug 19943: Fix NoIssuesChargeGuarantees.t - create the biblioitem entry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73198&action=edit Bug 19943: Fix dateexpiry.t - create the biblioitem entry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73199 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73199&action=edit Bug 19943: Fix SwitchOnSiteCheckouts.t - create the biblioitem entry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 73200 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73200&action=edit Bug 19943: Fix Borrower_PrevCheckout.t - create the biblioitem entry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Last 4 patches pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to Master |RESOLVED --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Enhancement, not backported for 17.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20499 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20499 [Bug 20499] Checkout of bad barcode: Internal Server Error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20825 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20825 [Bug 20825] The method is not covered by tests! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19943 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20889 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20889 [Bug 20889] Items marked as not for loan can be checked out -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org