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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Martin Renvoize 2024-10-23 14:26:39 +01:00 committed by Katrin Fischer
parent 85ba989f82
commit e08797d51e
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
3 changed files with 30 additions and 22 deletions

View File

@ -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',
);

View File

@ -1338,10 +1338,11 @@ tables:
message_transport_type: email
lang: default
content:
- "Dear [%- INCLUDE 'patron-title.inc' patron => new_owner -%],<br>"
- "Dear [%- INCLUDE 'patron-title.inc' patron => owner -%],<br>"
- "<br>"
- "A public list, titled <em>[% shelf.shelfname | html %]</em>, has been transferred to you[% IF ( old_owner ) %] by [%- INCLUDE 'patron-title.inc' patron => old_owner%][% IF ( old_owner.email ) %] <em>([% old_owner.email | html %])</em>[% END %][% END %].<br>"
- "A public list, titled <em>[% shelf.shelfname | html %]</em>, has been transferred to you[% IF ( librarian ) %] by [%- INCLUDE 'patron-title.inc' patron => librarian %][% END %].<br>"
- "<br>"
- "[% IF old_owner %]The list was previously owned by [%- INCLUDE 'patron-title.inc' patron => old_owner -%].<br><br>[% END %]"
- "Thank you<br>"
- "<br>"

View File

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