From ef08a843d4ec5d3aa614510f006f91e4cf0a21d8 Mon Sep 17 00:00:00 2001 From: Jake Deery Date: Mon, 2 Mar 2026 16:28:18 +0000 Subject: [PATCH] Bug 37346: Koha::VirtualShelf::owner is now an accessor This patch updates the DB field `virtualshelves`.`owner` to be `virtualshelves`.`owner_id`, and adds an accessor method in place of owner. The rationale behind this patch is that, in future patches, we can begin to reduce repeated calls of Koha::Patrons->find by providing an accessor for the aforementioned damp code. TO TEST: == APPLY PATCH == b) ensure the full test harness works, by running k.t.d with RUN_TESTS_AND_EXIT=yes in your .env file c) ensure ALL create, read, update, and delete functions work under the lists module on the staff client & OPAC (aka TEST EVERYTHING) d) ensure you can add to lists from the search on the staff client & OPAC e) ensure you can email lists from the send lists button on the staff client & OPAC f) create a list with at least one biblio inside g) enable the ERMModule system preference 1) goto the ERM module 2) create a package called package 3) under titles, select import from list 4) select package in the dropdown, and click import next to your list h) ensure a biblio is listed under titles in the ERMModule == SIGN OFF == --- C4/Utils/DataTables/VirtualShelves.pm | 10 +-- Koha/BackgroundJob/BatchUpdateBiblio.pm | 2 +- Koha/Exceptions/Virtualshelf.pm | 2 +- Koha/Patron.pm | 2 +- Koha/REST/V1/ERM/EHoldings/Titles/Local.pm | 2 +- Koha/REST/V1/Lists.pm | 2 +- Koha/Virtualshelf.pm | 48 ++++++++----- Koha/Virtualshelves.pm | 16 ++--- catalogue/detail.pl | 4 +- installer/data/mysql/kohastructure.sql | 6 +- opac/opac-addbybiblionumber.pl | 12 ++-- opac/opac-detail.pl | 4 +- opac/opac-shareshelf.pl | 4 +- opac/opac-shelves.pl | 6 +- t/db_dependent/Koha/Patrons.t | 28 ++++---- t/db_dependent/Koha/Virtualshelf.t | 8 +-- t/db_dependent/Koha/Virtualshelves.t | 40 +++++------ .../Utils/Datatables_Virtualshelves.t | 13 ++-- t/db_dependent/Virtualshelves.t | 67 ++++++++++--------- t/db_dependent/api/v1/public/lists.t | 12 ++-- t/db_dependent/selenium/regressions.t | 2 +- tools/batch_delete_records.pl | 2 +- tools/batch_record_modification.pl | 2 +- virtualshelves/addbybiblionumber.pl | 6 +- virtualshelves/shelves.pl | 4 +- 25 files changed, 165 insertions(+), 139 deletions(-) diff --git a/C4/Utils/DataTables/VirtualShelves.pm b/C4/Utils/DataTables/VirtualShelves.pm index 1bbfc40d30d..8cebc33e51c 100644 --- a/C4/Utils/DataTables/VirtualShelves.pm +++ b/C4/Utils/DataTables/VirtualShelves.pm @@ -27,7 +27,7 @@ sub search { # FIXME refactore the following queries # We should call Koha::Virtualshelves my $select = q| - SELECT vs.shelfnumber, vs.shelfname, vs.owner, vs.public AS public, + SELECT vs.shelfnumber, vs.shelfname, vs.owner_id, vs.public AS public, vs.created_on, vs.lastmodified as modification_time, bo.surname, bo.firstname, vs.sortfield as sortby, count(vc.biblionumber) as count @@ -35,7 +35,7 @@ sub search { my $from_total = q| FROM virtualshelves vs - LEFT JOIN borrowers bo ON vs.owner=bo.borrowernumber + LEFT JOIN borrowers bo ON vs.owner_id=bo.borrowernumber |; my $from = $from_total . q| @@ -86,7 +86,7 @@ sub search { push @args, $public; if ( !$public ) { - push @where_strs, '(vs.owner = ? OR sh.borrowernumber = ?)'; + push @where_strs, '(vs.owner_id = ? OR sh.borrowernumber = ?)'; push @args, $loggedinuser, $loggedinuser; } @@ -120,7 +120,7 @@ sub search { $limit = sprintf "LIMIT %s,%s", $start, $length; } - my $group_by = " GROUP BY vs.shelfnumber, vs.shelfname, vs.owner, vs.public, + my $group_by = " GROUP BY vs.shelfnumber, vs.shelfname, vs.owner_id, vs.public, vs.created_on, vs.lastmodified, bo.surname, bo.firstname, vs.sortfield "; my $query = join( @@ -140,7 +140,7 @@ sub search { # Get the recordsTotal DataTable variable $query = q|SELECT COUNT(vs.shelfnumber)| . $from_total . q| WHERE public = ?|; - $query .= q| AND (vs.owner = ? OR sh.borrowernumber = ?)| if !$public; + $query .= q| AND (vs.owner_id = ? OR sh.borrowernumber = ?)| if !$public; @args = !$public ? ( $loggedinuser, $public, $loggedinuser, $loggedinuser ) : ($public); ($recordsTotal) = $dbh->selectrow_array( $query, undef, @args ); diff --git a/Koha/BackgroundJob/BatchUpdateBiblio.pm b/Koha/BackgroundJob/BatchUpdateBiblio.pm index bbd4a7a5453..85f21622029 100644 --- a/Koha/BackgroundJob/BatchUpdateBiblio.pm +++ b/Koha/BackgroundJob/BatchUpdateBiblio.pm @@ -172,7 +172,7 @@ sub additional_report { return { lists => Koha::Virtualshelves->search( [ - { public => 0, owner => $loggedinuser }, + { public => 0, owner_id => $loggedinuser }, { public => 1 } ] ), diff --git a/Koha/Exceptions/Virtualshelf.pm b/Koha/Exceptions/Virtualshelf.pm index 40bc0dc6553..5e8313d59e7 100644 --- a/Koha/Exceptions/Virtualshelf.pm +++ b/Koha/Exceptions/Virtualshelf.pm @@ -74,7 +74,7 @@ Exception to be used when a share has expired. =head2 Koha::Exceptions::Virtualshelf::UseDbAdminAccount -Exception to be used when the owner is not set. +Exception to be used when the owner_id is not set. =cut diff --git a/Koha/Patron.pm b/Koha/Patron.pm index c0ef6e292c5..556d10adc0d 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -103,7 +103,7 @@ our $RESULTSET_PATRON_ID_MAPPING = { TagAll => 'borrowernumber', Virtualshelfcontent => 'borrowernumber', Virtualshelfshare => 'borrowernumber', - Virtualshelve => 'owner', + Virtualshelve => 'owner_id', }; =head1 NAME diff --git a/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm b/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm index 9041cc43a77..6372058bad3 100644 --- a/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm +++ b/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm @@ -227,7 +227,7 @@ sub import_from_list { my $list = Koha::Virtualshelves->find($list_id); my $patron = $c->stash('koha.user'); - unless ( $list && $list->owner == $c->stash('koha.user')->borrowernumber ) { + unless ( $list && $list->owner_id == $c->stash('koha.user')->borrowernumber ) { return $c->render_resource_not_found("List"); } diff --git a/Koha/REST/V1/Lists.pm b/Koha/REST/V1/Lists.pm index 560a23010f9..86cd9526ec3 100644 --- a/Koha/REST/V1/Lists.pm +++ b/Koha/REST/V1/Lists.pm @@ -56,7 +56,7 @@ sub list_public { my $lists_set = Koha::Virtualshelves->new; if ($only_mine) { - $lists_set = $lists_set->search( { owner => $user->id } ); + $lists_set = $lists_set->search( { owner_id => $user->id } ); } if ( $only_public || !$user ) { diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 339351b6226..743ef38942c 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -50,7 +50,7 @@ Missing POD for store. sub store { my ($self) = @_; - unless ( $self->owner ) { + unless ( $self->owner_id ) { Koha::Exceptions::Virtualshelf::UseDbAdminAccount->throw; } @@ -109,15 +109,15 @@ sub is_shelfname_valid { ( $self->shelfnumber ? ( "me.shelfnumber" => { '!=', $self->shelfnumber } ) : () ), }; - if ( $self->is_private and defined $self->owner ) { + if ( $self->is_private and defined $self->owner_id ) { $conditions->{-or} = { - "virtualshelfshares.borrowernumber" => $self->owner, - "me.owner" => $self->owner, + "virtualshelfshares.borrowernumber" => $self->owner_id, + "me.owner_id" => $self->owner_id, }; $conditions->{public} = 0; - } elsif ( $self->is_private and not defined $self->owner ) { - $conditions->{owner} = undef; - $conditions->{public} = 0; + } elsif ( $self->is_private and not defined $self->owner_id ) { + $conditions->{owner_id} = undef; + $conditions->{public} = 0; } else { $conditions->{public} = 1; } @@ -247,7 +247,7 @@ sub add_biblio { # Check permissions my $patron = Koha::Patrons->find($borrowernumber) or return 0; return 0 - unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) + unless ( $self->owner_id == $borrowernumber && $self->allow_change_from_owner ) || ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) || ( $self->allow_change_from_permitted_staff && $patron->can_patron_change_permitted_staff_lists ) || $self->allow_change_from_others; @@ -279,7 +279,7 @@ sub remove_biblios { my $number_removed = 0; my $patron = Koha::Patrons->find($borrowernumber) or return 0; - if ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) + if ( ( $self->owner_id == $borrowernumber && $self->allow_change_from_owner ) || ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) || ( $self->allow_change_from_permitted_staff && $patron->can_patron_change_permitted_staff_lists ) || $self->allow_change_from_others ) @@ -303,7 +303,7 @@ sub can_be_viewed { my ( $self, $borrowernumber ) = @_; return 1 if $self->is_public; return 0 unless $borrowernumber; - return 1 if $self->owner == $borrowernumber; + return 1 if $self->owner_id == $borrowernumber; return $self->get_shares->search( { borrowernumber => $borrowernumber, @@ -321,7 +321,7 @@ sub can_be_deleted { my ( $self, $borrowernumber ) = @_; return 0 unless $borrowernumber; - return 1 if $self->owner == $borrowernumber; + return 1 if $self->owner_id == $borrowernumber; my $patron = Koha::Patrons->find($borrowernumber) or return 0; @@ -339,7 +339,7 @@ Missing POD for can_be_managed. sub can_be_managed { my ( $self, $borrowernumber ) = @_; return 1 - if $borrowernumber and $self->owner == $borrowernumber; + if $borrowernumber and $self->owner_id == $borrowernumber; my $patron = Koha::Patrons->find($borrowernumber) or return 0; return 1 @@ -359,7 +359,7 @@ sub can_biblios_be_added { my $patron = Koha::Patrons->find($borrowernumber) or return 0; return 1 if $borrowernumber - and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) + and ( ( $self->owner_id == $borrowernumber && $self->allow_change_from_owner ) or ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) or ( $self->allow_change_from_permitted_staff && $patron->can_patron_change_permitted_staff_lists ) or $self->allow_change_from_others ) @@ -461,6 +461,22 @@ sub cannot_be_transferred { return 0; # serving as green light } +=head3 owner + +my $patron = Koha::Virtualshelf->owner; + +Return the Koha::Patron object representing the owner_id + +=cut + +sub owner { + my ($self) = @_; + my $patron_rs = $self->_result->owner_id; + return unless $patron_rs; + + return Koha::Patrons->find($patron_rs); +} + =head3 transfer_ownership $list->transfer_ownership( $patron_id ); @@ -476,14 +492,14 @@ sub transfer_ownership { unless $patron_id; ## before we change the owner, collect some details - my $old_owner = Koha::Patrons->find( $self->owner ); + my $old_owner = Koha::Patrons->find( $self->owner_id ); 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; - $self->set( { owner => $patron_id } )->store; + $self->set( { owner_id => $patron_id } )->store; ## now we message the new owner my $letter = C4::Letters::GetPreparedLetter( @@ -524,7 +540,7 @@ sub to_api_mapping { return { created_on => 'creation_date', lastmodified => 'updated_on_date', - owner => 'owner_id', + owner_id => 'owner_id', shelfname => 'name', shelfnumber => 'list_id', sortfield => 'default_sort_field', diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm index 8546d0ac1f7..12923619f65 100644 --- a/Koha/Virtualshelves.pm +++ b/Koha/Virtualshelves.pm @@ -95,7 +95,7 @@ sub get_private_shelves { public => 0, -or => { 'virtualshelfshares.borrowernumber' => $borrowernumber, - 'me.owner' => $borrowernumber, + 'me.owner_id' => $borrowernumber, } }, { @@ -180,7 +180,7 @@ sub get_some_shelves { push @conditions, { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -192,7 +192,7 @@ sub get_some_shelves { push @conditions, { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -203,7 +203,7 @@ sub get_some_shelves { push @conditions, { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -215,7 +215,7 @@ sub get_some_shelves { push @conditions, { -or => { "virtualshelfshares.borrowernumber" => $borrowernumber, - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, } }; } @@ -250,8 +250,8 @@ sub get_shelves_containing_record { { public => 0, -or => { - 'me.owner' => $borrowernumber, - -or => { + 'me.owner_id' => $borrowernumber, + -or => { 'virtualshelfshares.borrowernumber' => $borrowernumber, }, } @@ -301,7 +301,7 @@ sub filter_by_readable { Koha::Exceptions::MissingParameter->throw("Mandatory patron_id parameter missing") unless $params->{patron_id}; - return $self->search( { '-or' => { public => 1, owner => $params->{patron_id} } } ); + return $self->search( { '-or' => { public => 1, owner_id => $params->{patron_id} } } ); } =head2 Internal methods diff --git a/catalogue/detail.pl b/catalogue/detail.pl index d6ff6220315..cfd8c4d6725 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -452,8 +452,8 @@ if ( C4::Context->preference("virtualshelves") ) { { biblionumber => $biblionumber, '-or' => { - public => 1, - owner => $patron->borrowernumber + public => 1, + owner_id => $patron->borrowernumber } }, { diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index a29c7e49292..904fd714399 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -7047,7 +7047,7 @@ DROP TABLE IF EXISTS `virtualshelves`; CREATE TABLE `virtualshelves` ( `shelfnumber` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', `shelfname` varchar(255) DEFAULT NULL COMMENT 'name of the list', - `owner` int(11) DEFAULT NULL COMMENT 'foreign key linking to the borrowers table (using borrowernumber) for the creator of this list (changed from varchar(80) to int)', + `owner_id` int(11) DEFAULT NULL COMMENT 'foreign key linking to the borrowers table (using borrowernumber) for the creator of this list (changed from varchar(80) to int)', `public` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'If the list is public', `sortfield` varchar(16) DEFAULT 'title' COMMENT 'the field this list is sorted on', `lastmodified` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp() COMMENT 'date and time the list was last modified', @@ -7057,8 +7057,8 @@ CREATE TABLE `virtualshelves` ( `allow_change_from_staff` tinyint(1) DEFAULT 0 COMMENT 'can staff change contents?', `allow_change_from_permitted_staff` tinyint(1) DEFAULT 0 COMMENT 'can staff with edit_public_list_contents permission change contents?', PRIMARY KEY (`shelfnumber`), - KEY `virtualshelves_ibfk_1` (`owner`), - CONSTRAINT `virtualshelves_ibfk_1` FOREIGN KEY (`owner`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL + KEY `virtualshelves_ibfk_1` (`owner_id`), + CONSTRAINT `virtualshelves_ibfk_1` FOREIGN KEY (`owner_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index 368b8f17682..6222b6ab338 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/opac/opac-addbybiblionumber.pl @@ -66,7 +66,7 @@ if ( $op && $op !~ /^cud-/ ) { ) { my $shelf = eval { - Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, public => $public, owner => $loggedinuser, } ) + Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, public => $public, owner_id => $loggedinuser, } ) ->store; }; if ( $@ or not $shelf ) { @@ -112,7 +112,7 @@ if ( $op && $op !~ /^cud-/ ) { my $private_shelves = Koha::Virtualshelves->search( { public => 0, - owner => $loggedinuser, + owner_id => $loggedinuser, allow_change_from_owner => 1, }, { order_by => 'shelfname' } @@ -134,7 +134,7 @@ if ( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, allow_change_from_staff => 1, @@ -150,7 +150,7 @@ if ( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, allow_change_from_staff => 1 @@ -165,7 +165,7 @@ if ( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, ], @@ -180,7 +180,7 @@ if ( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, ], diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index edf609cb021..508c47d2af1 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -953,8 +953,8 @@ if ( C4::Context->preference("virtualshelves") ) { { biblionumber => $biblionumber, '-or' => { - public => 1, - owner => $borrowernumber + public => 1, + owner_id => $borrowernumber } }, { diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl index 812b61708d2..fb8dcce8a31 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -88,7 +88,7 @@ sub _init { my $shelfnumber = $param->{shelfnumber}; $shelf = Koha::Virtualshelves->find($shelfnumber) unless $param->{errcode}; $param->{shelfname} = $shelf ? $shelf->shelfname : q||; - $param->{owner} = $shelf ? $shelf->owner : -1; + $param->{owner} = $shelf ? $shelf->owner_id : -1; $param->{public} = $shelf ? $shelf->public : 0; return $param; @@ -154,7 +154,7 @@ sub handle_accept { $param->{errcode} = 2; } elsif ( $shelf->public ) { $param->{errcode} = 5; - } elsif ( $shelf->owner == $loggedinuser ) { + } elsif ( $shelf->owner_id == $loggedinuser ) { $param->{errcode} = 8; } return if $param->{errcode}; diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index bd908db01e2..7bad3e268dc 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -108,7 +108,7 @@ if ( $op eq 'add_form' ) { if ($shelf) { $public = $shelf->public; - my $patron = Koha::Patrons->find( $shelf->owner ); + my $patron = Koha::Patrons->find( $shelf->owner_id ); $template->param( owner => $patron, ); unless ( $shelf->can_be_managed($loggedinuser) ) { push @messages, { type => 'error', code => 'unauthorized_on_update' }; @@ -130,7 +130,7 @@ if ( $op eq 'add_form' ) { allow_change_from_others => $allow_changes_from == ANYONE, allow_change_from_staff => $allow_changes_from == STAFF, allow_change_from_permitted_staff => $allow_changes_from == PERMITTED, - owner => scalar $loggedinuser, + owner_id => scalar $loggedinuser, } ); $shelf->store; @@ -303,7 +303,7 @@ if ( $op eq 'add_form' ) { Koha::Virtualshelfshare->new( { shelfnumber => $shelfnumber, borrowernumber => $loggedinuser, sharedate => dt_from_string } ) ->store; - $shelf->owner($new_owner)->store; + $shelf->owner_id($new_owner)->store; } ); $op = 'list'; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 16ef2b22080..45db3cab9df 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -590,21 +590,21 @@ subtest "delete" => sub { my $private_list = Koha::Virtualshelf->new( { shelfname => "private", - owner => $patron->borrowernumber, + owner_id => $patron->borrowernumber, public => 0, } )->store; my $public_list = Koha::Virtualshelf->new( { shelfname => "public", - owner => $patron->borrowernumber, + owner_id => $patron->borrowernumber, public => 1, } )->store; my $list_to_share = Koha::Virtualshelf->new( { shelfname => "shared", - owner => $patron->borrowernumber, + owner_id => $patron->borrowernumber, public => 0, } )->store; @@ -631,7 +631,7 @@ subtest "delete" => sub { q|Koha::Patron->delete should have cancelled patron's holds 2| ); - my $transferred_lists = Koha::Virtualshelves->search( { owner => $staff_patron->borrowernumber } )->count; + my $transferred_lists = Koha::Virtualshelves->search( { owner_id => $staff_patron->borrowernumber } )->count; is( $transferred_lists, 2, 'Public and shared lists should stay in database under a different owner with a unique name, while private lists delete, with ListOwnershipPatronDeletion set to Transfer' @@ -645,7 +645,7 @@ subtest "delete" => sub { "But the other share is still there" ); is( - Koha::Virtualshelves->search( { owner => $patron->borrowernumber } )->count, 0, + Koha::Virtualshelves->search( { owner_id => $patron->borrowernumber } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists/removed their ownership| ); @@ -664,14 +664,14 @@ subtest "delete" => sub { my $designated_owner = $builder->build_object( { class => 'Koha::Patrons' } ); t::lib::Mocks::mock_preference( 'ListOwnerDesignated', $designated_owner->id ); $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - $private_list = Koha::Virtualshelf->new( { shelfname => "PR1", owner => $patron->id } )->store; - $public_list = Koha::Virtualshelf->new( { shelfname => "PU1", public => 1, owner => $patron->id } )->store; - $list_to_share = Koha::Virtualshelf->new( { shelfname => "SH1", owner => $patron->id } )->store; + $private_list = Koha::Virtualshelf->new( { shelfname => "PR1", owner_id => $patron->id } )->store; + $public_list = Koha::Virtualshelf->new( { shelfname => "PU1", public => 1, owner_id => $patron->id } )->store; + $list_to_share = Koha::Virtualshelf->new( { shelfname => "SH1", owner_id => $patron->id } )->store; $list_to_share->share("valid key")->accept( "valid key", $patron_for_sharing->id ); $patron->delete; - is( Koha::Virtualshelves->find( $private_list->id ), undef, 'Private list gone' ); - is( $public_list->discard_changes->get_column('owner'), $designated_owner->id, 'Public list transferred' ); - is( $list_to_share->discard_changes->get_column('owner'), $designated_owner->id, 'Shared list transferred' ); + is( Koha::Virtualshelves->find( $private_list->id ), undef, 'Private list gone' ); + is( $public_list->discard_changes->get_column('owner_id'), $designated_owner->id, 'Public list transferred' ); + is( $list_to_share->discard_changes->get_column('owner_id'), $designated_owner->id, 'Shared list transferred' ); # Finally test deleting lists t::lib::Mocks::mock_preference( 'ListOwnershipUponPatronDeletion', 'delete' ); @@ -680,21 +680,21 @@ subtest "delete" => sub { my $private_list2 = Koha::Virtualshelf->new( { shelfname => "private", - owner => $patron2->borrowernumber, + owner_id => $patron2->borrowernumber, public => 0, } )->store; my $public_list2 = Koha::Virtualshelf->new( { shelfname => "public", - owner => $patron2->borrowernumber, + owner_id => $patron2->borrowernumber, public => 1, } )->store; my $list_to_share2 = Koha::Virtualshelf->new( { shelfname => "shared", - owner => $patron2->borrowernumber, + owner_id => $patron2->borrowernumber, public => 0, } )->store; diff --git a/t/db_dependent/Koha/Virtualshelf.t b/t/db_dependent/Koha/Virtualshelf.t index 06d13134b02..a938732f921 100755 --- a/t/db_dependent/Koha/Virtualshelf.t +++ b/t/db_dependent/Koha/Virtualshelf.t @@ -42,14 +42,14 @@ subtest 'transfer_ownership() tests' => sub { my $public_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); my $private_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); @@ -82,7 +82,7 @@ subtest 'transfer_ownership() tests' => sub { $public_list->transfer_ownership( $patron_2->id ); $public_list->discard_changes; - is( $public_list->owner, $patron_2->id, 'Owner changed correctly' ); + is( $public_list->owner_id, $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->next->borrowernumber, $patron_2->id, "Public lists don't get the share removed" ); @@ -90,7 +90,7 @@ subtest 'transfer_ownership() tests' => sub { $private_list->transfer_ownership( $patron_2->id ); $private_list->discard_changes; - is( $private_list->owner, $patron_2->id ); + is( $private_list->owner_id, $patron_2->id ); my $private_list_shares = $private_list->get_shares; is( $private_list_shares->count, 1, 'Count is correct' ); is( diff --git a/t/db_dependent/Koha/Virtualshelves.t b/t/db_dependent/Koha/Virtualshelves.t index bc9365fb716..407a164f06d 100755 --- a/t/db_dependent/Koha/Virtualshelves.t +++ b/t/db_dependent/Koha/Virtualshelves.t @@ -46,21 +46,21 @@ subtest 'disown_or_delete() tests' => sub { my $public_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); my $private_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); my $private_list_shared = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); @@ -85,12 +85,12 @@ subtest 'disown_or_delete() tests' => sub { is( $rs->count, 2, 'The private/non-shared list was deleted' ); my $first = $rs->next; - is( $first->id, $public_list->id ); - is( $first->owner, $patron_2->id ); + is( $first->id, $public_list->id ); + is( $first->owner_id, $patron_2->id ); my $second = $rs->next; - is( $second->id, $private_list_shared->id ); - is( $second->owner, $patron_2->id ); + is( $second->id, $private_list_shared->id ); + is( $second->owner_id, $patron_2->id ); $schema->storage->txn_rollback; }; @@ -108,21 +108,21 @@ subtest 'disown_or_delete() tests' => sub { my $public_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); my $private_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); my $private_list_shared = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); @@ -141,7 +141,7 @@ subtest 'disown_or_delete() tests' => sub { my $public_list_to_delete = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); @@ -162,12 +162,12 @@ subtest 'disown_or_delete() tests' => sub { is( $rs->count, 2, 'The private/non-shared list was deleted' ); my $first = $rs->next; - is( $first->id, $public_list->id ); - is( $first->owner, $patron_3->id ); + is( $first->id, $public_list->id ); + is( $first->owner_id, $patron_3->id ); my $second = $rs->next; - is( $second->id, $private_list_shared->id ); - is( $second->owner, $patron_3->id ); + is( $second->id, $private_list_shared->id ); + is( $second->owner_id, $patron_3->id ); $schema->storage->txn_rollback; }; @@ -181,13 +181,13 @@ subtest 'disown_or_delete() tests' => sub { my $public_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); my $private_list_shared = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); $builder->build_object( @@ -225,21 +225,21 @@ subtest 'disown_or_delete() tests' => sub { my $public_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 1 } + value => { owner_id => $patron_1->id, public => 1 } } ); my $private_list = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); my $private_list_shared = $builder->build_object( { class => "Koha::Virtualshelves", - value => { owner => $patron_1->id, public => 0 } + value => { owner_id => $patron_1->id, public => 0 } } ); diff --git a/t/db_dependent/Utils/Datatables_Virtualshelves.t b/t/db_dependent/Utils/Datatables_Virtualshelves.t index b71b1f32487..d3dd481d7df 100755 --- a/t/db_dependent/Utils/Datatables_Virtualshelves.t +++ b/t/db_dependent/Utils/Datatables_Virtualshelves.t @@ -90,7 +90,7 @@ my $shelf1 = Koha::Virtualshelf->new( shelfname => 'my first private list (empty)', public => 0, sortfield => 'author', - owner => $john_doe{borrowernumber}, + owner_id => $john_doe{borrowernumber}, } )->store; @@ -99,7 +99,7 @@ my $shelf2 = Koha::Virtualshelf->new( shelfname => 'my second private list', public => 0, sortfield => 'title', - owner => $john_doe{borrowernumber}, + owner_id => $john_doe{borrowernumber}, } )->store; my $biblionumber1 = _add_biblio('title 1'); @@ -118,7 +118,7 @@ my $shelf3 = Koha::Virtualshelf->new( shelfname => 'The first public list', public => 1, sortfield => 'author', - owner => $jane_doe{borrowernumber}, + owner_id => $jane_doe{borrowernumber}, } )->store; my $biblionumber6 = _add_biblio('title 6'); @@ -133,7 +133,7 @@ my $shelf4 = Koha::Virtualshelf->new( shelfname => 'my second public list', public => 1, sortfield => 'title', - owner => $jane_doe{borrowernumber}, + owner_id => $jane_doe{borrowernumber}, } )->store; my $biblionumber9 = _add_biblio('title 9'); @@ -150,7 +150,7 @@ my $shelf5 = Koha::Virtualshelf->new( shelfname => 'my third private list', public => 0, sortfield => 'title', - owner => $jane_doe{borrowernumber}, + owner_id => $jane_doe{borrowernumber}, } )->store; my $biblionumber13 = _add_biblio('title 13'); @@ -171,7 +171,7 @@ for my $i ( 6 .. 15 ) { { shelfname => "another public list $i", public => 1, - owner => $john_smith{borrowernumber}, + owner_id => $john_smith{borrowernumber}, } )->store; } @@ -287,4 +287,3 @@ sub _add_biblio { my ( $biblionumber, $biblioitemnumber ) = AddBiblio( $biblio, '' ); return $biblionumber; } - diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index 7f77bcf9263..09c138dc9ac 100755 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -40,7 +40,7 @@ my $dbh = C4::Context->dbh; teardown(); subtest 'CRUD' => sub { - plan tests => 15; + plan tests => 16; my $patron = $builder->build( { source => 'Borrower', @@ -54,7 +54,7 @@ subtest 'CRUD' => sub { my $shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron->{borrowernumber}, + owner_id => $patron->{borrowernumber}, public => 0, } )->store; @@ -75,6 +75,13 @@ subtest 'CRUD' => sub { 'The creation time should have been set to today' ); + # Test owner accessor is working as expected + my $owner = $shelf->owner; + is( + $owner->borrowernumber, $shelf->owner_id, + 'The owner accessor\'s borrowenumber should match the shelf\'s owner_id' + ); + # Test if creation date will not be overwritten by store my $created = dt_from_string->subtract( hours => 1 ); $shelf->created_on($created); @@ -93,7 +100,7 @@ subtest 'CRUD' => sub { $shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron->{borrowernumber}, + owner_id => $patron->{borrowernumber}, public => 0, } )->store; @@ -114,7 +121,7 @@ subtest 'CRUD' => sub { $shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $another_patron->{borrowernumber}, + owner_id => $another_patron->{borrowernumber}, public => 0, } )->store; @@ -218,7 +225,7 @@ subtest 'Sharing' => sub { my $shelf_to_share = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron_wants_to_share->{borrowernumber}, + owner_id => $patron_wants_to_share->{borrowernumber}, public => 0, } )->store; @@ -226,7 +233,7 @@ subtest 'Sharing' => sub { my $shelf_not_to_share = Koha::Virtualshelf->new( { shelfname => "my second shelf", - owner => $patron_wants_to_share->{borrowernumber}, + owner_id => $patron_wants_to_share->{borrowernumber}, public => 0, } )->store; @@ -331,7 +338,7 @@ subtest 'Shelf content' => sub { my $shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, lastmodified => $dt_yesterday, } @@ -473,7 +480,7 @@ subtest 'Shelf permissions' => sub { my $public_shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 1, allow_change_from_owner => 0, allow_change_from_others => 0, @@ -905,7 +912,7 @@ subtest 'Shelf permissions' => sub { my $private_shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, allow_change_from_owner => 0, allow_change_from_others => 0, @@ -1248,42 +1255,42 @@ subtest 'Get shelves' => sub { my $private_shelf1_1 = Koha::Virtualshelf->new( { shelfname => "private shelf 1 for patron 1", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, } )->store; my $private_shelf1_2 = Koha::Virtualshelf->new( { shelfname => "private shelf 2 for patron 1", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, } )->store; my $private_shelf2_1 = Koha::Virtualshelf->new( { shelfname => "private shelf 1 for patron 2", - owner => $patron2->{borrowernumber}, + owner_id => $patron2->{borrowernumber}, public => 0, } )->store; my $public_shelf1_1 = Koha::Virtualshelf->new( { shelfname => "public shelf 1 for patron 1", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 1, } )->store; my $public_shelf1_2 = Koha::Virtualshelf->new( { shelfname => "public shelf 2 for patron 1", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 1, } )->store; my $shelf_to_share = Koha::Virtualshelf->new( { shelfname => "shared shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, } )->store; @@ -1360,21 +1367,21 @@ subtest 'Get shelves containing biblios' => sub { my $shelf1 = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, } )->store; my $shelf2 = Koha::Virtualshelf->new( { shelfname => "my x second shelf", # 'x' to make it sorted after 'third' - owner => $patron2->{borrowernumber}, + owner_id => $patron2->{borrowernumber}, public => 0, } )->store; my $shelf3 = Koha::Virtualshelf->new( { shelfname => "my third shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 1, } )->store; @@ -1467,9 +1474,9 @@ subtest 'cannot_be_transferred' => sub { $removed_patron->delete; # Create three lists - my $private_list = Koha::Virtualshelf->new( { shelfname => "A", owner => $listowner->id } )->store; - my $public_list = Koha::Virtualshelf->new( { shelfname => "B", public => 1, owner => $listowner->id } )->store; - my $shared_list = Koha::Virtualshelf->new( { shelfname => "C", owner => $listowner->id } )->store; + my $private_list = Koha::Virtualshelf->new( { shelfname => "A", owner_id => $listowner->id } )->store; + my $public_list = Koha::Virtualshelf->new( { shelfname => "B", public => 1, owner_id => $listowner->id } )->store; + my $shared_list = Koha::Virtualshelf->new( { shelfname => "C", owner_id => $listowner->id } )->store; $shared_list->share("key")->accept( "key", $receiver->id ); # Test on private list @@ -1606,8 +1613,8 @@ subtest 'filter_by_readable() tests' => sub { { class => 'Koha::Virtualshelves', value => { - public => 0, - owner => $patron_1->id, + public => 0, + owner_id => $patron_1->id, } } ); @@ -1615,8 +1622,8 @@ subtest 'filter_by_readable() tests' => sub { { class => 'Koha::Virtualshelves', value => { - public => 1, - owner => $patron_1->id, + public => 1, + owner_id => $patron_1->id, } } ); @@ -1624,8 +1631,8 @@ subtest 'filter_by_readable() tests' => sub { { class => 'Koha::Virtualshelves', value => { - public => 0, - owner => $patron_2->id, + public => 0, + owner_id => $patron_2->id, } } ); @@ -1633,8 +1640,8 @@ subtest 'filter_by_readable() tests' => sub { { class => 'Koha::Virtualshelves', value => { - public => 1, - owner => $patron_2->id, + public => 1, + owner_id => $patron_2->id, } } ); @@ -1653,7 +1660,7 @@ subtest 'filter_by_readable() tests' => sub { is( $lists->count, 3, 'Three lists are returned' ); while ( my $list = $lists->next ) { - ok( $list->owner == $patron_1->id || $list->public, 'Only public or self lists in the resultset' ); + ok( $list->owner_id == $patron_1->id || $list->public, 'Only public or self lists in the resultset' ); } $schema->storage->txn_rollback; diff --git a/t/db_dependent/api/v1/public/lists.t b/t/db_dependent/api/v1/public/lists.t index 15512e6593a..997365aa11e 100755 --- a/t/db_dependent/api/v1/public/lists.t +++ b/t/db_dependent/api/v1/public/lists.t @@ -53,13 +53,17 @@ subtest 'list_public() tests' => sub { my $patron_2_userid = $patron_2->userid; my $list_1 = - $builder->build_object( { class => 'Koha::Virtualshelves', value => { owner => $patron_1->id, public => 1 } } ); + $builder->build_object( + { class => 'Koha::Virtualshelves', value => { owner_id => $patron_1->id, public => 1 } } ); my $list_2 = - $builder->build_object( { class => 'Koha::Virtualshelves', value => { owner => $patron_1->id, public => 0 } } ); + $builder->build_object( + { class => 'Koha::Virtualshelves', value => { owner_id => $patron_1->id, public => 0 } } ); my $list_3 = - $builder->build_object( { class => 'Koha::Virtualshelves', value => { owner => $patron_2->id, public => 1 } } ); + $builder->build_object( + { class => 'Koha::Virtualshelves', value => { owner_id => $patron_2->id, public => 1 } } ); my $list_4 = - $builder->build_object( { class => 'Koha::Virtualshelves', value => { owner => $patron_2->id, public => 0 } } ); + $builder->build_object( + { class => 'Koha::Virtualshelves', value => { owner_id => $patron_2->id, public => 0 } } ); my $q = encode_json( { diff --git a/t/db_dependent/selenium/regressions.t b/t/db_dependent/selenium/regressions.t index 5cfb15a16ae..1667328ba07 100755 --- a/t/db_dependent/selenium/regressions.t +++ b/t/db_dependent/selenium/regressions.t @@ -267,7 +267,7 @@ subtest 'XSS vulnerabilities in pagination' => sub { public => 1, allow_change_from_owner => 1, allow_change_from_others => 0, - owner => $patron->borrowernumber + owner_id => $patron->borrowernumber } } ); diff --git a/tools/batch_delete_records.pl b/tools/batch_delete_records.pl index 13540781c60..fca72594bc8 100755 --- a/tools/batch_delete_records.pl +++ b/tools/batch_delete_records.pl @@ -61,7 +61,7 @@ if ( $op eq 'form' ) { op => 'form', lists => Koha::Virtualshelves->search( [ - { public => 0, owner => $loggedinuser }, + { public => 0, owner_id => $loggedinuser }, { public => 1 } ] ) diff --git a/tools/batch_record_modification.pl b/tools/batch_record_modification.pl index 560f420d954..82d0d454cc0 100755 --- a/tools/batch_record_modification.pl +++ b/tools/batch_record_modification.pl @@ -87,7 +87,7 @@ if ( $op eq 'form' ) { view => 'form', lists => Koha::Virtualshelves->search( [ - { public => 0, owner => $loggedinuser }, + { public => 0, owner_id => $loggedinuser }, { public => 1 } ] ) diff --git a/virtualshelves/addbybiblionumber.pl b/virtualshelves/addbybiblionumber.pl index 82dcbb18858..513cfddb1ed 100755 --- a/virtualshelves/addbybiblionumber.pl +++ b/virtualshelves/addbybiblionumber.pl @@ -100,7 +100,7 @@ if ( $op && $op !~ /^cud-/ ) { shelfname => $newvirtualshelf, public => $public, sortfield => $sortfield, - owner => $loggedinuser, + owner_id => $loggedinuser, } )->store; }; @@ -155,7 +155,7 @@ if ( $op && $op !~ /^cud-/ ) { my $private_shelves = Koha::Virtualshelves->search( { public => 0, - owner => $loggedinuser, + owner_id => $loggedinuser, allow_change_from_owner => 1, }, { order_by => 'shelfname' } @@ -174,7 +174,7 @@ if ( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, allow_change_from_staff => 1, diff --git a/virtualshelves/shelves.pl b/virtualshelves/shelves.pl index f64caf75428..a2f8a1568ec 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -78,7 +78,7 @@ if ( $op eq 'add_form' ) { if ($shelf) { $public = $shelf->public; - my $patron = Koha::Patrons->find( $shelf->owner )->unblessed; + my $patron = Koha::Patrons->find( $shelf->owner_id )->unblessed; $template->param( owner => $patron, ); unless ( $shelf->can_be_managed($loggedinuser) ) { push @messages, { type => 'alert', code => 'unauthorized_on_update' }; @@ -100,7 +100,7 @@ if ( $op eq 'add_form' ) { allow_change_from_others => $allow_changes_from == ANYONE, allow_change_from_staff => $allow_changes_from == STAFF, allow_change_from_permitted_staff => $allow_changes_from == PERMITTED, - owner => scalar $query->param('owner'), + owner_id => scalar $query->param('owner'), } ); $shelf->store; -- 2.50.1 (Apple Git-155)