Bugzilla – Attachment 186755 Details for
Bug 40850
Add `Koha::ILL::Request->add_or_update_attributes`
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40850: Add `Koha::ILL::Request->add_or_update_attributes`
Bug-40850-Add-KohaILLRequest-addorupdateattributes.patch (text/plain), 6.62 KB, created by
Martin Renvoize (ashimema)
on 2025-09-22 20:55:56 UTC
(
hide
)
Description:
Bug 40850: Add `Koha::ILL::Request->add_or_update_attributes`
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-09-22 20:55:56 UTC
Size:
6.62 KB
patch
obsolete
>From 1cc9d7f167403cb54038290515f7c51bcc967271 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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 <martin.renvoize@openfifth.co.uk> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40850
:
186734
|
186735
|
186736
| 186755 |
186756
|
186757