@@ -, +, @@
interface, but one call in the unit test was affected.
A followup report may deal with unifying those routines.
be removed.
---
Koha/Virtualshelf.pm | 74 +++++++++-------------
Koha/Virtualshelves.pm | 12 ++--
.../prog/en/modules/virtualshelves/shelves.tt | 35 +++-------
.../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 44 ++++---------
opac/opac-addbybiblionumber.pl | 17 ++---
opac/opac-shelves.pl | 17 +++--
virtualshelves/addbybiblionumber.pl | 17 ++---
virtualshelves/shelves.pl | 17 +++--
8 files changed, 98 insertions(+), 135 deletions(-)
--- a/Koha/Virtualshelf.pm
+++ a/Koha/Virtualshelf.pm
@@ -60,18 +60,26 @@ sub store {
Koha::Exceptions::Virtualshelves::DuplicateObject->throw;
}
- $self->allow_add( 0 )
- unless defined $self->allow_add;
- $self->allow_delete_own( 1 )
- unless defined $self->allow_delete_own;
- $self->allow_delete_other( 0 )
- unless defined $self->allow_delete_other;
+ $self->allow_change_from_owner( 1 )
+ unless defined $self->allow_change_from_owner;
+ $self->allow_change_from_others( 0 )
+ unless defined $self->allow_change_from_others;
$self->created_on( dt_from_string );
return $self->SUPER::store( $self );
}
+sub is_public {
+ my ( $self ) = @_;
+ return $self->category == $PUBLIC;
+}
+
+sub is_private {
+ my ( $self ) = @_;
+ return $self->category == $PRIVATE;
+}
+
sub is_shelfname_valid {
my ( $self ) = @_;
@@ -80,14 +88,14 @@ sub is_shelfname_valid {
( $self->shelfnumber ? ( "me.shelfnumber" => { '!=', $self->shelfnumber } ) : () ),
};
- if ( $self->category == $PRIVATE and defined $self->owner ) {
+ if ( $self->is_private and defined $self->owner ) {
$conditions->{-or} = {
"virtualshelfshares.borrowernumber" => $self->owner,
"me.owner" => $self->owner,
};
$conditions->{category} = $PRIVATE;
}
- elsif ( $self->category == $PRIVATE and not defined $self->owner ) {
+ elsif ( $self->is_private and not defined $self->owner ) {
$conditions->{owner} = undef;
$conditions->{category} = $PRIVATE;
}
@@ -174,6 +182,10 @@ sub add_biblio {
}
)->count;
return if $already_exists;
+
+ # Check permissions
+ return unless ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) || $self->allow_change_from_others;
+
my $content = Koha::Virtualshelfcontent->new(
{
shelfnumber => $self->shelfnumber,
@@ -194,38 +206,18 @@ sub remove_biblios {
return unless @$biblionumbers;
my $number_removed = 0;
- for my $biblionumber ( @$biblionumbers ) {
- if ( $self->owner == $borrowernumber or $self->allow_delete_own ) {
- $number_removed += $self->get_contents->search(
- {
- biblionumber => $biblionumber,
- borrowernumber => $borrowernumber,
- }
- )->delete;
- }
- if ( $self->allow_delete_other ) {
- $number_removed += $self->get_contents->search(
- {
- biblionumber => $biblionumber,
- # FIXME
- # This does not make sense, but it's has been backported from DelFromShelf.
- # Why shouldn't we allow to remove his own contribution if allow_delete_other is on?
- borrowernumber => {
- -or => {
- '!=' => $borrowernumber,
- '=' => undef
- }
- },
- }
- )->delete;
- }
+ if( ( $self->owner == $borrowernumber && $self->allow_change_from_owner )
+ || $self->allow_change_from_others ) {
+ $number_removed += $self->get_contents->search({
+ biblionumber => $biblionumbers,
+ })->delete;
}
return $number_removed;
}
sub can_be_viewed {
my ( $self, $borrowernumber ) = @_;
- return 1 if $self->category == $PUBLIC;
+ return 1 if $self->is_public;
return 0 unless $borrowernumber;
return 1 if $self->owner == $borrowernumber;
return $self->get_shares->search(
@@ -243,7 +235,7 @@ sub can_be_deleted {
my $patron = Koha::Patrons->find( $borrowernumber );
- return 1 if $self->category == $PUBLIC and C4::Auth::haspermission( $patron->userid, { lists => 'delete_public_lists' } );
+ return 1 if $self->is_public and C4::Auth::haspermission( $patron->userid, { lists => 'delete_public_lists' } );
return 0;
}
@@ -260,20 +252,14 @@ sub can_biblios_be_added {
return 1
if $borrowernumber
- and ( $self->owner == $borrowernumber
- or $self->allow_add );
+ and ( ( $self->owner == $borrowernumber && $self->allow_change_from_owner ) or $self->allow_change_from_others );
return 0;
}
sub can_biblios_be_removed {
my ( $self, $borrowernumber ) = @_;
-
- return 1
- if $borrowernumber
- and ( $self->owner == $borrowernumber
- or $self->allow_delete_own
- or $self->allow_delete_other );
- return 0;
+ return $self->can_biblios_be_added( $borrowernumber );
+ # Same answer since bug 18228
}
sub _type {
--- a/Koha/Virtualshelves.pm
+++ a/Koha/Virtualshelves.pm
@@ -90,10 +90,13 @@ sub get_some_shelves {
if ( $add_allowed ) {
push @conditions, {
-or =>
- {
- "me.allow_add" => 1,
- "me.owner" => $borrowernumber,
- }
+ [
+ {
+ "me.owner" => $borrowernumber,
+ "me.allow_change_from_owner" => 1,
+ },
+ "me.allow_change_from_others" => 1,
+ ]
};
}
if ( $category == 1 ) {
@@ -135,7 +138,6 @@ sub get_shelves_containing_record {
'me.owner' => $borrowernumber,
-or => {
'virtualshelfshares.borrowernumber' => $borrowernumber,
- "me.allow_add" => 1,
},
}
},
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
@@ -299,13 +299,13 @@ function placeHold () {
[% ELSE %]
Lists
[% END %]
- [% IF shelf AND shelf.category == PRIVATE %] ›
+ [% IF shelf AND shelf.is_private %] ›
[% IF op == 'view' OR op == 'edit_form' %]
Your lists
[% ELSE %]
Your lists
[% END %]
- [% ELSIF shelf AND shelf.category == PUBLIC %] ›
+ [% ELSIF shelf AND shelf.is_public %] ›
[% IF op == 'view' %]
Public lists
[% ELSE %]
@@ -537,12 +537,12 @@ function placeHold () {
-
-
-
-
-
- [% IF shelf and not shelf.allow_delete_own %][% ELSE %][% END %]
- [% IF not shelf or shelf.allow_delete_own %][% ELSE %][% END %]
-
- anyone to remove their own contributed entries.
-
-
-
-
- [% IF shelf.allow_delete_other %][% ELSE %][% END %]
- [% IF shelf.allow_delete_other %][% ELSE %][% END %]
-
- anyone to remove other contributed entries.
-
-
- [% IF shelf.allow_add %][% ELSE %][% END %]
- [% IF shelf.allow_add %][% ELSE %][% END %]
+
+
+ [% IF !shelf.allow_change_from_owner %][% ELSE %][% END %]
+ [% IF shelf.allow_change_from_owner && !shelf.allow_change_from_others %][% ELSE %][% END %]
+ [% IF shelf.allow_change_from_owner && shelf.allow_change_from_others %][% ELSE %][% END %]
- anyone else to add entries.
-
-
-
-
- [% IF shelf and not shelf.allow_delete_own %][% ELSE %][% END %]
- [% IF not shelf or shelf.allow_delete_own %][% ELSE %][% END %]
-
- anyone to remove their own contributed entries.
-
-
-
-
- [% IF shelf.allow_delete_other %][% ELSE %][% END %]
- [% IF shelf.allow_delete_other %][% ELSE %][% END %]
-
- anyone to remove other contributed entries.
[% END %]
@@ -51,13 +35,13 @@
Lists
[% END %]
- [% IF shelf and shelf.category == PRIVATE %]
+ [% IF shelf and shelf.is_private %]
[% IF op == 'view' OR op == 'edit_form' %]
- [% IF shelf.category == PRIVATE %]
+ [% IF shelf.is_private %]
[% ELSE %]
[% END %]
- [% IF shelf.category == PUBLIC %]
+ [% IF shelf.is_public %]
[% ELSE %]
[% END %]
- [% IF shelf.category == PUBLIC AND NOT Koha.Preference('OpacAllowPublicListCreation') %]
+ [% IF shelf.is_public AND NOT Koha.Preference('OpacAllowPublicListCreation') %]
The library has disabled the ability for patrons to create new public lists. If you make your list private, you will not be able to make it public again.
[% END %]
[% END %]
- [% IF shelf.category == PUBLIC OR shelf.is_shared %]
- [% INCLUDE list_permissions %]
- [% END %]
+ [% INCLUDE list_permissions %]
[% UNLESS Koha.Preference('OpacAllowPublicListCreation') OR category == PUBLIC %]
@@ -671,7 +653,7 @@