From 7c5d91b3f0654f40c087fa8e26dc451c5a4adcb6 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 24 Sep 2021 12:02:16 -0300 Subject: [PATCH] Bug 29110: Refactor Koha::ArticleRequest This patchset is simple to test. It adds lots of tests for the existing routines, and then tweaks them. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/ArticleRequest* => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind --- Koha/ArticleRequest.pm | 187 ++++++++++++++------------- Koha/Schema/Result/ArticleRequest.pm | 38 +++++- 2 files changed, 133 insertions(+), 92 deletions(-) diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm index 21b3751ac5..d23a9aee2f 100644 --- a/Koha/ArticleRequest.pm +++ b/Koha/ArticleRequest.pm @@ -27,6 +27,7 @@ use Koha::Items; use Koha::Libraries; use Koha::DateUtils qw( dt_from_string ); use Koha::ArticleRequest::Status; +use Koha::Exceptions; use Koha::Exceptions::ArticleRequest; use base qw(Koha::Object); @@ -39,8 +40,45 @@ Koha::ArticleRequest - Koha Article Request Object class =head2 Class methods +=head3 store + +Overridden I method that performs some checks and triggers. + =cut +sub store { + my ($self) = @_; + + if ( !$self->in_storage ) { + # new request + Koha::Exceptions::MissingParameter->throw( + "Missing mandatory parameter: borrowernumber") + unless $self->borrowernumber; + + my $patron = Koha::Patrons->find( $self->borrowernumber ); + + Koha::Exceptions::Object::FKConstraint->throw( + broken_fk => 'borrowernumber', + value => $self->borrowernumber + ) unless $patron; + + Koha::Exceptions::ArticleRequest::LimitReached->throw( + error => 'Patron cannot request more articles for today' ) + unless $patron->can_request_article; + + $self->created_on( dt_from_string() ); + # this is to avoid calling ->discard_changes to retrieve + # the default value from the DB + $self->status( Koha::ArticleRequest::Status::Requested ) + unless $self->status; + } + + $self->SUPER::store; + $self->notify; + + return $self; +} + =head3 request =cut @@ -48,14 +86,8 @@ Koha::ArticleRequest - Koha Article Request Object class sub request { my ($self) = @_; - Koha::Exceptions::ArticleRequest::LimitReached->throw( - error => 'Patron cannot request more articles for today' - ) unless $self->borrower->can_request_article; - $self->status(Koha::ArticleRequest::Status::Requested); - $self->SUPER::store(); - $self->notify(); - return $self; + return $self->store; } =head3 set_pending @@ -66,9 +98,7 @@ sub set_pending { my ($self) = @_; $self->status(Koha::ArticleRequest::Status::Pending); - $self->SUPER::store(); - $self->notify(); - return $self; + return $self->store; } =head3 process @@ -79,9 +109,7 @@ sub process { my ($self) = @_; $self->status(Koha::ArticleRequest::Status::Processing); - $self->store(); - $self->notify(); - return $self; + return $self->store; } =head3 complete @@ -92,9 +120,7 @@ sub complete { my ($self) = @_; $self->status(Koha::ArticleRequest::Status::Completed); - $self->store(); - $self->notify(); - return $self; + return $self->store; } =head3 cancel @@ -107,59 +133,7 @@ sub cancel { $self->status(Koha::ArticleRequest::Status::Canceled); $self->cancellation_reason($cancellation_reason) if $cancellation_reason; $self->notes($notes) if $notes; - $self->store(); - $self->notify(); - return $self; -} - -=head3 notify - -=cut - -sub notify { - my ($self) = @_; - - my $status = $self->status; - my $reason = $self->notes; - if ( !defined $reason && $self->cancellation_reason ) { - my $av = Koha::AuthorisedValues->search( - { - category => 'AR_CANCELLATION', - authorised_value => $self->cancellation_reason - } - )->next; - $reason = $av->lib_opac ? $av->lib_opac : $av->lib if $av; - } - - require C4::Letters; - if ( - my $letter = C4::Letters::GetPreparedLetter( - module => 'circulation', - letter_code => "AR_$status", # AR_REQUESTED, AR_PENDING, AR_PROCESSING, AR_COMPLETED, AR_CANCELED - message_transport_type => 'email', - lang => $self->borrower->lang, - tables => { - article_requests => $self->id, - borrowers => $self->borrowernumber, - biblio => $self->biblionumber, - biblioitems => $self->biblionumber, - items => $self->itemnumber, - branches => $self->branchcode, - }, - substitute => { - reason => $reason, - }, - ) - ) - { - C4::Letters::EnqueueLetter( - { - letter => $letter, - borrowernumber => $self->borrowernumber, - message_transport_type => 'email', - } - ) or warn "can't enqueue letter " . $letter->{code}; - } + return $self->store; } =head3 biblio @@ -171,9 +145,8 @@ Returns the Koha::Biblio object for this article request sub biblio { my ($self) = @_; - $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); - - return $self->{_biblio}; + my $biblio = $self->_result->biblio; + return Koha::Biblio->_new_from_dbic( $biblio ); } =head3 item @@ -185,9 +158,8 @@ Returns the Koha::Item object for this article request sub item { my ($self) = @_; - $self->{_item} ||= Koha::Items->find( $self->itemnumber() ); - - return $self->{_item}; + my $item = $self->_result->item; + return Koha::Item->_new_from_dbic( $item ); } =head3 borrower @@ -199,9 +171,8 @@ Returns the Koha::Patron object for this article request sub borrower { my ($self) = @_; - $self->{_borrower} ||= Koha::Patrons->find( $self->borrowernumber() ); - - return $self->{_borrower}; + my $patron = $self->_result->patron; + return Koha::Patron->_new_from_dbic( $patron ); } =head3 branch @@ -213,29 +184,63 @@ Returns the Koha::Library object for this article request sub branch { my ($self) = @_; - $self->{_branch} ||= Koha::Libraries->find( $self->branchcode() ); - - return $self->{_branch}; + my $library = $self->_result->library; + return Koha::Library->_new_from_dbic( $library ); } -=head3 store +=head2 Internal methods -Override the default store behavior so that new opac requests -will have notifications sent. +=head3 notify =cut -sub store { +sub notify { my ($self) = @_; - if ( $self->in_storage ) { - return $self->SUPER::store; - } else { - $self->created_on( dt_from_string() ); - return $self->request; + + my $status = $self->status; + my $reason = $self->notes; + if ( !defined $reason && $self->cancellation_reason ) { + my $av = Koha::AuthorisedValues->search( + { + category => 'AR_CANCELLATION', + authorised_value => $self->cancellation_reason + } + )->next; + $reason = $av->lib_opac ? $av->lib_opac : $av->lib if $av; + } + + require C4::Letters; + if ( + my $letter = C4::Letters::GetPreparedLetter( + module => 'circulation', + letter_code => "AR_$status", # AR_REQUESTED, AR_PENDING, AR_PROCESSING, AR_COMPLETED, AR_CANCELED + message_transport_type => 'email', + lang => $self->borrower->lang, + tables => { + article_requests => $self->id, + borrowers => $self->borrowernumber, + biblio => $self->biblionumber, + biblioitems => $self->biblionumber, + items => $self->itemnumber, + branches => $self->branchcode, + }, + substitute => { + reason => $reason, + }, + ) + ) + { + C4::Letters::EnqueueLetter( + { + letter => $letter, + borrowernumber => $self->borrowernumber, + message_transport_type => 'email', + } + ) or warn "can't enqueue letter " . $letter->{code}; } } -=head2 Internal methods + =head3 _type diff --git a/Koha/Schema/Result/ArticleRequest.pm b/Koha/Schema/Result/ArticleRequest.pm index 13748ba8c8..456e53868e 100644 --- a/Koha/Schema/Result/ArticleRequest.pm +++ b/Koha/Schema/Result/ArticleRequest.pm @@ -293,6 +293,42 @@ __PACKAGE__->belongs_to( # Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-08-18 10:41:01 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:1q7Vlki3fRVBdww1zHLCRA +__PACKAGE__->belongs_to( + "biblio", + "Koha::Schema::Result::Biblio", + { biblionumber => "biblionumber" }, + { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" }, +); + +__PACKAGE__->belongs_to( + "item", + "Koha::Schema::Result::Item", + { itemnumber => "itemnumber" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "SET NULL", + on_update => "CASCADE", + }, +); + +__PACKAGE__->belongs_to( + "library", + "Koha::Schema::Result::Branch", + { branchcode => "branchcode" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "SET NULL", + on_update => "CASCADE", + }, +); + +__PACKAGE__->belongs_to( + "patron", + "Koha::Schema::Result::Borrower", + { borrowernumber => "borrowernumber" }, + { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" }, +); -# You can replace this text with custom code or comments, and it will be preserved on regeneration 1; -- 2.20.1