From 40458900ad5d3f28efe4d5d6c5256606ff0f0a2a 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 --- 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 1cb82a1c5cb..2cea7e61e88 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 3354e24b30e..c049eefb5a3 100644 --- a/installer/data/mysql/en/mandatory/sample_notices.yml +++ b/installer/data/mysql/en/mandatory/sample_notices.yml @@ -1314,10 +1314,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 c7061cc7b96..fef044292ee 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.47.0