Bug 18923 - Resolve a warn in Biblio::GetCOinSBiblio
Summary: Resolve a warn in Biblio::GetCOinSBiblio
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Marcel de Rooy
QA Contact: Julian Maurice
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2017-07-10 13:02 UTC by Marcel de Rooy
Modified: 2019-10-14 19:56 UTC (History)
4 users (show)

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


Attachments
Bug 18923: Warning in Biblio::GetCOinSBiblio (1.50 KB, patch)
2017-07-10 13:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18923: Warning in Biblio::GetCOinSBiblio (1.82 KB, patch)
2017-10-09 08:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18923: Warning in Biblio::GetCOinSBiblio (1.83 KB, patch)
2017-10-12 12:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18923: Warning in Biblio::GetCOinSBiblio (1.89 KB, patch)
2017-10-13 12:46 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-07-10 13:02:54 UTC

    
Comment 1 Marcel de Rooy 2017-07-10 13:03:33 UTC
Created attachment 64961 [details] [review]
Bug 18923: Warning in Biblio::GetCOinSBiblio

Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1456.

Test plan:
Enable COinSinOPACResults.
Without this patch, do some opac searches. Check the log.
Apply this patch, search again and check the log.
Comment 2 Katrin Fischer 2017-08-26 22:27:57 UTC
Tried to test this, but was not able to make the warn show in the logs on kohadevbox. Checked koha-plack-err and koha-opac-err. 
Also made sure my result list had some records with 773 (analytics)
Comment 3 Marcel de Rooy 2017-08-28 07:16:51 UTC
(In reply to Katrin Fischer from comment #2)
> Tried to test this, but was not able to make the warn show in the logs on
> kohadevbox. Checked koha-plack-err and koha-opac-err. 
> Also made sure my result list had some records with 773 (analytics)

Enable COinSinOPACResults.
Make sure that either 773a or 773t is not there.
Comment 4 Katrin Fischer 2017-08-28 07:18:25 UTC
Sorry, but already did that :(
Comment 5 Marcel de Rooy 2017-08-28 07:40:20 UTC
(In reply to Katrin Fischer from comment #4)
> Sorry, but already did that :(

And without this patch, perform some opac searches (that include the record with incomplete 773)
Comment 6 Katrin Fischer 2017-08-28 07:51:30 UTC
Hm, misread, NOT there. Will try that later.
Comment 7 Katrin Fischer 2017-10-07 18:19:15 UTC
My record had a 773 like this:

  <datafield tag="773" ind1="1" ind2="8">
    <subfield code="w">000621110</subfield>
    <subfield code="g">3</subfield>
    <subfield code="q">3,2</subfield>
  </datafield>

Still no luck. Can you please test again? Maybe another patch fixed it.
Comment 8 Marcel de Rooy 2017-10-09 08:50:34 UTC
(In reply to Katrin Fischer from comment #7)
> My record had a 773 like this:
> 
>   <datafield tag="773" ind1="1" ind2="8">
>     <subfield code="w">000621110</subfield>
>     <subfield code="g">3</subfield>
>     <subfield code="q">3,2</subfield>
>   </datafield>
> 
> Still no luck. Can you please test again? Maybe another patch fixed it.

Aah. Found it: This part of code in GetCOinSBiblio only is touched when some leader values are set. Position 6 should be a and position 7 should be a.
Note: First test without the patch. Then apply and test again. The warning should not be generated in the latter case.
Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1370.
Will adjust the test plan accordingly.
Comment 9 Marcel de Rooy 2017-10-09 08:51:04 UTC
Created attachment 67835 [details] [review]
Bug 18923: Warning in Biblio::GetCOinSBiblio

Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1456.
Note: In current master this is now line 1370 (Oct 9, 2017).

Test plan:
Enable COinSinOPACResults.
Select a record with leader pos6==a and pos7==a. This triggers genre to be
journalArticle and titletype to be a.
Without this patch, do an opac search that includes this record.
Check the log. You should see the warning.
Apply this patch, search again and check the log. The warning should not be
repeated again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Owen Leonard 2017-10-12 12:45:32 UTC
Created attachment 67999 [details] [review]
Bug 18923: Warning in Biblio::GetCOinSBiblio

Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1456.
Note: In current master this is now line 1370 (Oct 9, 2017).

Test plan:
Enable COinSinOPACResults.
Select a record with leader pos6==a and pos7==a. This triggers genre to be
journalArticle and titletype to be a.
Without this patch, do an opac search that includes this record.
Check the log. You should see the warning.
Apply this patch, search again and check the log. The warning should not be
repeated again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Julian Maurice 2017-10-13 12:46:13 UTC
Created attachment 68070 [details] [review]
Bug 18923: Warning in Biblio::GetCOinSBiblio

Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1456.
Note: In current master this is now line 1370 (Oct 9, 2017).

Test plan:
Enable COinSinOPACResults.
Select a record with leader pos6==a and pos7==a. This triggers genre to be
journalArticle and titletype to be a.
Without this patch, do an opac search that includes this record.
Check the log. You should see the warning.
Apply this patch, search again and check the log. The warning should not be
repeated again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 12 Jonathan Druart 2017-10-13 17:00:05 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 13 Marcel de Rooy 2017-12-04 13:20:13 UTC
Not backported?
Comment 14 Fridolin Somers 2018-01-09 13:20:45 UTC
(In reply to Marcel de Rooy from comment #13)
> Not backported?

Oh I missed it.
Comment 15 Fridolin Somers 2018-01-09 13:21:07 UTC
Pushed to 17.05.x for 17.05.08
Comment 16 Chris Cormack 2018-01-13 00:27:12 UTC
Pushed to 16.11.x will be in 16.11.16
Comment 17 Marcel de Rooy 2018-10-26 07:12:52 UTC
Hmm. In 17.05.x I still find this bug labeled as 18923, but in 16.11.x it pops up under 18922 ? What happened ?
Comment 18 Fridolin Somers 2018-11-02 15:54:16 UTC
(In reply to Marcel de Rooy from comment #17)
> Hmm. In 17.05.x I still find this bug labeled as 18923, but in 16.11.x it
> pops up under 18922 ? What happened ?

Whooo indeed, i dont understand because cherry-picked SHA is OK