From 1cc9d7f167403cb54038290515f7c51bcc967271 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Mon, 22 Sep 2025 15:15:29 -0300 Subject: [PATCH] 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 --- Koha/ILL/Request.pm | 51 +++++++++++++++++++++++ t/db_dependent/Koha/ILL/Request.t | 68 ++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/Koha/ILL/Request.pm b/Koha/ILL/Request.pm index 879bd354c53..f6892fc7e29 100644 --- a/Koha/ILL/Request.pm +++ b/Koha/ILL/Request.pm @@ -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; diff --git a/t/db_dependent/Koha/ILL/Request.t b/t/db_dependent/Koha/ILL/Request.t index 3e55dc50075..5ea701c8a8c 100755 --- a/t/db_dependent/Koha/ILL/Request.t +++ b/t/db_dependent/Koha/ILL/Request.t @@ -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; +}; -- 2.51.0