Attachment #128385 for bug #26346

View | Details | Raw Unified | Return to bug 26346
Collapse All | Expand All




    return \%return;
}

=head3 can_patron_change_staff_only_lists

$patron->can_patron_change_staff_only_lists;

Return 1 if a patron has 'Superlibrarian' or 'Catalogue' permission.
Otherwise, return 0.

=cut

sub can_patron_change_staff_only_lists {
    my ( $self, $params ) = @_;
    return 1 if C4::Auth::haspermission( $self->userid, { 'catalogue' => 1 });
    return 0;
}

=head2 Internal methods

=head3 _type

Lines 56-61 sub store { Link Here

(-)a/Koha/Virtualshelf.pm (-4 / +14 lines)
56	unless defined $self->allow_change_from_owner;	56	unless defined $self->allow_change_from_owner;
57	$self->allow_change_from_others( 0 )	57	$self->allow_change_from_others( 0 )
58	unless defined $self->allow_change_from_others;	58	unless defined $self->allow_change_from_others;
		59	$self->allow_change_from_staff( 0 )
		60	unless defined $self->allow_change_from_staff;
59		61
60	$self->created_on( dt_from_string )	62	$self->created_on( dt_from_string )
61	unless defined $self->created_on;	63	unless defined $self->created_on;
Lines 177-183 sub add_biblio { Link Here
177	return if $already_exists;	179	return if $already_exists;
178		180
179	# Check permissions	181	# Check permissions
180	return unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) \|\| $self->allow_change_from_others;	182	my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
		183	return 0 unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) \|\| ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) \|\| $self->allow_change_from_others;
181		184
182	my $content = Koha::Virtualshelfcontent->new(	185	my $content = Koha::Virtualshelfcontent->new(
183	{	186	{
Lines 199-205 sub remove_biblios { Link Here
199	return unless @$biblionumbers;	202	return unless @$biblionumbers;
200		203
201	my $number_removed = 0;	204	my $number_removed = 0;
		205	my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
202	if( ( $self->owner == $borrowernumber && $self->allow_change_from_owner )	206	if( ( $self->owner == $borrowernumber && $self->allow_change_from_owner )
		207	\|\| ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists )
203	\|\| $self->allow_change_from_others ) {	208	\|\| $self->allow_change_from_others ) {
204	$number_removed += $self->get_contents->search({	209	$number_removed += $self->get_contents->search({
205	biblionumber => $biblionumbers,	210	biblionumber => $biblionumbers,
Lines 226-234 sub can_be_deleted { Link Here
226	return 0 unless $borrowernumber;	231	return 0 unless $borrowernumber;
227	return 1 if $self->owner == $borrowernumber;	232	return 1 if $self->owner == $borrowernumber;
228		233
229	my $patron = Koha::Patrons->find( $borrowernumber );	234	my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
230		235
231	return 1 if $self->is_public and C4::Auth::haspermission( $patron->userid, { lists => 'delete_public_lists' } );	236	return 1 if $self->is_public and haspermission( $patron->userid, { lists => 'delete_public_lists' } );
232		237
233	return 0;	238	return 0;
234	}	239	}
Lines 237-251 sub can_be_managed { Link Here
237	my ( $self, $borrowernumber ) = @_;	242	my ( $self, $borrowernumber ) = @_;
238	return 1	243	return 1
239	if $borrowernumber and $self->owner == $borrowernumber;	244	if $borrowernumber and $self->owner == $borrowernumber;
		245
		246	my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
		247	return 1
		248	if $self->is_public and haspermission( $patron->userid, { lists => 'edit_public_lists' } );
240	return 0;	249	return 0;
241	}	250	}
242		251
243	sub can_biblios_be_added {	252	sub can_biblios_be_added {
244	my ( $self, $borrowernumber ) = @_;	253	my ( $self, $borrowernumber ) = @_;
245		254
		255	my $patron = Koha::Patrons->find( $borrowernumber ) or return 0;
246	return 1	256	return 1
247	if $borrowernumber	257	if $borrowernumber
248	and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or $self->allow_change_from_others );	258	and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or ( $self->allow_change_from_staff && $patron->can_patron_change_staff_only_lists ) or $self->allow_change_from_others );
249	return 0;	259	return 0;
250	}	260	}
251		261





use Koha::Virtualshelf;


use base qw(Koha::Objects);

=head1 NAME

    my $add_allowed = $params->{add_allowed};

    my @conditions;
    my $patron;
    my $staffuser = 0;
    if ( $borrowernumber != 0 ) {
        $patron = Koha::Patrons->find( $borrowernumber );
        $staffuser = $patron->can_patron_change_staff_only_lists;
    }
    if ( $add_allowed ) {
        if ( $staffuser ) {
            push @conditions, {
                -or =>
                [
                    {
                        "me.owner" => $borrowernumber,
                        "me.allow_change_from_owner" => 1,
                    },
                    "me.allow_change_from_others" => 1,
                    "me.allow_change_from_staff"  => 1
                ]
            };
        } else {
            push @conditions, {
                -or =>
                [
                    {
                        "me.owner" => $borrowernumber,
                        "me.allow_change_from_owner" => 1,
                    },
                    "me.allow_change_from_others" => 1,
                ]
            };
        }
    }
    if ( !$public ) {
        push @conditions, {

Lines 675-680 Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc (+5 lines)
675	Delete public lists	675	Delete public lists
676	</span>	676	</span>
677	<span class="permissioncode">([% name \| html %])</span>	677	<span class="permissioncode">([% name \| html %])</span>
		678	[%- CASE 'edit_public_lists' -%]
		679	<span class="sub_permission edit_public_lists_subpermission">
		680	Edit public lists
		681	</span>
		682	<span class="permissioncode">([% name \| html %])</span>
678	[%- CASE 'upload_general_files' -%]	683	[%- CASE 'upload_general_files' -%]
679	<span class="sub_permission upload_general_files_subpermission">	684	<span class="sub_permission upload_general_files_subpermission">
680	Upload any file	685	Upload any file

Lines 33-40 Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt (-1 / +13 lines)
33		33
34	[% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %]	34	[% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %]
35		35
		36	[% IF shelf.allow_change_from_staff %]<option value="3" selected="selected">Staff only</option>[% ELSE %]<option value="3">Staff only</option>[% END %]
		37
36	</select>	38	</select>
37	&emsp; <span id="anyone_remark" style="display:none;color:red;">The Anyone permission has no actual effect while this list is strictly private.</span>	39	&emsp; <span id="anyone_remark" style="display:none;color:red;">The Anyone permission has no actual effect while this list is strictly private.</span>
		40	&emsp; <span id="staff_remark" style="display:none;color:red;">The Staff only permission has no actual effect while this list is strictly private.</span>
38	</li>	41	</li>
39	[% END %]	42	[% END %]
40	</head>	43	</head>
Lines 803-818 Link Here
803		806
804	if( perms < 2 ) {	807	if( perms < 2 ) {
805	$("#anyone_remark").hide();	808	$("#anyone_remark").hide();
		809	$("#staff_remark").hide();
806	} else if( public==0 ) {	810	} else if( public==0 ) {
807	// If we move to Private (without shares), show Anyone remark	811	// If we move to Private (without shares), show Anyone remark
808	// Note: the number of shares is not tested real-time	812	// Note: the number of shares is not tested real-time
809	[% IF !shelf.is_shared %]	813	[% IF !shelf.is_shared %]
810	$("#anyone_remark").show();	814	if( perms== 2) {
		815	$("#anyone_remark").show();
		816	$("#staff_remark").hide();
		817	} else if ( perms==3 ) {
		818	$("#anyone_remark").hide();
		819	$("#staff_remark").show();
		820	}
811	[% ELSE %]	821	[% ELSE %]
812	$("#anyone_remark").hide();	822	$("#anyone_remark").hide();
		823	$("#staff_remark").hide();
813	[% END %]	824	[% END %]
814	} else { // public==1	825	} else { // public==1
815	$("#anyone_remark").hide();	826	$("#anyone_remark").hide();
		827	$("#staff_remark").hide();
816	}	828	}
817	}	829	}
818	[% IF op == 'view' %]	830	[% IF op == 'view' %]

Lines 50-57 Link Here

(-)a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt (-1 / +13 lines)
50		50
51	[% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %]	51	[% IF shelf.allow_change_from_others %]<option value="2" selected="selected">Anyone seeing this list</option>[% ELSE %]<option value="2">Anyone seeing this list</option>[% END %]
52		52
		53	[% IF staffuser == 1 %][% IF shelf.allow_change_from_staff %]<option value="3" selected="selected">Staff only</option>[% ELSE %]<option value="3">Staff only</option>[% END %][% END %]
		54
53	</select>	55	</select>
54	&emsp; <span id="anyone_remark" style="display:none;color:red;">The "Anyone" permission has no actual effect while this list is strictly private.</span>	56	&emsp; <span id="anyone_remark" style="display:none;color:red;">The "Anyone" permission has no actual effect while this list is strictly private.</span>
		57	&emsp; <span id="staff_remark" style="display:none;color:red;">The "Staff only" permission has no actual effect while this list is strictly private.</span>
55	</li>	58	</li>
56	[% END %]	59	[% END %]
57		60
Lines 1042-1057 function AdjustRemark() { Link Here
1042		1045
1043	if( perms < 2 ) {	1046	if( perms < 2 ) {
1044	$("#anyone_remark").hide();	1047	$("#anyone_remark").hide();
		1048	$("#staff_remark").hide();
1045	} else if( public==0 ) {	1049	} else if( public==0 ) {
1046	// If we move to Private (without shares), show Anyone remark	1050	// If we move to Private (without shares), show Anyone remark
1047	// Note: the number of shares is not tested real-time	1051	// Note: the number of shares is not tested real-time
1048	[% IF !shelf.is_shared %]	1052	[% IF !shelf.is_shared %]
1049	$("#anyone_remark").show();	1053	if ( perms==2 ) {
		1054	$("#anyone_remark").show();
		1055	$("#staff_remark").hide();
		1056	} else if ( perms==3 ) {
		1057	$("#anyone_remark").hide();
		1058	$("#staff_remark").show();
		1059	}
1050	[% ELSE %]	1060	[% ELSE %]
1051	$("#anyone_remark").hide();	1061	$("#anyone_remark").hide();
		1062	$("#staff_remark").hide();
1052	[% END %]	1063	[% END %]
1053	} else { // public==1	1064	} else { // public==1
1054	$("#anyone_remark").hide();	1065	$("#anyone_remark").hide();
		1066	$("#staff_remark").hide();
1055	}	1067	}
1056	}	1068	}
1057	</script>	1069	</script>




            },
            { join => 'virtualshelfshares', }
        );
        my $public_shelves;
        if ( $loggedinuser ) {
            if ( Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists ) {
                $public_shelves = Koha::Virtualshelves->search(
                    {   public   => 1,
                        -or      => [
                            -and => {
                                allow_change_from_owner => 1,
                                owner     => $loggedinuser,
                            },
                            allow_change_from_others => 1,
                            allow_change_from_staff  => 1
                        ],
                    },
                    { order_by => 'shelfname' }
                );
            } else {
                $public_shelves = Koha::Virtualshelves->search(
                    {   public   => 1,
                        -or      => [
                            -and => {
                                allow_change_from_owner => 1,
                                owner => $loggedinuser,
                            },
                            allow_change_from_others => 1,
                        ],
                    },
                    {order_by => 'shelfname' }
                );
            }
        } else {
            $public_shelves = Koha::Virtualshelves->search(
                {   public   => 1,
                    -or      => [
                        -and => {
                            allow_change_from_owner => 1,
                            owner => $loggedinuser,
                        },
                        allow_change_from_others => 1,
                    ],
                },
                {order_by => 'shelfname' }
            );
        }

        $template->param(
            private_shelves                => $private_shelves,
            private_shelves_shared_with_me => $shelves_shared_with_me,




use Koha::RecordProcessor;

use constant ANYONE => 2;
use constant STAFF => 3;

my $query = CGI->new;


                    public             => $public,
                    allow_change_from_owner => $allow_changes_from > 0,
                    allow_change_from_others => $allow_changes_from == ANYONE,
                    allow_change_from_staff => $allow_changes_from == STAFF,
                    owner              => scalar $loggedinuser,
                }
            );

            my $allow_changes_from = $query->param('allow_changes_from');
            $shelf->allow_change_from_owner( $allow_changes_from > 0 );
            $shelf->allow_change_from_others( $allow_changes_from == ANYONE );
            $shelf->allow_change_from_staff( $allow_changes_from == STAFF );
            $shelf->public( $public );
            eval { $shelf->store };


        ),
    );
}
my $staffuser;
$staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser;
$template->param(
    op       => $op,
    referer  => $referer,

    public   => $public,
    print    => scalar $query->param('print') || 0,
    listsview => 1,
    staffuser => $staffuser,
);

my $content_type = $query->param('rss')? 'rss' : 'html';





use Modern::Perl;

use Test::More tests => 11;
use Test::Exception;
use Test::Warn;


    is( $article_requests->count, 4, '4 article requests' );

    $schema->storage->txn_rollback;

};

subtest 'can_patron_change_staff_only_lists() tests' => sub {

    plan tests => 3;

    $schema->storage->txn_begin;

    # make a user with no special permissions
    my $patron = $builder->build_object(
        {
            class => 'Koha::Patrons',
            value => {
                flags => undef
            }
        }
    );
    is( $patron->can_patron_change_staff_only_lists(), 0, 'Patron without permissions cannot change staff only lists');

    # make it a 'Catalogue' permission
    $patron->set({ flags => 4 })->store->discard_changes;
    is( $patron->can_patron_change_staff_only_lists(), 1, 'Catalogue patron can change staff only lists');


    # make it a superlibrarian
    $patron->set({ flags => 1 })->store->discard_changes;
    is( $patron->can_patron_change_staff_only_lists(), 1, 'Superlibrarian patron can change staff only lists');

    $schema->storage->txn_rollback;
};




teardown();

subtest 'CRUD' => sub {
    plan tests => 14;
    my $patron = $builder->build({
        source => 'Borrower',
    });

    is( $number_of_shelves, 1, '1 shelf should have been inserted' );
    is( $shelf->allow_change_from_owner, 1, 'The default value for allow_change_from_owner should be 1' );
    is( $shelf->allow_change_from_others, 0, 'The default value for allow_change_from_others should be 0' );
    is ( $shelf->allow_change_from_staff, 0, 'The default value for allow_change_from_staff should be 0');
    is( t::lib::Dates::compare( $shelf->created_on, dt_from_string), 0, 'The creation time should have been set to today' );

    # Test if creation date will not be overwritten by store


subtest 'Shelf content' => sub {

    plan tests => 21;
    my $patron1 = $builder->build( { source => 'Borrower', } );
    my $patron2 = $builder->build( { source => 'Borrower', } );
    my $patron3 = $builder->build( { source => 'Borrower', value => {flags => 1} });
    my $biblio1 = $builder->build_sample_biblio;
    my $biblio2 = $builder->build_sample_biblio;
    my $biblio3 = $builder->build_sample_biblio;

    $number_of_contents = Koha::Virtualshelfcontents->search->count;
    is( $number_of_contents, 3, 'Back to three entries' );

    # allow_change_from_staff == 1 and allow_change_from_others == 0
    $shelf->allow_change_from_staff( 1 );
    $shelf->allow_change_from_others( 0 );
    $content4 = $shelf->add_biblio( $biblio3->biblionumber, $patron3->{borrowernumber} );
    $number_of_contents = Koha::Virtualshelfcontents->search->count;
    is( $number_of_contents, 4, 'The biblio should have been added to the shelf by patron 2');
    $number_of_deleted_biblios = $shelf->remove_biblios( { biblionumbers => [ $biblio3->biblionumber ], borrowernumber => $patron3->{borrowernumber} } );
    is( $number_of_deleted_biblios, 1, 'Biblio 3 deleted by patron 2' );
    $number_of_contents = Koha::Virtualshelfcontents->search->count;
    is( $number_of_contents, 3, 'Back to three entries' );

    teardown();
};

subtest 'Shelf permissions' => sub {

    plan tests => 100;
    my $patron1 = $builder->build( { source => 'Borrower', value => { flags => '2096766' } } ); # 2096766 is everything checked but not superlibrarian
    my $patron2 = $builder->build( { source => 'Borrower', value => { flags => '1048190' } } ); # 1048190 is everything checked but not superlibrarian and delete_public_lists
    my $patron3 = $builder->build( { source => 'Borrower', value => { flags => '0' } } ); # this is a patron with no special permissions
    my $patron4 = $builder->build( { source => 'Borrower', value => { flags => '0' } } );
    my $sth = $dbh->prepare("INSERT INTO user_permissions (borrowernumber, module_bit, code) VALUES (?,?,?)");
    $sth->execute($patron4->{borrowernumber}, 20, 'edit_public_lists'); # $patron4 only has the edit_public_lists sub-permission checked

    my $biblio1 = $builder->build_sample_biblio;
    my $biblio2 = $builder->build_sample_biblio;
    my $biblio3 = $builder->build_sample_biblio;
    my $biblio4 = $builder->build_sample_biblio;
    my $biblio5 = $builder->build_sample_biblio;

    my $public_shelf = Koha::Virtualshelf->new(
        {   shelfname    => "my first shelf",

            public       => 1,
            allow_change_from_owner => 0,
            allow_change_from_others => 0,
            allow_change_from_staff => 0,
        }
    )->store;

    is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their public list' );
    is( $public_shelf->can_be_viewed( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by another staff member');
    is( $public_shelf->can_be_viewed( $patron3->{borrowernumber} ), 1, 'Public list should be viewed by someone with no special permissions' );
    is( $public_shelf->can_be_viewed( $patron4->{borrowernumber} ), 1, 'Public list should be viewed by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' );
    is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by another staff member' );
    is( $public_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Public list should not be deleted by someone with no special permissions' );
    is( $public_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Public list should not be deleted by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' );
    is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by another staff member' );
    is( $public_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Public list should not be managed by someone with no special permissions' );
    is( $public_shelf->can_be_managed( $patron4->{borrowernumber} ), 1, 'Public list should be managed by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 0, 'The owner should not be able to add biblios to their list' );
    is( $public_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (add) by another staff member' );
    is( $public_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with no special permissions' );
    is( $public_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 0, 'The owner should not be able to remove biblios to their list' );
    is( $public_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (remove) by another staff member' );
    is ( $public_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (removed) by someone with no special permissions' );
    is( $public_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (removed) by someone with the edit_public_lists sub-permission checked' );

    $public_shelf->allow_change_from_owner(1);
    $public_shelf->store;

    is( $public_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their public list' );
    is( $public_shelf->can_be_viewed( $patron2->{borrowernumber} ), 1, 'Public list should be viewed by staff member' );
    is( $public_shelf->can_be_viewed( $patron3->{borrowernumber} ), 1, 'Public list should be viewed by someone with no special permissions' );
    is( $public_shelf->can_be_viewed( $patron4->{borrowernumber} ), 1, 'Public list should be viewable by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' );
    is( $public_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Public list should not be deleted by another staff member' );
    is( $public_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Public list should not be deleted by someone with no special permissions' );
    is( $public_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Public list should not be deleted by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage thier list' );
    is( $public_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Public list should not be managed by another staff member' );
    is( $public_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Public list should not be managed by someone with no special permissions' );
    is( $public_shelf->can_be_managed( $patron4->{borrowernumber} ), 1, 'Public list should be managed by someone with the edit_public_lists sub-permission checked' );

    is( $public_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' );
    is( $public_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (add) by another staff member' );
    is( $public_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with no special permissions' );
    is( $public_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (add) by someone with the edit_public_lists sub-permission checked' );
    is( $public_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone else' );

    is( $public_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' );
    is( $public_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Public list should not be modified (remove) by another staff member' );
    is( $public_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone with no special permissions' );
    is( $public_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Public list should not be modified (remove) by someone with the edit_public_list sub-permission checked' );

    my $private_shelf = Koha::Virtualshelf->new(
        {   shelfname    => "my first shelf",

            public       => 0,
            allow_change_from_owner => 0,
            allow_change_from_others => 0,
            allow_change_from_staff => 0,
        }
    )->store;

    is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' );
    is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' );
    is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' );
    is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' );
    is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' );
    is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' );
    is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' );
    is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' );
    is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' );
    is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 0, 'The owner should not be able to add biblios to their list' );
    is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (add) by another staff member' );
    is( $private_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with no special permissions' );
    is( $private_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 0, 'The owner should not be able to remove biblios to their list' );
    is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 0, 'Private list should not be modified (remove) by another staff member' );
    is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with no special permissions' );
    is( $private_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with the edit_public_lists sub-permissions' );

    $private_shelf->allow_change_from_owner(1);
    $private_shelf->allow_change_from_staff(1);
    $private_shelf->allow_change_from_others(0);
    $private_shelf->store;
    is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' );
    is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' );
    is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' );
    is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' );
    is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' );
    is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' );
    is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' );
    is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' );
    is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' );
    is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' );
    is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 1, 'Private list should not modified (add) by another staff member # individual check done later' );
    is( $private_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with no special permissions' );
    is ( $private_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (add) by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' );
    is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 1, 'Private list should be modified (remove) by another staff member # individual check done later' );
    is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with no special permissions' );
    is( $private_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 0, 'Private list should not be modified (remove) by someone with the edit_public_lists sub-permission checked' );

    $private_shelf->allow_change_from_owner(1);
    $private_shelf->allow_change_from_others(1);
    $private_shelf->store;

    is( $private_shelf->can_be_viewed( $patron1->{borrowernumber} ), 1, 'The owner should be able to view their list' );
    is( $private_shelf->can_be_viewed( $patron2->{borrowernumber} ), 0, 'Private list should not be viewed by another staff member' );
    is( $private_shelf->can_be_viewed( $patron3->{borrowernumber} ), 0, 'Private list should not be viewed by someone with no special permissions' );
    is( $private_shelf->can_be_viewed( $patron4->{borrowernumber} ), 0, 'Private list should not be viewed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_deleted( $patron1->{borrowernumber} ), 1, 'The owner should be able to delete their list' );
    is( $private_shelf->can_be_deleted( $patron2->{borrowernumber} ), 0, 'Private list should not be deleted by another staff member' );
    is( $private_shelf->can_be_deleted( $patron3->{borrowernumber} ), 0, 'Private list should not be deleted by someone with no special permissions' );
    is( $private_shelf->can_be_deleted( $patron4->{borrowernumber} ), 0, 'Private list should not be deleted by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_be_managed( $patron1->{borrowernumber} ), 1, 'The owner should be able to manage their list' );
    is( $private_shelf->can_be_managed( $patron2->{borrowernumber} ), 0, 'Private list should not be managed by another staff member' );
    is( $private_shelf->can_be_managed( $patron3->{borrowernumber} ), 0, 'Private list should not be managed by someone with no special permissions' );
    is( $private_shelf->can_be_managed( $patron4->{borrowernumber} ), 0, 'Private list should not be managed by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_added( $patron1->{borrowernumber} ), 1, 'The owner should be able to add biblios to their list' );
    is( $private_shelf->can_biblios_be_added( $patron2->{borrowernumber} ), 1, 'Private list could be modified (add) by another staff member # individual check done later' );
    is( $private_shelf->can_biblios_be_added( $patron3->{borrowernumber} ), 1, 'Private list could be modified (add) by someone with no special permissions' );
    is( $private_shelf->can_biblios_be_added( $patron4->{borrowernumber} ), 1, 'Private list could be modified (add) by someone with the edit_public_lists sub-permission checked' );

    is( $private_shelf->can_biblios_be_removed( $patron1->{borrowernumber} ), 1, 'The owner should be able to remove biblios to their list' );
    is( $private_shelf->can_biblios_be_removed( $patron2->{borrowernumber} ), 1, 'Private list could be modified (remove) by another staff member # individual check done later' );
    is( $private_shelf->can_biblios_be_removed( $patron3->{borrowernumber} ), 1, 'Private list could be modified (remove) by someone with no special permissions' );
    is( $private_shelf->can_biblios_be_removed( $patron4->{borrowernumber} ), 1, 'Private list could be modified (remove) by someone with the edit_public_lists sub-permission checked' );

    teardown();
};




use Koha::Virtualshelves;

use constant ANYONE => 2;
use constant STAFF  => 3;

my $query = CGI->new;


                public             => $public,
                allow_change_from_owner => $allow_changes_from > 0,
                allow_change_from_others => $allow_changes_from == ANYONE,
                allow_change_from_staff => $allow_changes_from == STAFF,
                owner              => scalar $query->param('owner'),
            }
        );

            my $allow_changes_from = $query->param('allow_changes_from');
            $shelf->allow_change_from_owner( $allow_changes_from > 0 );
            $shelf->allow_change_from_others( $allow_changes_from == ANYONE );
            $shelf->allow_change_from_staff( $allow_changes_from == STAFF );
            $shelf->public( scalar $query->param('public') );
            eval { $shelf->store };

- 

Return to bug 26346

Lines 1910-1915 sub queue_notice { Link Here

(-)a/Koha/Patron.pm (+15 lines)
1910	return \%return;	1910	return \%return;
1911	}	1911	}
1912		1912
		1913	=head3 can_patron_change_staff_only_lists
		1914
		1915	$patron->can_patron_change_staff_only_lists;
		1916
		1917	Return 1 if a patron has 'Superlibrarian' or 'Catalogue' permission.
		1918	Otherwise, return 0.
		1919
		1920	=cut
		1921
		1922	sub can_patron_change_staff_only_lists {
		1923	my ( $self, $params ) = @_;
		1924	return 1 if C4::Auth::haspermission( $self->userid, { 'catalogue' => 1 });
		1925	return 0;
		1926	}
		1927
1913	=head2 Internal methods	1928	=head2 Internal methods
1914		1929
1915	=head3 _type	1930	=head3 _type

Lines 117-134 if ($newvirtualshelf) { Link Here

(-)a/opac/opac-addbybiblionumber.pl (-11 / +44 lines)
117	},	117	},
118	{ join => 'virtualshelfshares', }	118	{ join => 'virtualshelfshares', }
119	);	119	);
120	my $public_shelves = Koha::Virtualshelves->search(	120	my $public_shelves;
121	{ public => 1,	121	if ( $loggedinuser ) {
122	-or => [	122	if ( Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists ) {
123	-and => {	123	$public_shelves = Koha::Virtualshelves->search(
124	allow_change_from_owner => 1,	124	{ public => 1,
125	owner => $loggedinuser,	125	-or => [
		126	-and => {
		127	allow_change_from_owner => 1,
		128	owner => $loggedinuser,
		129	},
		130	allow_change_from_others => 1,
		131	allow_change_from_staff => 1
		132	],
126	},	133	},
127	allow_change_from_others => 1,	134	{ order_by => 'shelfname' }
128	],	135	);
129	},	136	} else {
130	{ order_by => 'shelfname' }	137	$public_shelves = Koha::Virtualshelves->search(
131	);	138	{ public => 1,
		139	-or => [
		140	-and => {
		141	allow_change_from_owner => 1,
		142	owner => $loggedinuser,
		143	},
		144	allow_change_from_others => 1,
		145	],
		146	},
		147	{order_by => 'shelfname' }
		148	);
		149	}
		150	} else {
		151	$public_shelves = Koha::Virtualshelves->search(
		152	{ public => 1,
		153	-or => [
		154	-and => {
		155	allow_change_from_owner => 1,
		156	owner => $loggedinuser,
		157	},
		158	allow_change_from_others => 1,
		159	],
		160	},
		161	{order_by => 'shelfname' }
		162	);
		163	}
		164
132	$template->param(	165	$template->param(
133	private_shelves => $private_shelves,	166	private_shelves => $private_shelves,
134	private_shelves_shared_with_me => $shelves_shared_with_me,	167	private_shelves_shared_with_me => $shelves_shared_with_me,

Lines 22-27 use Koha::Database; Link Here

(-)a/Koha/Virtualshelves.pm (-10 / +31 lines)
22		22
23	use Koha::Virtualshelf;	23	use Koha::Virtualshelf;
24		24
		25
25	use base qw(Koha::Objects);	26	use base qw(Koha::Objects);
26		27
27	=head1 NAME	28	=head1 NAME
Lines 86-102 sub get_some_shelves { Link Here
86	my $add_allowed = $params->{add_allowed};	87	my $add_allowed = $params->{add_allowed};
87		88
88	my @conditions;	89	my @conditions;
		90	my $patron;
		91	my $staffuser = 0;
		92	if ( $borrowernumber != 0 ) {
		93	$patron = Koha::Patrons->find( $borrowernumber );
		94	$staffuser = $patron->can_patron_change_staff_only_lists;
		95	}
89	if ( $add_allowed ) {	96	if ( $add_allowed ) {
90	push @conditions, {	97	if ( $staffuser ) {
91	-or =>	98	push @conditions, {
92	[	99	-or =>
93	{	100	[
94	"me.owner" => $borrowernumber,	101	{
95	"me.allow_change_from_owner" => 1,	102	"me.owner" => $borrowernumber,
96	},	103	"me.allow_change_from_owner" => 1,
97	"me.allow_change_from_others" => 1,	104	},
98	]	105	"me.allow_change_from_others" => 1,
99	};	106	"me.allow_change_from_staff" => 1
		107	]
		108	};
		109	} else {
		110	push @conditions, {
		111	-or =>
		112	[
		113	{
		114	"me.owner" => $borrowernumber,
		115	"me.allow_change_from_owner" => 1,
		116	},
		117	"me.allow_change_from_others" => 1,
		118	]
		119	};
		120	}
100	}	121	}
101	if ( !$public ) {	122	if ( !$public ) {
102	push @conditions, {	123	push @conditions, {

Lines 45-50 use Koha::Virtualshelves; Link Here

(-)a/opac/opac-shelves.pl (-1 / +6 lines)
45	use Koha::RecordProcessor;	45	use Koha::RecordProcessor;
46		46
47	use constant ANYONE => 2;	47	use constant ANYONE => 2;
		48	use constant STAFF => 3;
48		49
49	my $query = CGI->new;	50	my $query = CGI->new;
50		51
Lines 116-121 if ( $op eq 'add_form' ) { Link Here
116	public => $public,	117	public => $public,
117	allow_change_from_owner => $allow_changes_from > 0,	118	allow_change_from_owner => $allow_changes_from > 0,
118	allow_change_from_others => $allow_changes_from == ANYONE,	119	allow_change_from_others => $allow_changes_from == ANYONE,
		120	allow_change_from_staff => $allow_changes_from == STAFF,
119	owner => scalar $loggedinuser,	121	owner => scalar $loggedinuser,
120	}	122	}
121	);	123	);
Lines 147-152 if ( $op eq 'add_form' ) { Link Here
147	my $allow_changes_from = $query->param('allow_changes_from');	149	my $allow_changes_from = $query->param('allow_changes_from');
148	$shelf->allow_change_from_owner( $allow_changes_from > 0 );	150	$shelf->allow_change_from_owner( $allow_changes_from > 0 );
149	$shelf->allow_change_from_others( $allow_changes_from == ANYONE );	151	$shelf->allow_change_from_others( $allow_changes_from == ANYONE );
		152	$shelf->allow_change_from_staff( $allow_changes_from == STAFF );
150	$shelf->public( $public );	153	$shelf->public( $public );
151	eval { $shelf->store };	154	eval { $shelf->store };
152		155
Lines 439-445 if ( $op eq 'list' ) { Link Here
439	),	442	),
440	);	443	);
441	}	444	}
442		445	my $staffuser;
		446	$staffuser = Koha::Patrons->find( $loggedinuser )->can_patron_change_staff_only_lists if $loggedinuser;
443	$template->param(	447	$template->param(
444	op => $op,	448	op => $op,
445	referer => $referer,	449	referer => $referer,
Lines 448-453 $template->param( Link Here
448	public => $public,	452	public => $public,
449	print => scalar $query->param('print') \|\| 0,	453	print => scalar $query->param('print') \|\| 0,
450	listsview => 1,	454	listsview => 1,
		455	staffuser => $staffuser,
451	);	456	);
452		457
453	my $content_type = $query->param('rss')? 'rss' : 'html';	458	my $content_type = $query->param('rss')? 'rss' : 'html';

Lines 19-25 Link Here

(-)a/t/db_dependent/Koha/Patron.t (-1 / +31 lines)
19		19
20	use Modern::Perl;	20	use Modern::Perl;
21		21
22	use Test::More tests => 10;	22	use Test::More tests => 11;
23	use Test::Exception;	23	use Test::Exception;
24	use Test::Warn;	24	use Test::Warn;
25		25
Lines 845-848 subtest 'article_requests() tests' => sub { Link Here
845	is( $article_requests->count, 4, '4 article requests' );	845	is( $article_requests->count, 4, '4 article requests' );
846		846
847	$schema->storage->txn_rollback;	847	$schema->storage->txn_rollback;
		848
		849	};
		850
		851	subtest 'can_patron_change_staff_only_lists() tests' => sub {
		852
		853	plan tests => 3;
		854
		855	$schema->storage->txn_begin;
		856
		857	# make a user with no special permissions
		858	my $patron = $builder->build_object(
		859	{
		860	class => 'Koha::Patrons',
		861	value => {
		862	flags => undef
		863	}
		864	}
		865	);
		866	is( $patron->can_patron_change_staff_only_lists(), 0, 'Patron without permissions cannot change staff only lists');
		867
		868	# make it a 'Catalogue' permission
		869	$patron->set({ flags => 4 })->store->discard_changes;
		870	is( $patron->can_patron_change_staff_only_lists(), 1, 'Catalogue patron can change staff only lists');
		871
		872
		873	# make it a superlibrarian
		874	$patron->set({ flags => 1 })->store->discard_changes;
		875	is( $patron->can_patron_change_staff_only_lists(), 1, 'Superlibrarian patron can change staff only lists');
		876
		877	$schema->storage->txn_rollback;
848	};	878	};

Lines 41-46 use Koha::Patrons; Link Here

(-)a/virtualshelves/shelves.pl (-1 / +3 lines)
41	use Koha::Virtualshelves;	41	use Koha::Virtualshelves;
42		42
43	use constant ANYONE => 2;	43	use constant ANYONE => 2;
		44	use constant STAFF => 3;
44		45
45	my $query = CGI->new;	46	my $query = CGI->new;
46		47
Lines 84-89 if ( $op eq 'add_form' ) { Link Here
84	public => $public,	85	public => $public,
85	allow_change_from_owner => $allow_changes_from > 0,	86	allow_change_from_owner => $allow_changes_from > 0,
86	allow_change_from_others => $allow_changes_from == ANYONE,	87	allow_change_from_others => $allow_changes_from == ANYONE,
		88	allow_change_from_staff => $allow_changes_from == STAFF,
87	owner => scalar $query->param('owner'),	89	owner => scalar $query->param('owner'),
88	}	90	}
89	);	91	);
Lines 113-118 if ( $op eq 'add_form' ) { Link Here
113	my $allow_changes_from = $query->param('allow_changes_from');	115	my $allow_changes_from = $query->param('allow_changes_from');
114	$shelf->allow_change_from_owner( $allow_changes_from > 0 );	116	$shelf->allow_change_from_owner( $allow_changes_from > 0 );
115	$shelf->allow_change_from_others( $allow_changes_from == ANYONE );	117	$shelf->allow_change_from_others( $allow_changes_from == ANYONE );
		118	$shelf->allow_change_from_staff( $allow_changes_from == STAFF );
116	$shelf->public( scalar $query->param('public') );	119	$shelf->public( scalar $query->param('public') );
117	eval { $shelf->store };	120	eval { $shelf->store };
118		121
119	-