From e08797d51e4cf4fee026b94ee1b4f9218f65c6ba Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 23 Oct 2024 14:26:39 +0100 Subject: [PATCH] Bug 30955: (QA follow-up) Reduce database hits and clarify notice This patch updates the library selection from using the new owners home library to using the current sessions library in keeping with the direction of travel in other bugs of this type in Koha currently. We also update the default notice text to clarify who sent the notice vs who previously owned the list. Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- Koha/Virtualshelf.pm | 15 ++++----- .../mysql/en/mandatory/sample_notices.yml | 5 +-- t/db_dependent/Koha/Virtualshelf.t | 32 ++++++++++++------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 1cb82a1c5c..2cea7e61e8 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -361,9 +361,10 @@ sub transfer_ownership { unless $patron_id; ## before we change the owner, collect some details - my $old_owner = Koha::Patrons->find( $self->owner ); - my $new_owner = Koha::Patrons->find($patron_id); - my $library = $new_owner->library->unblessed; + my $old_owner = Koha::Patrons->find( $self->owner ); + my $new_owner = Koha::Patrons->find($patron_id); + my $userenv = C4::Context->userenv; + my $branchcode = $userenv->{branch}; ## first we change the owner $self->remove_share($patron_id) if $self->is_private; @@ -373,16 +374,14 @@ sub transfer_ownership { my $letter = C4::Letters::GetPreparedLetter( module => 'lists', letter_code => 'TRANSFER_OWNERSHIP', - branchcode => $library->{branchcode}, + branchcode => $branchcode, lang => $new_owner->lang || 'default', objects => { old_owner => $old_owner, - new_owner => $new_owner, + owner => $new_owner, shelf => $self, }, - tables => { - 'branches' => $library->{branchcode}, - }, + want_librarian => 1, message_transport_type => 'email', ); diff --git a/installer/data/mysql/en/mandatory/sample_notices.yml b/installer/data/mysql/en/mandatory/sample_notices.yml index 3ca1a9aea5..25ec93fbd8 100644 --- a/installer/data/mysql/en/mandatory/sample_notices.yml +++ b/installer/data/mysql/en/mandatory/sample_notices.yml @@ -1338,10 +1338,11 @@ tables: message_transport_type: email lang: default content: - - "Dear [%- INCLUDE 'patron-title.inc' patron => new_owner -%],
" + - "Dear [%- INCLUDE 'patron-title.inc' patron => owner -%],
" - "
" - - "A public list, titled [% shelf.shelfname | html %], has been transferred to you[% IF ( old_owner ) %] by [%- INCLUDE 'patron-title.inc' patron => old_owner%][% IF ( old_owner.email ) %] ([% old_owner.email | html %])[% END %][% END %].
" + - "A public list, titled [% shelf.shelfname | html %], has been transferred to you[% IF ( librarian ) %] by [%- INCLUDE 'patron-title.inc' patron => librarian %][% END %].
" - "
" + - "[% IF old_owner %]The list was previously owned by [%- INCLUDE 'patron-title.inc' patron => old_owner -%].

[% END %]" - "Thank you
" - "
" diff --git a/t/db_dependent/Koha/Virtualshelf.t b/t/db_dependent/Koha/Virtualshelf.t index c7061cc7b9..fef044292e 100755 --- a/t/db_dependent/Koha/Virtualshelf.t +++ b/t/db_dependent/Koha/Virtualshelf.t @@ -34,9 +34,9 @@ subtest 'transfer_ownership() tests' => sub { $schema->storage->txn_begin; - my $patron_1 = $builder->build_object({ class => 'Koha::Patrons' }); - my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); - my $patron_3 = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron_2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron_3 = $builder->build_object( { class => 'Koha::Patrons' } ); my $public_list = $builder->build_object( { @@ -52,9 +52,8 @@ subtest 'transfer_ownership() tests' => sub { } ); - throws_ok - { $public_list->transfer_ownership } - 'Koha::Exceptions::MissingParameter', + throws_ok { $public_list->transfer_ownership } + 'Koha::Exceptions::MissingParameter', 'Exception thrown if missing parameter'; like( "$@", qr/Mandatory parameter 'patron' missing/, 'Exception string as expected' ); @@ -63,7 +62,7 @@ subtest 'transfer_ownership() tests' => sub { $builder->build_object( { class => 'Koha::Virtualshelfshares', - value => { shelfnumber => $public_list->id, invitekey => undef, borrowernumber => $patron_2->id } + value => { shelfnumber => $public_list->id, invitekey => undef, borrowernumber => $patron_2->id } } ); $builder->build_object( @@ -84,7 +83,7 @@ subtest 'transfer_ownership() tests' => sub { is( $public_list->owner, $patron_2->id, 'Owner changed correctly' ); my $public_list_shares = $public_list->get_shares; - is( $public_list_shares->count, 1, 'Count is correct' ); + is( $public_list_shares->count, 1, 'Count is correct' ); is( $public_list_shares->next->borrowernumber, $patron_2->id, "Public lists don't get the share removed" ); $private_list->transfer_ownership( $patron_2->id ); @@ -93,7 +92,10 @@ subtest 'transfer_ownership() tests' => sub { is( $private_list->owner, $patron_2->id ); my $private_list_shares = $private_list->get_shares; is( $private_list_shares->count, 1, 'Count is correct' ); - is( $private_list_shares->next->borrowernumber, $patron_3->id, "Private lists get the share for the new owner removed" ); + is( + $private_list_shares->next->borrowernumber, $patron_3->id, + "Private lists get the share for the new owner removed" + ); my %params; my $mocked_letters = Test::MockModule->new('C4::Letters'); @@ -114,10 +116,16 @@ subtest 'transfer_ownership() tests' => sub { $public_list->transfer_ownership( $patron_1->id ); $public_list->discard_changes; - is( $params{module}, "lists", "Enqueued letter with module lists correctly" ); + is( $params{module}, "lists", "Enqueued letter with module lists correctly" ); is( $params{letter_code}, "TRANSFER_OWNERSHIP", "Enqueued letter with code TRANSFER_OWNERSHIP correctly" ); - is( $params{objects}->{old_owner}->borrowernumber, $patron_2->borrowernumber, "old_owner passed to enqueue letter correctly" ); - is( $params{objects}->{new_owner}->borrowernumber, $patron_1->borrowernumber, "new_owner passed to enqueue letter correctly" ); + is( + $params{objects}->{old_owner}->borrowernumber, $patron_2->borrowernumber, + "old_owner passed to enqueue letter correctly" + ); + is( + $params{objects}->{owner}->borrowernumber, $patron_1->borrowernumber, + "owner passed to enqueue letter correctly" + ); is( $params{objects}->{shelf}->shelfnumber, $public_list->shelfnumber, "shelf passed to enqueue letter correctly" ); $schema->storage->txn_rollback; -- 2.39.5