From 0adae3885f5ad9ef8191f73e2e40f1c9709bbac0 Mon Sep 17 00:00:00 2001 From: PerplexedTheta Date: Mon, 19 Aug 2024 17:22:54 +0100 Subject: [PATCH] Bug 37346: Rename 'owner' to 'owner_id' in virtualshelves This patch will rename the owner field in the virtualshelves database to owner_id. This makes it more consistent with other tables, and paves the way for a patch which will allow a direct accessor to the borrower specified in owner_id. Test plan: a) 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 --- C4/Utils/DataTables/VirtualShelves.pm | 10 ++--- Koha/Exceptions/Virtualshelf.pm | 2 +- Koha/Patron.pm | 2 +- Koha/REST/V1/ERM/EHoldings/Titles/Local.pm | 4 +- Koha/Schema/Result/Borrower.pm | 6 +-- Koha/Schema/Result/Virtualshelve.pm | 10 ++--- Koha/Virtualshelf.pm | 26 ++++++------ Koha/Virtualshelves.pm | 12 +++--- ...er-column-to-owner_id-in-virtualshelves.pl | 16 ++++++++ installer/data/mysql/kohastructure.sql | 6 +-- .../prog/en/modules/virtualshelves/shelves.tt | 10 ++--- .../virtualshelves/tables/shelves_results.tt | 2 +- .../bootstrap/en/modules/opac-shelves.tt | 2 +- opac/opac-addbybiblionumber.pl | 12 +++--- opac/opac-shelves.pl | 8 ++-- t/db_dependent/Koha/Patrons.t | 26 ++++++------ t/db_dependent/Koha/Virtualshelf.t | 8 ++-- t/db_dependent/Koha/Virtualshelves.t | 28 ++++++------- .../Utils/Datatables_Virtualshelves.t | 12 +++--- t/db_dependent/Virtualshelves.t | 40 +++++++++---------- virtualshelves/addbybiblionumber.pl | 6 +-- virtualshelves/shelves.pl | 8 ++-- 22 files changed, 136 insertions(+), 120 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_37346-rename-owner-column-to-owner_id-in-virtualshelves.pl diff --git a/C4/Utils/DataTables/VirtualShelves.pm b/C4/Utils/DataTables/VirtualShelves.pm index 2c4c1628e1..0f167083ad 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| @@ -74,7 +74,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; } @@ -107,7 +107,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( @@ -127,7 +127,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/Exceptions/Virtualshelf.pm b/Koha/Exceptions/Virtualshelf.pm index c3d32133fb..091e9595f6 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 7234a3be5b..b5df1be860 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -97,7 +97,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 c45d4d6914..07b39d515d 100644 --- a/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm +++ b/Koha/REST/V1/ERM/EHoldings/Titles/Local.pm @@ -242,7 +242,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"); } @@ -341,4 +341,4 @@ sub import_from_kbart_file { }; } -1; \ No newline at end of file +1; diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index bf8ba36030..bf68d1ff14 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -2133,7 +2133,7 @@ Related object: L __PACKAGE__->has_many( "virtualshelves", "Koha::Schema::Result::Virtualshelve", - { "foreign.owner" => "self.borrowernumber" }, + { "foreign.owner_id" => "self.borrowernumber" }, { cascade_copy => 0, cascade_delete => 0 }, ); @@ -2188,8 +2188,8 @@ Composing rels: L -> permission __PACKAGE__->many_to_many("permissions", "user_permissions", "permission"); -# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-05-02 11:36:36 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:jUOMoSonNmA3KwtZKVxqOA +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-08-19 14:51:55 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:APSxVanTtXYB9oNkdsQ5VA __PACKAGE__->has_many( "restrictions", diff --git a/Koha/Schema/Result/Virtualshelve.pm b/Koha/Schema/Result/Virtualshelve.pm index b2cd76ec4f..12c1c05e41 100644 --- a/Koha/Schema/Result/Virtualshelve.pm +++ b/Koha/Schema/Result/Virtualshelve.pm @@ -39,7 +39,7 @@ unique identifier assigned by Koha name of the list -=head2 owner +=head2 owner_id data_type: 'integer' is_foreign_key: 1 @@ -120,7 +120,7 @@ __PACKAGE__->add_columns( { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, "shelfname", { data_type => "varchar", is_nullable => 1, size => 255 }, - "owner", + "owner_id", { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, "public", { data_type => "tinyint", default_value => 0, is_nullable => 0 }, @@ -179,7 +179,7 @@ Related object: L __PACKAGE__->belongs_to( "owner", "Koha::Schema::Result::Borrower", - { borrowernumber => "owner" }, + { borrowernumber => "owner_id" }, { is_deferrable => 1, join_type => "LEFT", @@ -219,8 +219,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-08-17 19:59:33 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:EKq7nDW2AeZ2NQce2BPMWA +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-08-19 14:51:55 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4fO02fpsQwOOMqJKtNQ7Hg sub koha_object_class { 'Koha::Virtualshelf'; diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm index 930c4d6239..7937c32888 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -44,7 +44,7 @@ Koha::Virtualshelf - Koha Virtualshelf Object class sub store { my ($self) = @_; - unless ( $self->owner ) { + unless ( $self->owner_id ) { Koha::Exceptions::Virtualshelf::UseDbAdminAccount->throw; } @@ -85,14 +85,14 @@ 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; + } elsif ( $self->is_private and not defined $self->owner_id ) { + $conditions->{owner_id} = undef; $conditions->{public} = 0; } else { $conditions->{public} = 1; @@ -181,7 +181,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; @@ -207,7 +207,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 ) @@ -225,7 +225,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, @@ -237,7 +237,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; @@ -249,7 +249,7 @@ sub can_be_deleted { 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 @@ -263,7 +263,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 ) @@ -361,7 +361,7 @@ sub transfer_ownership { unless $patron_id; $self->remove_share($patron_id) if $self->is_private; - return $self->set( { owner => $patron_id } )->store; + return $self->set( { owner_id => $patron_id } )->store; } =head2 Internal methods diff --git a/Koha/Virtualshelves.pm b/Koha/Virtualshelves.pm index 9b71a016bb..ad4176d10a 100644 --- a/Koha/Virtualshelves.pm +++ b/Koha/Virtualshelves.pm @@ -90,7 +90,7 @@ sub get_private_shelves { public => 0, -or => { 'virtualshelfshares.borrowernumber' => $borrowernumber, - 'me.owner' => $borrowernumber, + 'me.owner_id' => $borrowernumber, } }, { @@ -148,7 +148,7 @@ sub get_some_shelves { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -161,7 +161,7 @@ sub get_some_shelves { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -173,7 +173,7 @@ sub get_some_shelves { -or => [ { - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, "me.allow_change_from_owner" => 1, }, "me.allow_change_from_others" => 1, @@ -186,7 +186,7 @@ sub get_some_shelves { -or => { "virtualshelfshares.borrowernumber" => $borrowernumber, - "me.owner" => $borrowernumber, + "me.owner_id" => $borrowernumber, } }; } @@ -221,7 +221,7 @@ sub get_shelves_containing_record { { public => 0, -or => { - 'me.owner' => $borrowernumber, + 'me.owner_id' => $borrowernumber, -or => { 'virtualshelfshares.borrowernumber' => $borrowernumber, }, diff --git a/installer/data/mysql/atomicupdate/bug_37346-rename-owner-column-to-owner_id-in-virtualshelves.pl b/installer/data/mysql/atomicupdate/bug_37346-rename-owner-column-to-owner_id-in-virtualshelves.pl new file mode 100755 index 0000000000..435217ff3d --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_37346-rename-owner-column-to-owner_id-in-virtualshelves.pl @@ -0,0 +1,16 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_failure say_success say_info); + +return { + bug_number => "37346", + description => "The VirtualShelf object should have an 'owner' accessor to return the related owner Koha::Patron", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + if( !column_exists( 'virtualshelves', 'owner_id' ) ) { + $dbh->do(q{ ALTER TABLE virtualshelves RENAME COLUMN owner TO owner_id; }); + say $out "Rename owner column to owner_id in virtualshelves"; + } + }, +}; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 753ae3cc57..b7c3dbf053 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -6565,7 +6565,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', @@ -6575,8 +6575,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/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt index 4a0d749579..4caafe3366 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt @@ -415,12 +415,12 @@
  • Owner: [% IF op == 'add_form' %] - [% logged_in_user.userid | html %] + [% logged_in_user.userid | html %] [% ELSE %] - [% IF owner %] - [% owner.firstname _ ' ' _ owner.surname | html %] + [% IF owner_id %] + [% owner_id.firstname _ ' ' _ owner_id.surname | html %] [% ELSE %] - [% logged_in_user.userid | html %] + [% logged_in_user.userid | html %] [% END %] [% END %]
    • @@ -635,7 +635,7 @@ 'vs.shelfname', 'count', 'vs.public', - 'vs.owner', + 'vs.owner_id', 'vs.sortfield', 'vs.created_on', 'vs.lastmodified', diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt index de301479f3..5550cca9af 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt @@ -18,7 +18,7 @@ "dt_is_shared": "[% IF d.public %]Public[% ELSIF d.is_shared %]Shared[% ELSE %]Private[% END %]", "dt_owner": - "[% d.firstname | html | $To %] [% d.surname | html | $To %]", + "[% d.firstname | html | $To %] [% d.surname | html | $To %]", "dt_sortby": [% IF d.sortby == "author" %]"Author"[% ELSIF d.sortby == "copyrightdate" %]"Year"[% ELSIF d.sortby == "itemcallnumber" %]"Call number"[% ELSIF d.sortby == "dateadded" %]"Date added"[% ELSE %]"Title"[% END %], "dt_created_on": diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt index d1fc75c883..812dc3b524 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ -572,7 +572,7 @@ [% END %] - +
    1. diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl index 1916c26e9e..f46396f4d4 100755 --- a/opac/opac-addbybiblionumber.pl +++ b/opac/opac-addbybiblionumber.pl @@ -62,7 +62,7 @@ if( $op && $op !~ /^cud-/ ) { and ( !$public or $public and $loggedinuser > 0 && C4::Context->preference('OpacAllowPublicListCreation') ) ) { - my $shelf = eval { Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, public => $public, owner => $loggedinuser, } )->store; }; + my $shelf = eval { Koha::Virtualshelf->new( { shelfname => $newvirtualshelf, public => $public, owner_id => $loggedinuser, } )->store; }; if ( $@ or not $shelf ) { $errcode = 1; $authorized = 0; @@ -108,7 +108,7 @@ if( $op && $op !~ /^cud-/ ) { if ( $loggedinuser > 0 ) { my $private_shelves = Koha::Virtualshelves->search( { public => 0, - owner => $loggedinuser, + owner_id => $loggedinuser, allow_change_from_owner => 1, }, { order_by => 'shelfname' } @@ -128,7 +128,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, @@ -143,7 +143,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 @@ -157,7 +157,7 @@ if( $op && $op !~ /^cud-/ ) { -or => [ -and => { allow_change_from_owner => 1, - owner => $loggedinuser, + owner_id => $loggedinuser, }, allow_change_from_others => 1, ], @@ -171,7 +171,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-shelves.pl b/opac/opac-shelves.pl index 728dad57d1..8887f5c734 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -103,8 +103,8 @@ if ( $op eq 'add_form' ) { if ( $shelf ) { $public = $shelf->public; - my $patron = Koha::Patrons->find( $shelf->owner ); - $template->param( owner => $patron, ); + my $patron = Koha::Patrons->find( $shelf->owner_id ); + $template->param( owner_id => $patron, ); unless ( $shelf->can_be_managed( $loggedinuser ) ) { push @messages, { type => 'error', code => 'unauthorized_on_update' }; $op = 'list'; @@ -124,7 +124,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; @@ -290,7 +290,7 @@ if ( $op eq 'add_form' ) { $shelf->_result->result_source->schema->txn_do( sub { $shelf->get_shares->search({ borrowernumber => $new_owner })->delete; 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 954b220a39..7df39312f3 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -473,17 +473,17 @@ subtest "delete" => sub { my $modification = $builder->build_object({ class => 'Koha::Patron::Modifications', value => { borrowernumber => $patron->borrowernumber } }); 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; @@ -499,11 +499,11 @@ subtest "delete" => sub { is( Koha::Holds->search( { borrowernumber => $patron->borrowernumber } )->count, 0, 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'); is( Koha::Virtualshelfshares->search({ borrowernumber => $staff_patron->borrowernumber })->count, 0, "New owner of list should have shares removed" ); is( Koha::Virtualshelfshares->search({ borrowernumber => $patron_for_sharing->borrowernumber })->count, 1, "But the other share is still there" ); - is( Koha::Virtualshelves->search({ owner => $patron->borrowernumber })->count, 0, q|Koha::Patron->delete should have deleted patron's lists/removed their ownership| ); + is( Koha::Virtualshelves->search({ owner_id => $patron->borrowernumber })->count, 0, q|Koha::Patron->delete should have deleted patron's lists/removed their ownership| ); is( Koha::Patron::Modifications->search( { borrowernumber => $patron->borrowernumber } )->count, 0, q|Koha::Patron->delete should have deleted patron's modifications| ); @@ -514,14 +514,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( $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' ); @@ -529,17 +529,17 @@ subtest "delete" => sub { my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); 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; $list_to_share2->share("valid key")->accept( "valid key", $patron_for_sharing->borrowernumber ); diff --git a/t/db_dependent/Koha/Virtualshelf.t b/t/db_dependent/Koha/Virtualshelf.t index af6c2e1b32..227499c468 100755 --- a/t/db_dependent/Koha/Virtualshelf.t +++ b/t/db_dependent/Koha/Virtualshelf.t @@ -41,14 +41,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( $private_list_shares->next->borrowernumber, $patron_3->id, "Private lists get the share for the new owner removed" ); diff --git a/t/db_dependent/Koha/Virtualshelves.t b/t/db_dependent/Koha/Virtualshelves.t index 20d2454c99..babf3c396e 100755 --- a/t/db_dependent/Koha/Virtualshelves.t +++ b/t/db_dependent/Koha/Virtualshelves.t @@ -45,21 +45,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 } } ); @@ -83,11 +83,11 @@ 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->owner_id, $patron_2->id ); my $second = $rs->next; is( $second->id, $private_list_shared->id ); - is( $second->owner, $patron_2->id ); + is( $second->owner_id, $patron_2->id ); $schema->storage->txn_rollback; }; @@ -105,21 +105,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 } } ); @@ -137,7 +137,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 } } ); @@ -158,11 +158,11 @@ 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->owner_id, $patron_3->id ); my $second = $rs->next; is( $second->id, $private_list_shared->id ); - is( $second->owner, $patron_3->id ); + is( $second->owner_id, $patron_3->id ); $schema->storage->txn_rollback; }; @@ -180,21 +180,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 de4398a7c7..a0bceb3cc8 100755 --- a/t/db_dependent/Utils/Datatables_Virtualshelves.t +++ b/t/db_dependent/Utils/Datatables_Virtualshelves.t @@ -89,7 +89,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; @@ -98,7 +98,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'); @@ -117,7 +117,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'); @@ -132,7 +132,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'); @@ -149,7 +149,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'); @@ -170,7 +170,7 @@ for my $i ( 6 .. 15 ) { { shelfname => "another public list $i", public => 1, - owner => $john_smith{borrowernumber}, + owner_id => $john_smith{borrowernumber}, } )->store; } diff --git a/t/db_dependent/Virtualshelves.t b/t/db_dependent/Virtualshelves.t index 0b8c5bf5e7..05519d7b3e 100755 --- a/t/db_dependent/Virtualshelves.t +++ b/t/db_dependent/Virtualshelves.t @@ -33,7 +33,7 @@ subtest 'CRUD' => sub { my $shelf = Koha::Virtualshelf->new({ shelfname => "my first shelf", - owner => $patron->{borrowernumber}, + owner_id => $patron->{borrowernumber}, public => 0, } )->store; @@ -62,7 +62,7 @@ subtest 'CRUD' => sub { eval { $shelf = Koha::Virtualshelf->new({ shelfname => "my first shelf", - owner => $patron->{borrowernumber}, + owner_id => $patron->{borrowernumber}, public => 0, } )->store; @@ -78,7 +78,7 @@ subtest 'CRUD' => sub { $shelf = Koha::Virtualshelf->new({ shelfname => "my first shelf", - owner => $another_patron->{borrowernumber}, + owner_id => $another_patron->{borrowernumber}, public => 0, } )->store; @@ -110,14 +110,14 @@ 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; 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; @@ -199,7 +199,7 @@ subtest 'Shelf content' => sub { my $dt_yesterday = dt_from_string->subtract_duration( DateTime::Duration->new( days => 1 ) ); my $shelf = Koha::Virtualshelf->new( { shelfname => "my first shelf", - owner => $patron1->{borrowernumber}, + owner_id => $patron1->{borrowernumber}, public => 0, lastmodified => $dt_yesterday, } @@ -316,7 +316,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, @@ -456,7 +456,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, @@ -578,37 +578,37 @@ 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; @@ -644,19 +644,19 @@ 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; @@ -731,9 +731,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 diff --git a/virtualshelves/addbybiblionumber.pl b/virtualshelves/addbybiblionumber.pl index 69483d5dc1..841c692cab 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; }; @@ -153,7 +153,7 @@ if ( $op && $op !~ /^cud-/ ) { } else { my $private_shelves = Koha::Virtualshelves->search( { public => 0, - owner => $loggedinuser, + owner_id => $loggedinuser, allow_change_from_owner => 1, }, { order_by => 'shelfname' } @@ -170,7 +170,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 e3e7a868e4..bbd607b70f 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -76,8 +76,8 @@ if ( $op eq 'add_form' ) { if ( $shelf ) { $public = $shelf->public; - my $patron = Koha::Patrons->find( $shelf->owner )->unblessed; - $template->param( owner => $patron, ); + my $patron = Koha::Patrons->find( $shelf->owner_id )->unblessed; + $template->param( owner_id => $patron, ); unless ( $shelf->can_be_managed( $loggedinuser ) ) { push @messages, { type => 'alert', code => 'unauthorized_on_update' }; $op = 'list'; @@ -97,7 +97,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_id'), } ); $shelf->store; @@ -266,7 +266,7 @@ if ( $op eq 'add_form' ) { push @messages, { type => 'error', code => $error_code }; $op = 'list'; } else { - $shelf->owner($new_owner)->store; + $shelf->owner_id($new_owner)->store; $op = 'list'; } } -- 2.39.3 (Apple Git-146)