Created attachment 9045 [details] labeled marc with no 090 If you click to view the marc display of a record in the OPAC it doesn't show all of your fields. If you click 'view plain' you do see all the fields. If it's MARC view you should see all the filled in fields on both views, just with a slightly different view. For example see the attached images - the 090 is there on view plain, but not on the labeled view.
Created attachment 9046 [details] plain marc with 090
[13:13] <jcamins> nengard: did you check your frameworks re: 7933? [13:14] <jcamins> Remember the "hidden" attribute only controls expanded MARC view and cataloging. [13:14] <nengard> my argument there is that it should control everything then [13:14] <nengard> it's confusing to the librarian [13:14] <jcamins> I agree it should. [13:14] <jcamins> But it doesn't. [13:15] <jcamins> So the bug isn't that 090 doesn't appear on the expanded MARC view. It's that the "hidden" attribute hasn't been taken out back and shot. [13:16] <sekjal> MARC definitions need to be separated from frameworks [13:17] <jcamins> That would be great. [13:17] <sekjal> hopefully structured in a way that would be as quick to load as possible [13:18] <sekjal> marc defs could be file-based; they don't really change all that much
Cannot reproduce the situation. It appears that in master, the labeled and plain views both display the same MARC fields ( whatever is defined in the record's current framework ).
I just tested, and was able to reproduce the problem on latest master: http://screencast.com/t/BzcOItus
Created attachment 9308 [details] [review] Bug 7933 - Fields in marc display not the same The fields displayed in the 'labeled' and 'plain' views for opac-MARCdetail.pl differ. The labeled view hides subfields marked as hidden, while the 'plain' view does not. This is because the plain view simply grabs the marcxml and transforms it via xslt. The labeled view is processed in a completely different way, which respects the hidden field flag for the set framework. This commit resolves this issue by adding a subroutine to remove hidden fields from a given record, and another subroutine to convert marc records to marx xml. Where before, the raw marc xml was transformed, now the script gets a record with the hidden subfields removed, transforms that to marc xml, and then transforms it again via xslt ( as before ).
We narrowed it down to the tabs in the framework. A number of our librarians request that commonly-used MARC tags appear on the tab 0 for editing. We noticed a pattern that if a MARC tag is not allocated its "natural" tab then that tab will not show in the MARC [view labeled] display. We could prove this by changing the tab for a subfield and making it appear and disappear. There is a loop in opac-MARCdetail.pl which goes through the tabs: # loop through each tab 0 through 9 for ( my $tabloop = 0 ; $tabloop <= 10 ; $tabloop++ ) { (is this another bug as that loop goes through 10!). Lower down is a condition: next if ( ($sf_def->{tab}||'') ne $tabloop); which if commented puts out all tags about 10 times. Added another condition which refines the condition to allow the first character of the tag to match the tabloop and be allowed to display: next if ( ($sf_def->{tab}||'') ne $tabloop && substr($fields[$x_i]->tag(),0,1) ne $tabloop); This seems to do the trick. I have no idea why opac-MARCdetail.pl would want to go through the display tabs when listing the MARC data for a bib record. However until that is unravelled this should solve (at least) the problem identified here.
I'm a little confused by the conversations going on in this bug. The issue reported in comment 6 seems to have to do with the labeled MARC view rather than the plain MARC view, and the proposed patch doesn't implement hidden in a maintainable way. In particular, it seems to take hidden as a boolean, and offers no method for specifying context, which is a rather major functional change (and step back, probably). I think a contextual filter is required, at least.
Comment 6 was in response to the initial (#1) bug. The problems and (a) solution apply to code at version 3.06.02.003 Koha. The code in comment 6 does not mention the hidden characteristic, but the existing code in opac-MARCdetail.pl does indeed do a "next" if hidden. I'll leave it for others to debate the virtue of hiding MARC subfields in the two MARC display formats. However the point I was trying to fix was that in labeled (should be labelled) MARC view it was not showing subfields if we had chosen to move the subfield to a different edit tab (eg move 245$a to tab 0 for convenience). I am sure there are more elegant fixes than mine, but on basic testing the fix proposed works. I could not see any reference in comment 6 to processing for hidden. I see the time of Kyle's post (#5) is the same date as post #6 so if Kyle's post removes the problem I apologise for any confusion but I can only see reference to handling for "hidden" in other recent posts and no mention of tags being moved to different edit tabs failing to display.
Marking this patch failed QA since it misapprehends the meaning of the "hidden" parameter.
Just created 9147 to hide in OPAC subfields in ISBD view like they are in MARC view. It was trivial since framework informations where already used, but not "hidden". Is this not a valid behavior ? I have a request from a library that wants to *really* hide a note field at OPAC. This note contains important informations for librarian only. Are there no other libraries with this request ?
Created attachment 13707 [details] [review] Proposed patch (revisited) I propose a new patch based on same behavior : at OPAC, remove from MARC::Record all subfields declared hidden in framework. (if all subfields are removed, field is removed). C4::Biblio : ------------ I created RemoveHiddenSubfields for that. It can be used for OPAC and intranet to remove hidden subfields from record (see catalogue/MARCdetail.pl). My purpose is to allow it other calls than OPAC pages. GetMarcBiblio gets 2 more args : $template and $withouthidden. They are optional to avoid modifying intranet calls. I created GetMarcBiblioForOpac that calls GetMarcBiblio with $template='opac' and $withouthidden=1. This method is to explicit the fact that hidden fields are removed at OPAC. GetISBDView already as the $template parameter. I used it to call GetMarcBiblioForOpac if $template='opac'. C4::Record : ------------ CSV export exists at OPAC so marc2csv() gets a new arg : $template. marc2csv is based on marcrecord2csv. I modified its args to use directly modified record and since it is only for C4::Record, I turned it into private : _marcrecord2csv. C4::Search : ------------ In result page, records come from C4::Search::searchResults. So I add RemoveHiddenSubfields if from OPAC. OPAC pages to test : -------------------- I added behavior in all opac pages using GetMarcBiblio : - opac-ISBDdetail.pl - opac-MARCdetail.pl (avoid duplicated behavior) - opac-basket.pl - opac-detail.pl (behavior with and without XSLT) - opac-downloadcart.pl - opac-downloadshelf.pl - opac-export.pl - opac-reserve.pl - opac-search.pl - opac-sendbasket.pl - opac-sendshelf.pl - opac-showmarc.pl - opac-showreviews.pl Other : ------- Note that behavior does not apply to OAI and ILS-DI. I think there is no need. Note that to any subfield can be hidden, many pages use C4::Biblio::GetBiblioData and C4::Items::GetItemsInfo. They get datas from talbles biblio, bilioitems and items. So subfields mapped to a database column can theorically not be hidden. But these datas are very common informations about biblio (title, author, ...), they sould not be hidden. Please feel free to discuss and ask modifications.
Hi, maybe I'm doing a wrong test or do not understand the purpose of the patch. For a given record, MARC view don't show 999, plain view yes, and marcxml export yes too, and it's a hidden field. Are not supposed to be equal? Also koha-qa complains a little: * C4/Biblio.pm FAIL pod FAIL *** ERROR: Spurious text after =cut in file C4/Biblio.pm forbidden patterns OK valid OK critic OK
(In reply to comment #12) > For a given record, MARC view don't show 999, plain view yes, and marcxml > export yes too, and it's a hidden field. > Are not supposed to be equal? Yes, this is the purpose of the patch. Witch hidden value did you use ? Do you try with OPAC ?
My mistake, have wrong hidden values. It works. I was using OPAC.
Now it works, I'm ready to sign. But please, resubmit fixing pod error, or add a followup, since it will not pass QA.
(In reply to comment #15) > But please, resubmit fixing pod error, or add a followup, > since it will not pass QA. I assume this means the status of this bug should be 'Failed QA'
Created attachment 16231 [details] [review] Proposed patch (rebased master) Corrected perl doc problem and rebased master.
Jared: I am wondering if we should look to this report as an enhancement rather than a mere bugfix at this point in time?
Unit tests are mandatory for new routines in C4 and Koha namespaces. Marked as Failed QA.
(In reply to comment #19) > Unit tests are mandatory for new routines in C4 and Koha namespaces. Working on it
Created attachment 18913 [details] [review] Bug 7933: Fields in marc display not the same Small doc correction
Created attachment 18914 [details] [review] Bug 7933: Unit test for C4::Biblio::RemoveHiddenSubfields Whooo, my first unit-test. I hope it is correct.
Created attachment 20880 [details] [review] [SIGNED-OFF] Bug 7933: Fields in marc display not the same Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described. No koha-qa errors Solved minor conflict on applying patch. Test: Edited framework and set a tag to hidden for OPAC Correct behavior on both MARC views, exported record and cart
Created attachment 20881 [details] [review] [SIGNED-OFF] Bug 7933: Unit test for C4::Biblio::RemoveHiddenSubfields Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Run without errors. No koha-qa errors
Created attachment 21018 [details] [review] Bug 7933: QA Followup - Remove unnecessary object creation
Created attachment 21039 [details] [review] Bug 7933: Fields in marc display not the same Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described. No koha-qa errors Solved minor conflict on applying patch. Test: Edited framework and set a tag to hidden for OPAC Correct behavior on both MARC views, exported record and cart Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 21040 [details] [review] Bug 7933: Unit test for C4::Biblio::RemoveHiddenSubfields Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Run without errors. No koha-qa errors Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 21041 [details] [review] Bug 7933: QA Followup - Remove unnecessary object creation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Is there something blocking master integration ?
I prefer the approach that Mark took in bug 11592 (the currently rebase issue aside) of creating a new GetFilteredOpacBiblio rather than changing GetMarcBiblio.
(In reply to Galen Charlton from comment #30) > I prefer the approach that Mark took in bug 11592 (the currently rebase > issue aside) of creating a new GetFilteredOpacBiblio rather than changing > GetMarcBiblio. Ok, bug 11592 is more recent also so I set this one has dublicate *** This bug has been marked as a duplicate of bug 11592 ***