Bug 18276 - Koha::Biblio - Remove GetBiblioFromItemNumber
Summary: Koha::Biblio - Remove GetBiblioFromItemNumber
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 17628 18277 19388 19431 19651 19798 19840 19899 20546 21168
  Show dependency treegraph
 
Reported: 2017-03-15 19:08 UTC by Jonathan Druart
Modified: 2019-10-14 19:58 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (9.38 KB, patch)
2017-03-15 19:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.65 KB, patch)
2017-03-15 19:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.21 KB, patch)
2017-03-15 19:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.71 KB, patch)
2017-03-15 19:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (1.99 KB, patch)
2017-03-15 19:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (9.08 KB, patch)
2017-03-16 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.65 KB, patch)
2017-03-16 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.21 KB, patch)
2017-03-16 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.32 KB, patch)
2017-03-16 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (1.99 KB, patch)
2017-03-16 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.79 KB, patch)
2017-04-12 13:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.65 KB, patch)
2017-04-12 13:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.21 KB, patch)
2017-04-12 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.26 KB, patch)
2017-04-12 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (1.99 KB, patch)
2017-04-12 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.79 KB, patch)
2017-04-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.65 KB, patch)
2017-04-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.21 KB, patch)
2017-04-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.25 KB, patch)
2017-04-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (1.99 KB, patch)
2017-04-20 15:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: FIX status display for course reserves (3.80 KB, patch)
2017-04-20 15:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.79 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.65 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.22 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.64 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (1.99 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: FIX status display for course reserves (3.80 KB, patch)
2017-05-04 15:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI (2.22 KB, patch)
2017-05-05 13:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.86 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.72 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.29 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.71 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (2.06 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: FIX status display for course reserves (3.87 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI (2.29 KB, patch)
2017-05-09 15:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy (1.21 KB, patch)
2017-06-30 09:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.88 KB, patch)
2017-07-05 17:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.76 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.29 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.89 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (2.05 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: FIX status display for course reserves (3.86 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI (2.46 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy (1.18 KB, patch)
2017-07-05 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: barcode may be nonexistent when checking out/in (6.10 KB, patch)
2017-07-05 17:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves (8.94 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI (1.85 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages (7.37 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones (50.90 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine (2.14 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: FIX status display for course reserves (3.95 KB, patch)
2017-07-07 07:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI (2.55 KB, patch)
2017-07-07 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy (1.26 KB, patch)
2017-07-07 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: barcode may be nonexistent when checking out/in (6.18 KB, patch)
2017-07-07 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: [QA Follow-up] Fix two new calls (2.57 KB, patch)
2017-07-07 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - Easy ones (881 bytes, patch)
2017-07-10 15:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - Easy ones (2.02 KB, patch)
2017-07-10 16:04 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-03-15 19:08:23 UTC

    
Comment 1 Jonathan Druart 2017-03-15 19:52:29 UTC
Created attachment 61122 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.
Comment 2 Jonathan Druart 2017-03-15 19:52:36 UTC
Created attachment 61123 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch
Comment 3 Jonathan Druart 2017-03-15 19:52:40 UTC
Created attachment 61124 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)
Comment 4 Jonathan Druart 2017-03-15 19:52:45 UTC
Created attachment 61125 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial
Comment 5 Jonathan Druart 2017-03-15 19:52:52 UTC
Created attachment 61126 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine
Comment 6 Marc Véron 2017-03-16 09:05:36 UTC
While testing first patch I get an Internal Server Error. Plack error log says:
Template process failed: undef error - The method ccode is not covered by tests! at (...)/C4/Templates.pm line 121.
Comment 7 Jonathan Druart 2017-03-16 16:49:06 UTC
Created attachment 61177 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.
Comment 8 Jonathan Druart 2017-03-16 16:49:20 UTC
Created attachment 61178 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch
Comment 9 Jonathan Druart 2017-03-16 16:49:39 UTC
Created attachment 61179 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)
Comment 10 Jonathan Druart 2017-03-16 16:49:56 UTC
Created attachment 61180 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial
Comment 11 Jonathan Druart 2017-03-16 16:50:17 UTC
Created attachment 61181 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine
Comment 12 Marc Véron 2017-04-12 12:51:40 UTC
Merge conflicts in 1st patch:
koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/add_items-step2.tt
course_reserves/add_items.pl

Maybe a merge conflict in 3rd patch as well (other patches applied when i skipped 1st and 3rd patch)
Comment 13 Jonathan Druart 2017-04-12 13:23:54 UTC
Created attachment 62081 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.
Comment 14 Jonathan Druart 2017-04-12 13:23:58 UTC
Created attachment 62082 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch
Comment 15 Jonathan Druart 2017-04-12 13:24:03 UTC
Created attachment 62083 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)
Comment 16 Jonathan Druart 2017-04-12 13:24:08 UTC
Created attachment 62084 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial
Comment 17 Jonathan Druart 2017-04-12 13:24:12 UTC
Created attachment 62085 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine
Comment 18 Marc Véron 2017-04-12 17:48:40 UTC
 Bug 18276: Remove GetBiblioFromItemNumber - Course reserves:

Internal server error while displaying course detail in OPAC.

Plack error log says:
Template process failed: undef error - The method datedue is not covered by tests! at .../koha/C4/Templates.pm line 121

It must be related with line 76 of opac-course-details.tt rsp. an error inside item-status.inc
<td>[% INCLUDE 'item-status.inc' item=cr.item issue=cr.issue %]</td>
(No error if I remove the template directive)
Comment 19 Jonathan Druart 2017-04-12 18:42:51 UTC
Kyle, could you take a look please?
On current master opac-course-details.tt includes item-status.inc.
But to be displayed correctly the status need info from the checkout.
These info are never sent to the template.
What were you expecting to display under this "status" column?
Comment 20 Kyle M Hall 2017-04-12 18:59:12 UTC
(In reply to Jonathan Druart from comment #19)
> Kyle, could you take a look please?
> On current master opac-course-details.tt includes item-status.inc.
> But to be displayed correctly the status need info from the checkout.
> These info are never sent to the template.
> What were you expecting to display under this "status" column?

Each element in the course_reserves loop should have a key 'issue' if it has an open issue. This data is pulled by GetCourseReserves using GetOpenIssue. It's meant to show what the item's general status is ( available, lost, damaged, checked out, etc ).
Does that make sense?
Comment 21 Jonathan Druart 2017-04-20 15:32:18 UTC
Kyle,
What I meant is that item-status.inc needs item.waiting, item.itemnotforloan, item.transfertwhen, item.datedue and more to display the status of the items.
These flags are set from C4::Search::searchResults, not GetBiblioFromItemNumber (which fills the item values sent to the include).
Comment 22 Jonathan Druart 2017-04-20 15:51:37 UTC
Created attachment 62476 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.
Comment 23 Jonathan Druart 2017-04-20 15:51:44 UTC
Created attachment 62477 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch
Comment 24 Jonathan Druart 2017-04-20 15:51:50 UTC
Created attachment 62478 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)
Comment 25 Jonathan Druart 2017-04-20 15:51:55 UTC
Created attachment 62479 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial
Comment 26 Jonathan Druart 2017-04-20 15:52:00 UTC
Created attachment 62480 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine
Comment 27 Jonathan Druart 2017-04-20 15:52:05 UTC
Created attachment 62481 [details] [review]
Bug 18276: FIX status display for course reserves

This include file is terribly wrong, it's called from different places
that do not set the same flag.
The status from detail and result page might be different from the one
display on the course reserve table.
Comment 28 Josef Moravec 2017-05-04 11:43:40 UTC
I've got this error on course reserve detail page in opac:

Template process failed: undef error - The method onsite_checkout is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.


Also there are some problems reported by qa tools:

FAIL	C4/SIP/ILS/Item.pm
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 74)
		forbidden pattern: tab char (line 99)
		forbidden pattern: tab char (line 100)
		forbidden pattern: tab char (line 90)

 FAIL	C4/SIP/ILS/Transaction/Hold.pm
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 108)

 FAIL	circ/returns.pl
   FAIL	  valid
		Global symbol "$biblio" requires explicit package name 
		circ/returns.pl had compilation errors.

 FAIL	circ/transferstoreceive.pl
   FAIL	  valid
		Global symbol "$num" requires explicit package name 
		Global symbol "$getransf" requires explicit package name 
		Global symbol "%getransf" requires explicit package name 
		syntax error 
		Global symbol "$biblio" requires explicit package name 
		Global symbol "%branchloop" requires explicit package name 
		circ/transferstoreceive.pl has too many errors.

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/moveitem.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 39)
		forbidden pattern: tab char (line 46)

 FAIL	offline_circ/process_koc.pl
   FAIL	  valid
		Global symbol "$biblio" requires explicit package name 
		offline_circ/process_koc.pl had compilation errors.
Comment 29 Jonathan Druart 2017-05-04 15:09:01 UTC
Created attachment 63121 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.
Comment 30 Jonathan Druart 2017-05-04 15:09:12 UTC
Created attachment 63122 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch
Comment 31 Jonathan Druart 2017-05-04 15:09:20 UTC
Created attachment 63123 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)
Comment 32 Jonathan Druart 2017-05-04 15:09:29 UTC
Created attachment 63124 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial
Comment 33 Jonathan Druart 2017-05-04 15:09:37 UTC
Created attachment 63125 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine
Comment 34 Jonathan Druart 2017-05-04 15:09:44 UTC
Created attachment 63126 [details] [review]
Bug 18276: FIX status display for course reserves

This include file is terribly wrong, it's called from different places
that do not set the same flag.
The status from detail and result page might be different from the one
display on the course reserve table.
Comment 35 Jonathan Druart 2017-05-04 15:10:49 UTC
I would suggest to not make this change a priority for 17.05
Comment 36 Josef Moravec 2017-05-05 09:57:24 UTC
(In reply to Jonathan Druart from comment #30)
> Created attachment 63122 [details] [review] [review]
> Bug 18276: Remove GetBiblioFromItemNumber - ILSDI
> 
> Test plan:
> Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
> The results should be the same before and after this patch

Works good for item level holds, with title level holds I see:

"Can't call method &quot;biblio&quot; on an undefined value at /home/vagrant/kohaclone/C4/ILSDI/Services.pm line 422"
Comment 37 Josef Moravec 2017-05-05 10:06:42 UTC
All other looks good for me
Comment 38 Jonathan Druart 2017-05-05 13:11:43 UTC
Created attachment 63166 [details] [review]
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI

Take into account biblio level holds
Comment 39 Jonathan Druart 2017-05-05 13:12:11 UTC
(In reply to Josef Moravec from comment #36)
> Works good for item level holds, with title level holds I see:
> 
> "Can't call method &quot;biblio&quot; on an undefined value at
> /home/vagrant/kohaclone/C4/ILSDI/Services.pm line 422"

Great catch Josef!
Comment 40 Josef Moravec 2017-05-09 15:14:21 UTC
Created attachment 63294 [details] [review]
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2017-05-09 15:14:28 UTC
Created attachment 63295 [details] [review]
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2017-05-09 15:14:33 UTC
Created attachment 63296 [details] [review]
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Josef Moravec 2017-05-09 15:14:39 UTC
Created attachment 63297 [details] [review]
[SIGNED-OFF] Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 44 Josef Moravec 2017-05-09 15:14:44 UTC
Created attachment 63298 [details] [review]
[SIGNED-OFF] Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 45 Josef Moravec 2017-05-09 15:14:49 UTC
Created attachment 63299 [details] [review]
[SIGNED-OFF] Bug 18276: FIX status display for course reserves

This include file is terribly wrong, it's called from different places
that do not set the same flag.
The status from detail and result page might be different from the one
display on the course reserve table.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 46 Josef Moravec 2017-05-09 15:14:56 UTC
Created attachment 63300 [details] [review]
[SIGNED-OFF] Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI

Take into account biblio level holds

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 47 Marcel de Rooy 2017-06-30 08:54:55 UTC
QA: Looking here now
Comment 48 Marcel de Rooy 2017-06-30 09:10:26 UTC
Created attachment 64748 [details] [review]
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy

FAIL pod Apparent command =cut not preceded by blank line in file misc/cronjobs/advance_notices.pl
FAIL pod Apparent command =cut not preceded by blank line in file C4/SIP/ILS/Item.pm
Comment 49 Marcel de Rooy 2017-06-30 09:12:33 UTC
Add checks for find please.

Can't call method "biblio" on an undefined value at circ/circulation.pl line 334.

Before this patch set you get a warning on an unknown barcode, now an Internal server error..
Comment 50 Jonathan Druart 2017-07-05 17:45:54 UTC
Created attachment 64809 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 51 Jonathan Druart 2017-07-05 17:46:01 UTC
Created attachment 64810 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 52 Jonathan Druart 2017-07-05 17:46:08 UTC
Created attachment 64811 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 53 Jonathan Druart 2017-07-05 17:46:14 UTC
Created attachment 64812 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 54 Jonathan Druart 2017-07-05 17:46:19 UTC
Created attachment 64813 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 55 Jonathan Druart 2017-07-05 17:46:24 UTC
Created attachment 64814 [details] [review]
Bug 18276: FIX status display for course reserves

This include file is terribly wrong, it's called from different places
that do not set the same flag.
The status from detail and result page might be different from the one
display on the course reserve table.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 56 Jonathan Druart 2017-07-05 17:46:29 UTC
Created attachment 64815 [details] [review]
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI

Take into account biblio level holds

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 57 Jonathan Druart 2017-07-05 17:46:34 UTC
Created attachment 64816 [details] [review]
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy

FAIL pod Apparent command =cut not preceded by blank line in file misc/cronjobs/advance_notices.pl
FAIL pod Apparent command =cut not preceded by blank line in file C4/SIP/ILS/Item.pm
Comment 58 Jonathan Druart 2017-07-05 17:48:14 UTC
Fixed conflicts with bug 17843 and bug 17738.
Comment 59 Jonathan Druart 2017-07-05 17:58:40 UTC
Created attachment 64817 [details] [review]
Bug 18276: barcode may be nonexistent when checking out/in
Comment 60 Jonathan Druart 2017-07-05 17:59:32 UTC
(In reply to Marcel de Rooy from comment #49)
> Add checks for find please.
> 
> Can't call method "biblio" on an undefined value at circ/circulation.pl line
> 334.
> 
> Before this patch set you get a warning on an unknown barcode, now an
> Internal server error..

Thanks for catching that Marcel. Must be fixed with the last patch.
The other calls look safe.
Comment 61 Marcel de Rooy 2017-07-07 07:29:34 UTC
Created attachment 64878 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Course reserves

Values from the items, biblio and biblioitems tables are used in the template,
so we need to pass all of them to the template, but separately.
That way we easily see which field from which table we are displaying.

Test plan:
Create a course reserve and add items.
Correct information must be displayed on the detail page of the course
reserve, on staff and OPAC interface.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 62 Marcel de Rooy 2017-07-07 07:29:39 UTC
Created attachment 64879 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - ILSDI

Test plan:
Hit /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_holds=1
The results should be the same before and after this patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 63 Marcel de Rooy 2017-07-07 07:29:43 UTC
Created attachment 64880 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - circulation pages

Test plan:
1. Checkout items in a batch
The information displayed should be ok (title, subtitles, author,
itemnotes, barcode)
2. Checkout an item using the standard checkout page
You should correctly see the information of the item that has been
checked out (title and author)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 64 Marcel de Rooy 2017-07-07 07:29:48 UTC
Created attachment 64881 [details] [review]
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title

This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info

Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds

QA will check the other changes reading the code, it's trivial

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 65 Marcel de Rooy 2017-07-07 07:29:53 UTC
Created attachment 64882 [details] [review]
Bug 18276: Remove the C4::Biblio::GetBiblioFromItemNumber subroutine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 66 Marcel de Rooy 2017-07-07 07:29:57 UTC
Created attachment 64883 [details] [review]
Bug 18276: FIX status display for course reserves

This include file is terribly wrong, it's called from different places
that do not set the same flag.
The status from detail and result page might be different from the one
display on the course reserve table.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 67 Marcel de Rooy 2017-07-07 07:30:02 UTC
Created attachment 64884 [details] [review]
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - ILSDI

Take into account biblio level holds

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 68 Marcel de Rooy 2017-07-07 07:30:07 UTC
Created attachment 64885 [details] [review]
Bug 18276: [QA Follow-up] Two newlines to make qa tools happy

FAIL pod Apparent command =cut not preceded by blank line in file misc/cronjobs/advance_notices.pl
FAIL pod Apparent command =cut not preceded by blank line in file C4/SIP/ILS/Item.pm

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 69 Marcel de Rooy 2017-07-07 07:30:12 UTC
Created attachment 64886 [details] [review]
Bug 18276: barcode may be nonexistent when checking out/in

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 70 Marcel de Rooy 2017-07-07 07:30:18 UTC
Created attachment 64887 [details] [review]
Bug 18276: [QA Follow-up] Fix two new calls

Found two calls in opac-issue-note.pl and svc/patron_notes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 71 Jonathan Druart 2017-07-10 15:55:28 UTC
Created attachment 64974 [details] [review]
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - Easy ones
Comment 72 Jonathan Druart 2017-07-10 16:04:16 UTC
Created attachment 64975 [details] [review]
Bug 18276: (follow-up) Remove GetBiblioFromItemNumber - Easy ones

- Item does not have a title attribute, it comes from biblio
- There is an additional call to effective_itemtype done on AddReturn,
so we need to catch both warnings
Comment 73 Jonathan Druart 2017-07-10 17:53:42 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 74 Fridolin Somers 2017-07-18 11:52:10 UTC
Enhancement not pushed to 17.05.x