Bug 40850: Add `Koha::ILL::Request->add_or_update_attributes`

Currently, ILL backends and plugins must implement custom logic to update
existing extended attributes on ILL requests, as the core extended_attributes()
method only supports adding new attributes and throws duplicate key errors when
attempting to update existing ones. This has led to code duplication across
multiple ILL plugins (Rapido, INNReach, SLNP) that all implement identical
add_or_update_attributes() methods, and forces the Standard backend to use
error-prone patterns when managing request metadata.

This patch adds add_or_update_attributes() method to Koha::ILL::Request that
provides a safe, efficient way to add new attributes or update existing ones
in a single operation, eliminating duplicate key errors and reducing code
duplication.

The method:
- Accepts a hashref of attribute type => value pairs
- Updates existing attributes only when values actually change
- Creates new attributes when they don't exist
- Skips undefined or empty values to avoid constraint errors
- Uses transactions for atomicity
- Returns the request object for method chaining

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/ILL/Request.t
=> SUCCESS: Tests pass! New method works correctly
3. Test adding new attributes works
4. Test updating existing attributes works
5. Test mixed add/update operations work
6. Test empty/undefined values are skipped
7. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Tomás Cohen Arazi 2025-09-22 15:15:29 -03:00 committed by Lucas Gass
parent 059d5d569b
commit 4753b73285
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 118 additions and 1 deletions

View File

@ -1340,6 +1340,57 @@ sub set_copyright_clearance_confirmed {
}
}
=head3 add_or_update_attributes
$request->add_or_update_attributes({
title => 'Book Title',
author => 'Author Name',
isbn => '1234567890'
});
Adds new attributes or updates existing ones. More efficient than extended_attributes()
for updating existing attributes as it avoids duplicate key errors.
=cut
sub add_or_update_attributes {
my ( $self, $attributes ) = @_;
return unless $attributes && ref($attributes) eq 'HASH';
my $schema = $self->_result->result_source->schema;
$schema->txn_do(
sub {
while ( my ( $type, $value ) = each %{$attributes} ) {
# Skip undefined or empty values
next unless defined $value && $value ne '';
my $attr = $self->extended_attributes->find( { type => $type } );
if ($attr) {
# Update existing attribute only if value changed
$attr->update( { value => $value } ) if $attr->value ne $value;
} else {
# Create new attribute
Koha::ILL::Request::Attribute->new(
{
illrequest_id => $self->id,
type => $type,
value => $value,
backend => $self->backend,
}
)->store;
}
}
}
);
return $self;
}
=head3 get_copyright_clearance_confirmed
my $confirmed = $request->get_copyright_clearance_confirmed;

View File

@ -20,7 +20,7 @@
use Modern::Perl;
use Test::NoWarnings;
use Test::More tests => 7;
use Test::More tests => 8;
use Test::MockModule;
use Koha::ILL::Requests;
@ -233,3 +233,69 @@ subtest 'copyright clearance methods tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'add_or_update_attributes() tests' => sub {
plan tests => 13;
$schema->storage->txn_begin;
my $request = $builder->build_object( { class => 'Koha::ILL::Requests' } );
# Test adding new attributes
$request->add_or_update_attributes(
{
title => 'Test Title',
author => 'Test Author',
isbn => '1234567890'
}
);
my $title_attr = $request->extended_attributes->find( { type => 'title' } );
my $author_attr = $request->extended_attributes->find( { type => 'author' } );
my $isbn_attr = $request->extended_attributes->find( { type => 'isbn' } );
ok( $title_attr, 'Title attribute created' );
is( $title_attr->value, 'Test Title', 'Title value set correctly' );
ok( $author_attr, 'Author attribute created' );
is( $author_attr->value, 'Test Author', 'Author value set correctly' );
ok( $isbn_attr, 'ISBN attribute created' );
is( $isbn_attr->value, '1234567890', 'ISBN value set correctly' );
# Test updating existing attributes
$request->add_or_update_attributes(
{
title => 'Updated Title',
author => 'Test Author', # Same value, should not update
year => '2023' # New attribute
}
);
$title_attr->discard_changes;
$author_attr->discard_changes;
my $year_attr = $request->extended_attributes->find( { type => 'year' } );
is( $title_attr->value, 'Updated Title', 'Title attribute updated' );
is( $author_attr->value, 'Test Author', 'Author attribute unchanged when same value' );
ok( $year_attr, 'Year attribute created' );
is( $year_attr->value, '2023', 'Year value set correctly' );
# Test with empty/undefined values (should be skipped)
$request->add_or_update_attributes(
{
empty_field => '',
undef_field => undef,
valid_field => 'valid'
}
);
my $empty_attr = $request->extended_attributes->find( { type => 'empty_field' } );
my $undef_attr = $request->extended_attributes->find( { type => 'undef_field' } );
my $valid_attr = $request->extended_attributes->find( { type => 'valid_field' } );
is( $empty_attr, undef, 'Empty value skipped' );
is( $undef_attr, undef, 'Undefined value skipped' );
ok( $valid_attr, 'Valid value processed' );
$schema->storage->txn_rollback;
};