Bug 13180 - Errors when acquiring from the new improved staged file, empty indicator warning
Summary: Errors when acquiring from the new improved staged file, empty indicator warning
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Keywords:
Depends on: 5961 7180
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-31 14:48 UTC by Olli-Antti Kivilahti
Modified: 2015-05-08 03:14 UTC (History)
2 users (show)

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


Attachments
Bug 13180 - While acquiring from the new improved staged file, warning "Indicator in 952 is empty at /home/koha/kohaclone/C4/Biblio.pm" is flooding the bejesus out of my koha-error.log (3.33 KB, patch)
2014-10-31 15:06 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 13180 - Errors when acquiring from the new improved staged file, empty indicator warning, is flooding the gosh-darn out of my koha-error.log... Nice to have you back joubu! (3.28 KB, patch)
2014-11-07 14:32 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2014-10-31 14:48:58 UTC
I get one warning for each mapped Koha items field.

This patch fakes the missing indicators to the addorderiso2709.pl/.tt.
Those indicators don't really exists for field 952 anyway.
Comment 1 Olli-Antti Kivilahti 2014-10-31 15:06:36 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-11-05 15:15:44 UTC
Comment on attachment 33078 [details] [review]
Bug 13180 - While acquiring from the new improved staged file, warning "Indicator in 952 is empty at /home/koha/kohaclone/C4/Biblio.pm" is flooding the bejesus out of my koha-error.log

Review of attachment 33078 [details] [review]:
-----------------------------------------------------------------

::: acqui/addorderiso2709.pl
@@ +272,2 @@
>              my @ind_tag   = $input->param('ind_tag');
>              my @indicator = $input->param('indicator');

Hum? This line already exists.
Comment 3 Jonathan Druart 2014-11-05 15:16:18 UTC
+            my @indicator   = $input->param('indicator'); #Passing the imaginary empty "  " -indicators from the templates, to stop C4::Biblio::TransformHtmlToXml() to warn the bejesus out of my koha-error.log
             my @ind_tag   = $input->param('ind_tag');
             my @indicator = $input->param('indicator');
Comment 4 Jonathan Druart 2014-11-05 15:16:56 UTC
Could you please update the patch with a decent commit message? :)
Comment 5 Olli-Antti Kivilahti 2014-11-07 14:32:30 UTC
Created attachment 33375 [details] [review]
Bug 13180 - Errors when acquiring from the new improved staged file, empty indicator warning, is flooding the gosh-darn out of my koha-error.log... Nice to have you back joubu!

I get one warning for each mapped Koha items field.

This patch fakes the missing indicators to the addorderiso2709.pl/.tt.
Those indicators don't really exists for field 952 anyway.
Comment 6 Olli-Antti Kivilahti 2014-11-07 14:34:22 UTC
Fixed the indecent commit message.
I am so sorry about that :(
Comment 7 Olli-Antti Kivilahti 2014-11-07 14:36:33 UTC
Maybe it would be best to just hard-code the indicator as '  ' in the addorderiso2709.pl instead of dragging it all the way from the templates?

Smarter men can decide.
Comment 8 Mark Tompsett 2015-05-08 03:02:39 UTC
Comment on attachment 33375 [details] [review]
Bug 13180 - Errors when acquiring from the new improved staged file, empty indicator warning, is flooding the gosh-darn out of my koha-error.log... Nice to have you back joubu!

Review of attachment 33375 [details] [review]:
-----------------------------------------------------------------

I stopped before I cried too much, not because of this patch, but because of the resulting issue(s) I found.

::: acqui/addorderiso2709.pl
@@ +269,4 @@
>              my @field_values = $input->param('field_value');
>              my @serials      = $input->param('serial');
>              my @ind_tag   = $input->param('ind_tag');
> +            my @indicator = $input->param('indicator'); #Passing the imaginary empty "  " -indicators from the templates, to stop C4::Biblio::TransformHtmlToXml() to warn the bejesus out of my koha-error.log

First, why is it @indicator and only $input->param and not $input->multi_param?

Next, why not append "// ();" so as to reduce the number of files modified?!

In fact, as far as I can tell addorderido2709.pl can't be called with an indicator parameter!

$ git grep addorderiso2709.pl

acqui/addorder.pl:    print $input->redirect("/cgi-bin/koha/acqui/addorderiso2709.pl?import_batch_id=$import_batch_id&basketno=$basketno&booksellerid=$booksellerid");
--- This won't pass indicator.

acqui/addorderiso2709.pl:$template->param(scriptname => "/cgi-bin/koha/acqui/addorderiso2709.pl",
--- This doesn't do anything to pass indicator.

koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-add-to-basket.inc:        <li><a href="/cgi-bin/koha/acqui/addorderiso2709.pl?booksellerid=[% booksellerid %]&amp;basketno=[% basketno %]"> From a staged file</a></li>
--- This is a link. No indicator passed.

koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt:                <form action="/cgi-bin/koha/acqui/addorderiso2709.pl" method="post" id="Aform">
--- Only the actual template could pass it.

$ grep indicator ...
--- You get the idea, nothing was there.

@@ +275,5 @@
>              push @{ $item->{subfields} },    $subfields[0];
>              push @{ $item->{field_values} }, $field_values[0];
>              push @{ $item->{ind_tag} },      $ind_tag[0];
>              push @{ $item->{indicator} },    $indicator[0];
> +            my $xml = TransformHtmlToXml( \@tags, \@subfields, \@field_values, \@indicator, \@ind_tag );

I checked the prototype of the TransfromHtmlToXML:
 $tags, $subfields, $values, $indicator, $ind_tag, $auth_type
I see you caught the swapped parameters. Good! :)

ARG! C4::Biblio insanity ($ind_tag isn't even used in the function!)

::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
@@ +299,4 @@
>                                          <input type="hidden" name="itemid" value="1" />
>                                          <input type="hidden" name="kohafield" value="[% iteminformatio.kohafield %]" />
>                                          <input type="hidden" name="tag" value="[% iteminformatio.tag %]" />
> +                                        <input type="hidden" name="indicator" value="  " /> [%#Pass imaginary indicators to stop warnings in C4::Biblio::TransformHtmlToXml() %]

Yuck. No.
Comment 9 Mark Tompsett 2015-05-08 03:05:15 UTC
Perhaps we can get a better patch? I'm switching to Failed QA so it doesn't languish in the "In Discussion" queue.