[Bug 15870] New: Add Filter for MARC to respect visibility settings
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Bug ID: 15870 Summary: Add Filter for MARC to respect visibility settings Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org There needs to be a new filter for MARC::Records such that when a record is processed a cloned record with hidden fields and sub-fields excluded. This will only affect MARC::Records, and as such will leave some visibility gaps still. This will only attempt to implement this filter on OPAC pages, though the filter should be usable for the staff client as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15871 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15871 [Bug 15871] Improve perl critic of t/RecordProcessor.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|15871 |15777 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15777 [Bug 15777] Refactor loop in which Record::Processor does not initialize parameters https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15871 [Bug 15871] Improve perl critic of t/RecordProcessor.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15871 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15871 [Bug 15871] Improve perl critic of t/RecordProcessor.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=11592 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Correction, this will ONLY be the filter, and tests for the filter. OPAC stuff will go on bug 11592. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11592 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11592 [Bug 11592] opac detail scripts do not respect MARC tag visibility -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48260&action=edit Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 1) Apply patch 2) prove -v t/db_dependpent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48260|0 |1 is obsolete| | --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48261&action=edit Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 0) Apply 15777 and 15871 as required. 1) Apply patch 2) prove -v t/db_dependpent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. BONUS 4) perlcritic -1 t/db_dependpent/RecordProcessor_ViewPolicy.t -- using the koha qa test tools perlcriticrc, it should pass though I am excluding checking print return values. (see bug 15646) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48261|0 |1 is obsolete| | --- Comment #4 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 48346 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48346&action=edit [SIGNED-OFF]Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 0) Apply 15777 and 15871 as required. 1) Apply patch 2) prove -v t/db_dependent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. BONUS 4) perlcritic -1 t/db_dependpent/RecordProcessor_ViewPolicy.t -- using the koha qa test tools perlcriticrc, it should pass though I am excluding checking print return values. (see bug 15646) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hector.hecaxmmx@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 48628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48628&action=edit Bug 15870: (QA followup) Add tests This patch introduces functionality tests for the new Koha::Filter. It iterates over all the possible 'hidden' values defined in the docs for hiding/showing fields and subfields, and checks the filter does the job. It tests the functionality for both opac and intranet interfaces and tries to catch all scenarios. It adds control fields to the sample record (they traverse a different branch in the code). TODO: Add another subfield to 245, set it to be hidden, and make sure 245 $a is still shown. The original tests file has been renamed conveniently. To test: - Run $ prove t/db_dependent/Filter_MARC_ViewPolicy.t Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This bug was lacking functionality tests. As it is in my best interest to have this moving, I've gone through the process of writing unit tests for this piece of code. Things that weren't tested: - opac vs. intranet (interface option) - test all the possible values. - control fields and datafields traverse different code branches, and needed to be tested. The tests I wrote cover most of what is needed. Please read the TODO in the commit message. The tests fail. Mark: you need to get rid of the clone. The RecordProcessor pipeline is designed so it works on the original object. Doing so will make several tests pass: foreach my $current_record (@records) { - my $result = $current_record->clone(); + my $result = $current_record; There are still problems. Hopefully problems in the tests. I couldn't spend more time on this. Also, there is a weird construct you should get rid off, so you simplify the code and make it more readable: + else { + # visibility is a "level" (-7 to +7), default to 0 + my $visibility = $marcsubfieldstructure->{$tag}->{q{@}}->{hidden}; + $visibility //= 0; + my $hidden; + if ( $display->{$interface}->{$visibility} ) { + $hidden = 0; + } + else { + $hidden = 1; + $result->delete_fields($field); + } + } could be just: + else { + # visibility is a "level" (-7 to +7), default to 0 + my $visibility = $marcsubfieldstructure->{$tag}->{q{@}}->{hidden}; + $visibility //= 0; + if ( ! $display->{$interface}->{$visibility} ) { + $result->delete_fields($field); + } + } Looking forward to your comments and/or followups. Count on me to have this done ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Wow. Much more comprehensive tests. I'll have to look into the clone issue. I can't remember why I had hidden, except to perhaps clarify that a delete was a hide. I'll look more closely at your comments. Though, I just noticed you missed a perldoc change in the package, because you renamed the test file. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Tomás Cohen Arazi from comment #6)
Mark: you need to get rid of the clone. The RecordProcessor pipeline is designed so it works on the original object.
I'm torn on that. %%%% SNIP %%%% sub process { my $self = shift; my $record = shift || $self->record; return unless defined $record; my $newrecord = $record; foreach my $filterobj (@{$self->filters}) { next unless $filterobj; $newrecord = $filterobj->filter($newrecord); } return $newrecord; } %%%% SNIP %%%% The clincher for me is that the $filterobj->filter call returns something. What is the point of returning something if we are modifying the original object? The return $newrecord is still necessary because of pre-call binding or passing the record(s) directly. However, the "$newrecord =" makes no sense if the original was not intended to stay intact, because it would have already been modified! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Progress. GetMarcStructure was pulling cached values. Still working on this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> ---
From 3.10.x online help hidden allows you to select from 19 possible visibility conditions, 17 of which are implemented. They are the following: -9 => Future use -8 => Flag -7 => OPAC !Intranet !Editor Collapsed -6 => OPAC Intranet !Editor !Collapsed -5 => OPAC Intranet !Editor Collapsed -4 => OPAC !Intranet !Editor !Collapsed -3 => OPAC !Intranet Editor Collapsed -2 => OPAC !Intranet Editor !Collapsed -1 => OPAC Intranet Editor Collapsed 0 => OPAC Intranet Editor !Collapsed 1 => !OPAC Intranet Editor Collapsed 2 => !OPAC !Intranet Editor !Collapsed 3 => !OPAC !Intranet Editor Collapsed 4 => !OPAC Intranet Editor !Collapsed 5 => !OPAC !Intranet !Editor Collapsed 6 => !OPAC Intranet !Editor !Collapsed 7 => !OPAC Intranet !Editor Collapsed 8 => !OPAC !Intranet !Editor !Collapsed 9 => Future use ( ! means 'not visible' or in the case of Collapsed 'not Collapsed') This is pre-bug 9894. So cait was right!
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48346|0 |1 is obsolete| | --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48766&action=edit Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 0) Apply 15777 and 15871 as required. 1) Apply patch 2) prove -v t/db_dependpent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. BONUS 4) perlcritic -1 t/db_dependpent/RecordProcessor_ViewPolicy.t -- using the koha qa test tools perlcriticrc, it should pass though I am excluding checking print return values. (see bug 15646) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48628|0 |1 is obsolete| | --- Comment #12 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48767&action=edit Bug 15870: (QA followup) Add tests This patch introduces functionality tests for the new Koha::Filter. It iterates over all the possible 'hidden' values defined in the docs for hiding/showing fields and subfields, and checks the filter does the job. It tests the functionality for both opac and intranet interfaces and tries to catch all scenarios. It adds control fields to the sample record (they traverse a different branch in the code). TODO: Add another subfield to 245, set it to be hidden, and make sure 245 $a is still shown. The original tests file has been renamed conveniently. To test: - Run $ prove t/db_dependent/Filter_MARC_ViewPolicy.t Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48768&action=edit Bug 15870 - Follow-up of filter and tests This patch: - improves perlcritic messages in the filter and tests. - changes should display logic to should hide logic to simplify filter. - perltidies the scripts - debugs the issues outstanding on the comprehensive tests provided in the second commit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #14 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48769&action=edit Bug 15870 - potential follow up to comment #8 This patch: - makes the Koha::RecordProcessor code more clear by removing the unnecessary newrecord variable. - revises the filter to be more clear about the expectation that operations are done directly on the record parameter. TEST PLAN --------- prove t/RecordProcessor.t prove t/db_dependent/Filter_MARC_ViewPolicy.t run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- TEST PLAN --------- apply all but the last patch prove -v t/db_dependent/Filter_MARC_ViewPolicy.t prove -v t/RecordProcessor.t run koha qa test tools apply the last patch prove -v t/db_dependent/Filter_MARC_ViewPolicy.t prove -v t/RecordProcessor.t run koha qa test tools Decide if the last patch is what addresses comment #8. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48766|0 |1 is obsolete| | --- Comment #16 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 48812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48812&action=edit [SIGNED-OFF]Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 0) Apply 15777 and 15871 as required. 1) Apply patch 2) prove -v t/db_dependpent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. BONUS 4) perlcritic -1 t/db_dependpent/RecordProcessor_ViewPolicy.t -- using the koha qa test tools perlcriticrc, it should pass though I am excluding checking print return values. (see bug 15646) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48767|0 |1 is obsolete| | --- Comment #17 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 48813 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48813&action=edit [SIGNED-OFF]Bug 15870: (QA followup) Add tests This patch introduces functionality tests for the new Koha::Filter. It iterates over all the possible 'hidden' values defined in the docs for hiding/showing fields and subfields, and checks the filter does the job. It tests the functionality for both opac and intranet interfaces and tries to catch all scenarios. It adds control fields to the sample record (they traverse a different branch in the code). TODO: Add another subfield to 245, set it to be hidden, and make sure 245 $a is still shown. The original tests file has been renamed conveniently. To test: - Run $ prove t/db_dependent/Filter_MARC_ViewPolicy.t Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48768|0 |1 is obsolete| | --- Comment #18 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 48814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48814&action=edit [SIGNED-OFF]Bug 15870: Follow-up of filter and tests This patch: - improves perlcritic messages in the filter and tests. - changes should display logic to should hide logic to simplify filter. - perltidies the scripts - debugs the issues outstanding on the comprehensive tests provided in the second commit. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48769|0 |1 is obsolete| | --- Comment #19 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 48815 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48815&action=edit [SIGNED-OFF]Bug 15870: potential follow up to comment #8 This patch: - makes the Koha::RecordProcessor code more clear by removing the unnecessary newrecord variable. - revises the filter to be more clear about the expectation that operations are done directly on the record parameter. TEST PLAN --------- prove t/RecordProcessor.t prove t/db_dependent/Filter_MARC_ViewPolicy.t run koha qa test tools Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. NO koha-qa errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 48814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48814 [SIGNED-OFF]Bug 15870: Follow-up of filter and tests Review of attachment 48814: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=15870&attachment=48814) ----------------------------------------------------------------- ::: t/db_dependent/Filter_MARC_ViewPolicy.t @@ +33,2 @@
use Koha::Database; +use English qw/-no_match_vars/;
You are not using regex, it's not needed. @@ +33,4 @@
use Koha::Database; +use English qw/-no_match_vars/; + +$OUTPUT_AUTOFLUSH = 1;
This is not used later. @@ +66,2 @@
+ $schema->storage->txn_begin();
I'd suggest to begin/rollback the transaction in the subtests. @@ +69,5 @@
+ my $update_sql = + q{UPDATE marc_subfield_structure SET hidden=? } + . q{WHERE tagfield='020' OR } + . q{ tagfield='008';}; + my $sth = $dbh->prepare($update_sql);
prepare should be outside of the loop, otherwise there are no needs to prepare it :) @@ +100,2 @@
# Data fields + if ( any { $_ == $hidden_value } @{ $hidden->{$interface} } ) {
You could have used grep, but does not really matter. @@ +152,5 @@
+ my @fields = ( + MARC::Field->new( '003', 'AR-CdUBM' ), + MARC::Field->new( '008', '######suuuu####ag_||||__||||_0||_|_uuu|d' ), + MARC::Field->new( '020', q{}, q{}, 'a' => $isbn ), + MARC::Field->new( '245', q{}, q{}, 'a' => $title ),
Would be good to make it marcflavour independent, but not a requirement yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am fine with modifying the record and not cloning it (better perfs), the caller will have to take care of that if it does want to keep the original record. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #22 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 49013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49013&action=edit Bug 15870: Follow up to address comment #20 - $OUTPUT_AUTOFLUSH wasn't used to it was removed - Use English was removed since $OUTPUT_AUTOFLUSH wasn't not needed - transaction start and end were moved to subtests - prepare was moved outside loop - partial MARC agnosticism was added TEST PLAN --------- 1) apply patch 2) prove t/db_dependent/Filter_MARC_ViewPolicy.t 3) run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49013|0 |1 is obsolete| | --- Comment #23 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 49035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49035&action=edit [SIGNED-OFF]Bug 15870: Follow up to address comment #20 - $OUTPUT_AUTOFLUSH wasn't used to it was removed - Use English was removed since $OUTPUT_AUTOFLUSH wasn't not needed - transaction start and end were moved to subtests - prepare was moved outside loop - partial MARC agnosticism was added TEST PLAN --------- 1) apply patch 2) prove t/db_dependent/Filter_MARC_ViewPolicy.t 3) run koha qa test tools Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 --- Comment #24 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #21)
I am fine with modifying the record and not cloning it (better perfs), the caller will have to take care of that if it does want to keep the original record.
BTW, the 4th patch removes the clone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16101 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48812|0 |1 is obsolete| | --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 49534 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49534&action=edit Bug 15870: MARC Filter to exclude fields/subfields lacking visibility TEST PLAN --------- 0) Apply 15777 and 15871 as required. 1) Apply patch 2) prove -v t/db_dependpent/RecordProcessor_ViewPolicy.t -- all should pass. 3) koha qa test tools. BONUS 4) perlcritic -1 t/db_dependpent/RecordProcessor_ViewPolicy.t -- using the koha qa test tools perlcriticrc, it should pass though I am excluding checking print return values. (see bug 15646) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48813|0 |1 is obsolete| | --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 49535 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49535&action=edit Bug 15870: (QA followup) Add tests This patch introduces functionality tests for the new Koha::Filter. It iterates over all the possible 'hidden' values defined in the docs for hiding/showing fields and subfields, and checks the filter does the job. It tests the functionality for both opac and intranet interfaces and tries to catch all scenarios. It adds control fields to the sample record (they traverse a different branch in the code). TODO: Add another subfield to 245, set it to be hidden, and make sure 245 $a is still shown. The original tests file has been renamed conveniently. To test: - Run $ prove t/db_dependent/Filter_MARC_ViewPolicy.t Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48814|0 |1 is obsolete| | --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 49536 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49536&action=edit Bug 15870: Follow-up of filter and tests This patch: - improves perlcritic messages in the filter and tests. - changes should display logic to should hide logic to simplify filter. - perltidies the scripts - debugs the issues outstanding on the comprehensive tests provided in the second commit. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48815|0 |1 is obsolete| | --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 49537 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49537&action=edit Bug 15870: potential follow up to comment #8 This patch: - makes the Koha::RecordProcessor code more clear by removing the unnecessary newrecord variable. - revises the filter to be more clear about the expectation that operations are done directly on the record parameter. TEST PLAN --------- prove t/RecordProcessor.t prove t/db_dependent/Filter_MARC_ViewPolicy.t run koha qa test tools Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. NO koha-qa errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49035|0 |1 is obsolete| | --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 49538 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49538&action=edit Bug 15870: Follow up to address comment #20 - $OUTPUT_AUTOFLUSH wasn't used to it was removed - Use English was removed since $OUTPUT_AUTOFLUSH wasn't not needed - transaction start and end were moved to subtests - prepare was moved outside loop - partial MARC agnosticism was added TEST PLAN --------- 1) apply patch 2) prove t/db_dependent/Filter_MARC_ViewPolicy.t 3) run koha qa test tools Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |brendan@bywatersolutions.co | |m --- Comment #30 from Brendan Gallagher <brendan@bywatersolutions.com> --- Pushed to Master - Should be in the May 2016 release. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19261 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19261 [Bug 19261] never hide biblionumber field in view policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #31 from David Cook <dcook@prosentient.com.au> --- I just noticed that we're not using this functionality for OPAC search results, which means we're not fully respecting the bibliographic frameworks for the OPAC. I've opened up https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 in regards to this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24458 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 [Bug 24458] OPAC search results don't use Koha::Filter::MARC::ViewPolicy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25019 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25019 [Bug 25019] Non standard initialization in ViewPolicy filter -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org