Bugzilla – Attachment 186824 Details for
Bug 40262
ILL - Save the fact that copyright clearance has been confirmed by the patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40262: (follow-up) Refactor copyright clearance to use dedicated methods
Bug-40262-follow-up-Refactor-copyright-clearance-t.patch (text/plain), 7.41 KB, created by
Pedro Amorim
on 2025-09-23 15:40:58 UTC
(
hide
)
Description:
Bug 40262: (follow-up) Refactor copyright clearance to use dedicated methods
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2025-09-23 15:40:58 UTC
Size:
7.41 KB
patch
obsolete
>From 0e18652bce3a5e36d13deb7b809439d1e90c6449 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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.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> >--- > 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 2648ebbc02f..879bd354c53 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 2d36fe08cfa..98064ff6674 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 @@ > <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"> >diff --git a/opac/opac-illrequests.pl b/opac/opac-illrequests.pl >index 0982584c6c3..2c9b1cec87c 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 bc0b2fb9647..3e55dc50075 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.39.5
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 40262
:
183607
|
183662
|
185096
|
186729
|
186730
|
186731
| 186824