http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11592
--- Comment #42 from M. Tompsett <[email protected]> --- (In reply to David Cook from comment #41) > ::: C4/Biblio.pm > > + my $rv = $sth->execute($frameworkcode); > > Why is there a "my $rv" here? It shouldn't be needed. It was debugging code that I forgot to strip. :) > > + if (!$show || $show==0) { > > $show should always be 1 or undefined, so I wouldn't think $show==0 > would be necessary. Actually, 1, 0, or undefined. Someone could put => 0 into the hash checked. I'll confirm if I can simplify the logic. > @@ +1334,5 @@ > > + my $show = $display->{$interface}->{ > > $data->{$fullfield}->{$tag}->{'hidden'} }; > > + if (!$show || $show==0) { > > + $shouldhidemarc{ $field } = 1; > > + } > > + elsif ( !exists($shouldhidemarc{ $field }) ) { > > It shouldn't be necessary to add fields that aren't hidden to this hash. > Plus, since the value is 0, you'll most likely get the same end result you > would if you didn't have this block of code. I don't like having undefined values. Those tend to generate annoying floody errors when some comparison works but was expecting a number, for example. > @@ +1493,5 @@ > > + # LDR field is excluded from $record->fields(). > > + # if we hide it here, the MARCXML->MARC::Record->MARCXML > > + # transformation blows up. > > + #} > > + foreach my $fields ($record->fields()) { > > Should probably use $field rather than $fields, as this incorrect > pluralization gets confusing lower down. Okay, that makes sense. > @@ +1495,5 @@ > > + # transformation blows up. > > + #} > > + foreach my $fields ($record->fields()) { > > + my $tag = $fields->tag(); > > + my $hidden; > > This $hidden variable isn't really necessary. If it should be hidden, you > should just hide/delete it upon detection. The code looks like it would work > either way, but it would be easier to read with less needless code. If you look at the hidden value setting that would be inside an if/else. I scoped it wider, so there was no need to have two lines of code. Fine, two lines of code it is. > > + my $visibility = > > $marcsubfieldstructure->{$tag}->{$subtag}->{hidden}; > > + $visibility //= 0; > > Is there are a reason to have this extra line instead of "my $visibility = > $marcsubfieldstructure->{$tag}->{$subtag}->{hidden} // 0"? Yes, because otherwise visibility is undef. It should be a valid number (-7 to +7) reflecting valid values set in the Advanced Constraints screen. > @@ +1670,4 @@ > > $oauthors .= "&rft.au=$au"; > > } > > } > > + $title = $record->subfield( '245', 'a' ) // ''; > > Why is this in this patch? This fixes a floody error I had during testing. I suppose it could be another patch. I'll confirm. > @@ +2122,4 @@ > > push @marcsubjects, { > > MARCSUBJECT_SUBFIELDS_LOOP => \@subfields_loop, > > authoritylink => $authoritylink, > > + } if $authoritylink || @subfields_loop; > > Why is this in this patch? This properly hides the subjects. > ::: C4/XSLT.pm > @@ +93,5 @@ > > + @new_subfields > > + ) ); > > + } > > + else { > > + $record->delete_fields($field); > > What is this doing here, and why is it in this patch? If you try to delete the last subfield of a field, you need to delete the field. If I recall, deleting the last subfield fails. Fields/subfields are deleted (from memory) to hide them from being displayed. > ::: opac/opac-detail.pl > @@ +683,4 @@ > > my $marcseriesarray = GetMarcSeries ($record,$marcflavour); > > my $marcurlsarray = GetMarcUrls ($record,$marcflavour); > > my $marchostsarray = GetMarcHosts($record,$marcflavour); > > +my ($st_tag,$st_subtag) = > > GetMarcFromKohaField('bibliosubtitle.subtitle',$dat->{'frameworkcode'}); > > I think that this section is a mistake. GetRecordValue, which is what was there initially, does not grab values from marc_subfield_structure, but rather fieldmapping. This means the subtitle is not properly hidden. > What is bibliosubtitle.subtitle? Also, why would this be in this patch? The value in the marc_subfield_structure table. Subtitles are a mess in Koha. > ::: opac/opac-showmarc.pl > @@ +55,4 @@ > > > > if ($view eq 'card' || $view eq 'html') { > > my $xmlrecord= $importid? $record->as_xml(): > > GetXmlBiblio($biblionumber); > > + if (!$importid && $view eq 'html') { > > I haven't looked at the context, but why not filter for all cases here? I think this is related to the save as exports. This is what worked for me. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
