Bug 33268: Overlay rules don't work correctly when source is set to *

Fallback to overlay rules with wildcard filter if no match
found for exact filter match.

To test:
1. Administration => record overlay rules
2. Add this:
Module  Filter	                   Tag Preset  Added Appended Removed Deleted
Source  Z39.50 	                   245 Protect Skip  Skip     Skip    Skip
Source  * 	                   300 Protect Skip  Skip     Skip    Skip
Source  Batch record modification  245 Protect Skip  Skip     Skip    Skip
3. Add MARC modification templates:
Update existing or add new field 245$a with value CATSCATSCATSCATS
Update existing or add new field 300$a with value CATSCATSCATSCATS
4. Find a record that has those fields (likely any record would)
5. Use batch record modification on the record
6. 300$a should have been catified (expected, that's the bug)
7. 245$a is preserved
8. Change the value of the 300$a to something else.
   Temporarly disable MARCOverlayRules to be able to do so. And reenable
   it.
9. Apply the patch, restart services
10. Use batch record modification on the record
11. 300$a should be preserved, cat protection should have worked
12. Run tests: prove t/db_dependent/Biblio/MarcOverlayRules.t
13. Celebrate! :D

Sponsored-by: Gothenburg University Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit: tidied inline (tcohen)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit cb2d98fc5b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This commit is contained in:
David Gustafsson 2023-11-14 19:56:32 +01:00 committed by Paul Derscheid
parent 850da7cd4f
commit 6fd5870d85
No known key found for this signature in database
GPG Key ID: C25F9FD44CED595B
1 changed files with 102 additions and 32 deletions

View File

@ -16,13 +16,14 @@ package Koha::MarcOverlayRules;
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use List::Util qw(first);
use List::Util qw(first any);
use Koha::MarcOverlayRule;
use Carp;
use Koha::Exceptions::MarcOverlayRule;
use Try::Tiny;
use Scalar::Util qw(looks_like_number);
use Clone qw(clone);
use parent qw(Koha::Objects);
@ -46,6 +47,16 @@ sub operations {
return ('add', 'append', 'remove', 'delete');
}
=head3 modules
Returns a list of all modules in order of priority.
=cut
sub modules {
return ( 'userid', 'categorycode', 'source' );
}
=head3 context_rules
my $rules = Koha::MarcOverlayRules->context_rules($context);
@ -55,61 +66,120 @@ Gets all MARC overlay rules for the supplied C<$context> (hashref with { module
=cut
sub context_rules {
my ($self, $context) = @_;
my ( $self, $context ) = @_;
return unless %{$context};
my $rules = $cache->get_from_cache('marc_overlay_rules', { unsafe => 1 });
my $rules = $cache->get_from_cache( 'marc_overlay_rules', { unsafe => 1 } );
if (!$rules) {
if ( !$rules ) {
$rules = {};
my @rules_rows = $self->_resultset()->search(
undef,
{
order_by => { -desc => [qw/id/] }
}
{ order_by => { -desc => [qw/id/] } }
);
foreach my $rule_row (@rules_rows) {
my %rule = $rule_row->get_columns();
my %rule = $rule_row->get_columns();
my $operations = {};
foreach my $operation ($self->operations) {
foreach my $operation ( $self->operations ) {
$operations->{$operation} = { allow => $rule{$operation}, rule => $rule{id} };
}
# TODO: Remove unless check and validate on saving rules?
if ($rule{tag} eq '*') {
unless (exists $rules->{$rule{module}}->{$rule{filter}}->{'*'}) {
$rules->{$rule{module}}->{$rule{filter}}->{'*'} = $operations;
if ( $rule{tag} eq '*' ) {
unless ( exists $rules->{ $rule{module} }->{ $rule{filter} }->{'*'} ) {
$rules->{ $rule{module} }->{ $rule{filter} }->{'*'} = $operations;
}
}
elsif ($rule{tag} =~ /^(\d{3})$/) {
unless (exists $rules->{$rule{module}}->{$rule{filter}}->{tags}->{$rule{tag}}) {
$rules->{$rule{module}}->{$rule{filter}}->{tags}->{$rule{tag}} = $operations;
} elsif ( $rule{tag} =~ /^(\d{3})$/ ) {
unless ( exists $rules->{ $rule{module} }->{ $rule{filter} }->{tags}->{ $rule{tag} } ) {
$rules->{ $rule{module} }->{ $rule{filter} }->{tags}->{ $rule{tag} } = $operations;
}
}
else {
my $regexps = ($rules->{$rule{module}}->{$rule{filter}}->{regexps} //= []);
push @{$regexps}, [$rule{tag}, $operations];
} else {
my $regexps = ( $rules->{ $rule{module} }->{ $rule{filter} }->{regexps} //= [] );
push @{$regexps}, [ $rule{tag}, $operations ];
}
}
$cache->set_in_cache('marc_overlay_rules', $rules);
$cache->set_in_cache( 'marc_overlay_rules', $rules );
}
my $context_rules = undef;
foreach my $module_name (keys %{$context}) {
if (
exists $rules->{$module_name} &&
exists $rules->{$module_name}->{$context->{$module_name}}
) {
$context_rules = $rules->{$module_name}->{$context->{$module_name}};
my $context_rules;
my @context_modules = grep { exists $context->{$_} } $self->modules();
foreach my $module_name (@context_modules) {
if ( exists $rules->{$module_name}
&& exists $rules->{$module_name}->{ $context->{$module_name} } )
{
$context_rules = $rules->{$module_name}->{ $context->{$module_name} };
unless ( exists $rules->{$module_name}->{'*'} ) {
# No wildcard filter rules defined
last;
}
# Merge in all wildcard filter rules which has not been overridden
# by the matching context
if ( $context_rules->{'*'} ) {
# Wildcard tag for matching context will override all rules,
# nothing left to do
last;
}
# Clone since we will potentially be modified cached value
# fetched with unsafe => 1
$context_rules = clone($context_rules);
if ( exists $rules->{$module_name}->{'*'}->{'*'} ) {
# Merge in wildcard filter wildcard tag rule if exists
$context_rules->{'*'} = $rules->{$module_name}->{'*'}->{'*'};
}
if ( exists $rules->{$module_name}->{'*'}->{tags} ) {
if ( exists $context_rules->{regexps} ) {
# If the current context has regexp rules, we have to make sure
# to not only to skip tags already present but also those matching
# any of those rules
my @regexps = map { qr/^$_->[0]$/ } @{ $context_rules->{regexps} };
foreach my $tag ( keys %{ $rules->{$module_name}->{'*'}->{tags} } ) {
unless ( ( any { $tag =~ $_ } @regexps ) || exists $context_rules->{tags}->{$tag} ) {
$context_rules->{tags}->{$tag} = $rules->{$module_name}->{'*'}->{tags}->{$tag};
}
}
} else {
# Merge in wildcard filter tag rules not already present
# in the matching context
foreach my $tag ( keys %{ $rules->{$module_name}->{'*'}->{tags} } ) {
unless ( exists $context_rules->{tags}->{$tag} ) {
$context_rules->{tags}->{$tag} = $rules->{$module_name}->{'*'}->{tags}->{$tag};
}
}
}
}
if ( exists $rules->{$module_name}->{'*'}->{regexps} ) {
# Merge in wildcard filter regexp rules last, making sure rules from the
# matching context have precedence
$context_rules->{regexps} //= [];
push @{ $context_rules->{regexps} }, @{ $rules->{$module_name}->{'*'}->{regexps} };
}
last;
}
}
if (!$context_rules) {
# No perms matching specific context conditions found, try wildcard value for each active context
foreach my $module_name (keys %{$context}) {
if (exists $rules->{$module_name}->{'*'}) {
if ( !$context_rules ) {
# No rules matching specific context conditions found, try wildcard value for each active context
foreach my $module_name (@context_modules) {
if ( exists $rules->{$module_name}->{'*'} ) {
$context_rules = $rules->{$module_name}->{'*'};
last;
}