[Koha-bugs] [Bug 11592] opac scripts do not respect MARC tag visibility

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11592

--- Comment #8 from Robin Sheat <robin at catalyst.net.nz> ---
Comment on attachment 24655
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24655
Bug 11592 - opac scripts do not respect marc tag visibility

Review of attachment 24655:
 --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11592&attachment=24655)
-----------------------------------------------------------------

Will this also hide appropriate fields in opac-ISBDdetail that should be
hidden?

::: C4/Biblio.pm
@@ +1222,5 @@
> +
> +C<$OpacHideMARC> is a ref to a hash which contains a series
> +of key value pairs indicating if that field (key) is
> +hidden (value == 1) or not (value == 0).
> +

How can this be used to get information about something that doesn't have a
kohafield attached? e.g. I want to hide marc field 505 from the OPAC, but it
doesn't have a corresponding Koha field. Would this still work?

@@ +1230,5 @@
> +
> +sub GetOpacHideMARC {
> +    my ( $frameworkcode ) = shift || '';
> +    my $dbh = C4::Context->dbh;
> +    my $sth = $dbh->prepare("SELECT kohafield AS field,tagfield AS tag,hidden FROM marc_subfield_structure WHERE LENGTH(kohafield)>0 AND frameworkcode=? ORDER BY field,tagfield;");

I would suspect that doing WHERE kohafield <> '' might be a tiny bit faster
than asking it to do a length calculation. Probably negligible though. I'd
probably also fix the spacing as right now it looks like it groups wrongly in
the columns that it's selecting, even though it doesn't.

@@ +1238,5 @@
> +    foreach my $fullfield (keys %{$data}) {
> +        my @tmpsplit = split(/\./,$fullfield);
> +        my $field = $tmpsplit[-1];
> +        foreach my $tag (keys %{$data->{$fullfield}}) {
> +            if ($data->{$fullfield}->{$tag}->{'hidden'}>0) {

!=0 is likely to be marginally faster (unless negatives are a thing you care
about.)

@@ +1270,5 @@
> +    my $filtered_record = $record->clone;
> +
> +    my ( $frameworkcode ) = shift || '';
> +
> +    my $marcsubfieldstructure = GetMarcStructure(0,$frameworkcode);

Maybe allow the marcsubfieldstructure to be passed in instead of the framework
code. This becomes important if this happens over and over, as it'll do a big
bunch of database work each time, this makes things very slow when it could be
cached outside and passed in.

It should be easy enough to see if you have a scalar or a ref, and so whether
you have a code or the structure.

@@ +1274,5 @@
> +    my $marcsubfieldstructure = GetMarcStructure(0,$frameworkcode);
> +    if ($marcsubfieldstructure->{'000'}->{'@'}->{hidden}>0) {
> +        # LDR field is excluded from $record->fields().
> +        # if we hide it here, the MARCXML->MARC::Record->MARCXML transformation blows up.
> +    }

This if doesn't actually do anything.

@@ +1977,4 @@
>          push @marcsubjects, {
>              MARCSUBJECT_SUBFIELDS_LOOP => \@subfields_loop,
>              authoritylink => $authoritylink,
> +        } if $authoritylink || $#subfields_loop>=0;

$#subfields_loop>=0 is a bit of an ugly construction. Best to use just
@subfields_loop, it does the same thing and is easier to read.

::: opac/opac-detail.pl
@@ +489,4 @@
>  }
>  
>  my $dat = &GetBiblioData($biblionumber);
> +my $OpacHideMARC  = &GetOpacHideMARC($dat->{'frameworkcode'});

& is a perl4-ism, not required.

@@ +654,5 @@
> +my ($st_tag,$st_subtag) = GetMarcFromKohaField('bibliosubtitle.subtitle',$dat->{'frameworkcode'});
> +my $subtitle;
> +if ($st_tag && $st_subtag) {
> +    my @subtitles = $record->subfield($st_tag,$st_subtag);
> +    $subtitle = \@subtitles if scalar @subtitles;

you don't need to say 'scalar' here.

@@ +656,5 @@
> +if ($st_tag && $st_subtag) {
> +    my @subtitles = $record->subfield($st_tag,$st_subtag);
> +    $subtitle = \@subtitles if scalar @subtitles;
> +}
> +if ($subtitle && scalar @$subtitle) {

nor here

-- 
You are receiving this mail because:
You are watching all bug changes.