From bd23a729c36da43d7f2709d43e88fd0018fa9075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Mon, 22 Sep 2025 12:01:24 -0300 Subject: [PATCH] Bug 40262: (follow-up) Refactor copyright clearance to use dedicated methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch refactors the copyright clearance confirmation functionality to follow proper separation of concerns by moving business logic from the controller to the model. Changes: - Add set_copyright_clearance_confirmed() method to Koha::ILL::Request - Add get_copyright_clearance_confirmed() method to Koha::ILL::Request - Update controller to use dedicated method instead of direct attribute manipulation - Update staff template to use the new dedicated accessor method - Add comprehensive test coverage for both methods Benefits: - Improved separation of concerns (business logic in model) - Consistency with existing patterns (get_type, get_type_disclaimer_value) - Better reusability and testability - Centralized attribute name management Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/ILL/Request/CopyrightClearance.t => SUCCESS: Tests pass! New methods work correctly 3. Test ILL copyright clearance workflow still works: - Enable ILLModule - Create HTML customization for ILLModuleCopyrightClearance - Create OPAC ILL request with copyright clearance - Verify confirmation is saved and displayed in staff interface 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi --- Koha/ILL/Request.pm | 49 +++++++++++++++++++ .../prog/en/modules/ill/ill-requests.tt | 3 +- opac/opac-illrequests.pl | 10 +--- t/db_dependent/Koha/ILL/Request.t | 38 +++++++++++++- 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/Koha/ILL/Request.pm b/Koha/ILL/Request.pm index 2648ebbc02..879bd354c5 100644 --- a/Koha/ILL/Request.pm +++ b/Koha/ILL/Request.pm @@ -1305,6 +1305,55 @@ sub get_type_disclaimer_value { return $attr->value; } +=head3 set_copyright_clearance_confirmed + + $request->set_copyright_clearance_confirmed(1); + +Sets the copyright clearance confirmation status for the request. + +=cut + +sub set_copyright_clearance_confirmed { + my ( $self, $confirmed ) = @_; + + # Normalize to boolean: 0 or 1 + my $value = $confirmed ? 1 : 0; + + # Check if attribute already exists + my $existing_attr = $self->extended_attributes->find( { type => 'copyrightclearance_confirmed' } ); + + if ($existing_attr) { + + # Update existing attribute + $existing_attr->value($value)->store; + } else { + + # Create new attribute + $self->extended_attributes( + [ + { + type => 'copyrightclearance_confirmed', + value => $value, + } + ] + ); + } +} + +=head3 get_copyright_clearance_confirmed + + my $confirmed = $request->get_copyright_clearance_confirmed; + +Returns 1 if copyright clearance has been confirmed, 0 otherwise. + +=cut + +sub get_copyright_clearance_confirmed { + my ($self) = @_; + my $attr = $self->extended_attributes->find( { type => 'copyrightclearance_confirmed' } ); + return $attr ? ( $attr->value ? 1 : 0 ) : 0; +} + =head3 get_type_disclaimer_date my $type = $abstract->type_disclaimer_date(); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt index 2d36fe08cf..98064ff667 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ -620,8 +620,7 @@

Request details

Details from library

- [% copyrightclearance_confirmed = request.extended_attributes.find({'type'=>'copyrightclearance_confirmed'}).value %] - [% IF copyrightclearance_confirmed %] + [% IF request.get_copyright_clearance_confirmed %]
Patron has confirmed copyright clearance for this request
[% END %]
diff --git a/opac/opac-illrequests.pl b/opac/opac-illrequests.pl index 0982584c6c..2c9b1cec87 100755 --- a/opac/opac-illrequests.pl +++ b/opac/opac-illrequests.pl @@ -200,14 +200,8 @@ if ( $op eq 'list' ) { ); if ( $backend_result->{stage} eq 'commit' ) { - $request->extended_attributes( - [ - { - type => 'copyrightclearance_confirmed', - value => 1, - } - ] - ) if $params->{copyrightclearance_confirmed}; + $request->set_copyright_clearance_confirmed(1) + if $params->{copyrightclearance_confirmed}; # After creation actions if ( $params->{type_disclaimer_submitted} ) { diff --git a/t/db_dependent/Koha/ILL/Request.t b/t/db_dependent/Koha/ILL/Request.t index bc0b2fb964..3e55dc5007 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 => 6; +use Test::More tests => 7; use Test::MockModule; use Koha::ILL::Requests; @@ -197,3 +197,39 @@ subtest 'get_backend_plugin() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'copyright clearance methods tests' => sub { + + plan tests => 8; + + $schema->storage->txn_begin; + + # Test set_copyright_clearance_confirmed with truthy value + my $request = $builder->build_object( { class => 'Koha::ILL::Requests' } ); + + $request->set_copyright_clearance_confirmed(1); + + my $attr = $request->extended_attributes->find( { type => 'copyrightclearance_confirmed' } ); + ok( $attr, 'Copyright clearance attribute created' ); + is( $attr->value, 1, 'Copyright clearance value set to 1' ); + + # Test setting to false creates attribute with value 0 + my $request2 = $builder->build_object( { class => 'Koha::ILL::Requests' } ); + $request2->set_copyright_clearance_confirmed(0); + + my $attr2 = $request2->extended_attributes->find( { type => 'copyrightclearance_confirmed' } ); + ok( $attr2, 'Attribute created for false value' ); + is( $attr2->value, 0, 'False value normalized to 0' ); + + # Test setting to false when already true updates the value + $request->set_copyright_clearance_confirmed(0); + my $attr_after_false = $request->extended_attributes->find( { type => 'copyrightclearance_confirmed' } ); + ok( $attr_after_false, 'Attribute still exists after setting to false' ); + is( $attr_after_false->value, 0, 'Attribute value updated to 0 when set to false' ); + + # Test get_copyright_clearance_confirmed returns boolean values + is( $request->get_copyright_clearance_confirmed, 0, 'Returns 0 when set to false' ); + is( $request2->get_copyright_clearance_confirmed, 0, 'Returns 0 when set to false value' ); + + $schema->storage->txn_rollback; +}; -- 2.51.0