http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11592 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #9 from M. Tompsett <mtompset@hotmail.com> ---
Will this also hide appropriate fields in opac-ISBDdetail that should be hidden?
I didn't even look at opac-ISBDdetail -- we don't use it. I suppose I should. Though, it has that syspref related to the output for it. I'll add a filter call.
+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?
OpacHideMARC is intended as a hack for the loop in opac-detail that creates template parameters based on koha field names. It was around like 699 in the master, 713 in the patch. This is how [% title %] and other parameters are created. This covers the half of the problem that doesn't use the MARC record directly. If you are trying to hide something that doesn't have a kohafield, you are looking at the other function: GetFilteredOpacBiblio. This takes the MARC record and strips out things marked to be hidden.
+ 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.
I didn't do testing, but LENGTH(NULL) = 0, right? Which way handles the NULL case better? -- Just checked >'' is used elsewhere and returns the same number on my data.
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.
I don't understand what you are trying to say. OH! field,tagfield lacks spacing. Will fix that to improve readability.
+ if ($data->{$fullfield}->{$tag}->{'hidden'}>0) {
!=0 is likely to be marginally faster (unless negatives are a thing you care about.)
valid values for hidden range from -15 to +... anyways... <=0 OPAC visibility is checked. >0 OPAC visibility is unchecked. So, yes, care about negatives.
@@ +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.
Hmm... GetMarcStructure is cached. Look in C4/Biblio.pm for "sub GetMarcStructure". You will see the $marc_structure_cache line just above that, and it being used at the top of the function.
@@ +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.
It explains why I didn't hide the LDR record. As this function is called only once, I don't think it is a big deal, but I will comment it out so people don't get the idea to fix the remaining LDR field problem this way.
@@ +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.
Okay, I suppose I can try that.
my $dat = &GetBiblioData($biblionumber); +my $OpacHideMARC = &GetOpacHideMARC($dat->{'frameworkcode'});
& is a perl4-ism, not required.
Okay, I was confused a little by it used in some places and not in others. I'll remove my &.
+ $subtitle = \@subtitles if scalar @subtitles;
you don't need to say 'scalar' here.
+if ($subtitle && scalar @$subtitle) {
nor here
I'll test and remove, once confirmed. -- You are receiving this mail because: You are watching all bug changes.