Bug 40262: (follow-up) Refactor copyright clearance to use dedicated methods

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.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 <tomascohen@theke.io>

PA amended: Test file name in commit message

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Tomás Cohen Arazi 2025-09-22 12:01:24 -03:00 committed by Lucas Gass
parent e8cc5a8d2f
commit 059d5d569b
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
4 changed files with 89 additions and 11 deletions

View File

@ -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();

View File

@ -620,8 +620,7 @@
<div class="page-section">
<h3>Request details</h3>
<h4>Details from library</h4>
[% copyrightclearance_confirmed = request.extended_attributes.find({'type'=>'copyrightclearance_confirmed'}).value %]
[% IF copyrightclearance_confirmed %]
[% IF request.get_copyright_clearance_confirmed %]
<h5> <i class="fa fa-fw fa-check text-success" aria-hidden="true"></i> <span class="tab-title">Patron has confirmed copyright clearance for this request</span> </h5>
[% END %]
<div class="rows">

View File

@ -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} ) {

View File

@ -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;
};