gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 18228: Implement the new columns in code 

The two new columns as mentioned in the commit message of the table
revision must be used in the codebase now.

Highlighting some changes in Koha::VirtualShel[f|ves]:
[1] Additional methods is_public and is_private.
[2] Method add_biblio did not check permissions. Does now. No impact on the
    interface, but one call in the unit test was affected.
[3] Method remove_biblios is signficantly simplified. Removed a FIXME.
[4] Method can_biblios_be_removed now redirects to can_biblios_be_added.
    A followup report may deal with unifying those routines.
[5] Condition in get_some_shelves changed.
[6] The reference to allow_add in get_shelves_containing_record can simply
    be removed.

opac-shelves.pl and shelves.pl now pass the default setting of Owner only
to the template.
Templates shelves.tt and opac-shelves.tt now include the new permission
field with three choices as mentioned in the table revision patch.

opac-addbybiblionumber.pl and addbybiblionumber now need a check on
allow_change_from_owner; search conditions slightly adjusted to the new
permission scheme.

Test plan:
When we refer to visibility in the test plan, please check the Add to-combo
on opac search results and staff results. And check opac-addbybiblionumber
by clicking Save to Lists from opac results.
The step 'Check delete' means: open the list in opac and check if you see
the Delete button below the entries (only check, do not delete).

[ 1] Create private list I01 (perm=Owner)
[ 2] Check visibility: Seen.
[ 3] Add a book. (Change by owner should be allowed.)
[ 4] Check delete: Yes.
[ 5] Edit list I01, set perm=Nobody
[ 6] Check visibility: Not seen.
[ 7] Check delete: No.
[ 8] Share list I01 with another patron.
[ 9] Check visibility for the other patron: Not seen.
[10] Check delete for the other patron: No.
[11] Change permission of list I01 to Anyone (by owner).
[12] Check visibility for the other patron: Seen.
[13] Let other patron add a book (change is allowed).
[14] Let owner delete the same book again (change allowed).

[15] Create public list U01 (perm=Owner)
[16] Check visibility: Seen.
[17] Add a book. (Change by owner should be allowed.)
[18] Login as other user. Check visibility: Not seen. Check delete: No.
[19] Change permission of U01 to Nobody (by owner)
[20] As owner: Check visibility: Not seen. Check delete: No.
[21] As other user: Check visibility: Not seen. Check delete: No.
[22] Create public list U02 (perm=Anyone)
[23] Add a book by owner.
[24] Delete the same book by other user. Add another book.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2017-03-08 15:31:25 +01:00 committed by Jonathan Druart
parent 1ef053564f
commit a58aca056b
8 changed files with 98 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
Koha/Virtualshelf.pm
View File

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

12
Koha/Virtualshelves.pm
View File

@ -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,
},
}
},

35
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
View File

@ -299,13 +299,13 @@ function placeHold () {
[% ELSE %]
Lists
[% END %]
[% IF shelf AND shelf.category == PRIVATE %] &rsaquo;
[% IF shelf AND shelf.is_private %] &rsaquo;
[% IF op == 'view' OR op == 'edit_form' %]
<a href="/cgi-bin/koha/virtualshelves/shelves.pl?op=list&amp;category=[% PRIVATE %]">Your lists</a>
[% ELSE %]
Your lists
[% END %]
[% ELSIF shelf AND shelf.category == PUBLIC %] &rsaquo;
[% ELSIF shelf AND shelf.is_public %] &rsaquo;
[% IF op == 'view' %]
<a href="/cgi-bin/koha/virtualshelves/shelves.pl?op=list&amp;category=[% PUBLIC %]">Public lists</a>
[% ELSE %]
@ -537,12 +537,12 @@ function placeHold () {
</select></li>
<li><label for="category">Category: </label>
<select id="category" name="category">
[% IF shelf.category == PRIVATE %]
[% IF shelf.is_private %]
<option value="1" selected="selected">Private</option>
[% ELSE %]
<option value="1">Private</option>
[% END %]
[% IF shelf.category == PUBLIC %]
[% IF shelf.is_public %]
<option value="2" selected="selected">Public</option>
[% ELSE %]
<option value="2">Public</option>
@ -550,29 +550,14 @@ function placeHold () {
</select></li>
<li>
<label for="allow_add">Permissions: </label>
<select name="allow_add" id="allow_add">
[% IF shelf.allow_add %]<option value="0">Do not allow</option>[% ELSE %]<option value="0" selected="selected">Do not allow</option>[% END %]
[% IF shelf.allow_add %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
<label for="allow_changes_from">Allow changes to contents from: </label>
<select name="allow_changes_from" id="allow_changes_from">
[% IF !shelf.allow_change_from_owner %]<option value="0">Nobody</option>[% ELSE %]<option value="0" selected="selected">Nobody</option>[% END %]
[% IF shelf.allow_change_from_owner && !shelf.allow_change_from_others %]<option value="1" selected="selected">Owner only</option>[% ELSE %]<option value="1">Owner only</option>[% END %]
[% IF shelf.allow_change_from_owner && 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 %]
</select>
&nbsp;<span>anyone else to add entries.</span>
</li>
<li>
<label>&nbsp;</label>
<select name="allow_delete_own" id="allow_delete_own">
[% IF shelf and not shelf.allow_delete_own %]<option value="0" selected="selected">Do not allow</option>[% ELSE %]<option value="0">Do not allow</option>[% END %]
[% IF not shelf or shelf.allow_delete_own %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
</select>
&nbsp;<span>anyone to remove their own contributed entries.</span>
</li>
<li>
<label>&nbsp;</label>
<select name="allow_delete_other" id="allow_delete_other">
[% IF shelf.allow_delete_other %]<option value="0">Do not allow</option>[% ELSE %]<option value="0" selected="selected">Do not allow</option>[% END %]
[% IF shelf.allow_delete_other %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
</select>
&nbsp;<span>anyone to remove other contributed entries.</span>
</li>
</ol>
</fieldset>

44
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
View File

@ -12,28 +12,12 @@
[% BLOCK list_permissions %]
<li>
<label for="allow_add">Permissions: </label>
<select name="allow_add" id="allow_add">
[% IF shelf.allow_add %]<option value="0">Do not allow</option>[% ELSE %]<option value="0" selected="selected">Do not allow</option>[% END %]
[% IF shelf.allow_add %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
<label for="allow_changes_from">Allow changes to contents from: </label>
<select name="allow_changes_from" id="allow_changes_from">
[% IF !shelf.allow_change_from_owner %]<option value="0">Nobody</option>[% ELSE %]<option value="0" selected="selected">Nobody</option>[% END %]
[% IF shelf.allow_change_from_owner && !shelf.allow_change_from_others %]<option value="1" selected="selected">Owner only</option>[% ELSE %]<option value="1">Owner only</option>[% END %]
[% IF shelf.allow_change_from_owner && 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 %]
</select>
&nbsp;<span>anyone else to add entries.</span>
</li>
<li>
<label>&nbsp;</label>
<select name="allow_delete_own" id="allow_delete_own">
[% IF shelf and not shelf.allow_delete_own %]<option value="0" selected="selected">Do not allow</option>[% ELSE %]<option value="0">Do not allow</option>[% END %]
[% IF not shelf or shelf.allow_delete_own %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
</select>
&nbsp;<span>anyone to remove their own contributed entries.</span>
</li>
<li>
<label>&nbsp;</label>
<select name="allow_delete_other" id="allow_delete_other">
[% IF shelf.allow_delete_other %]<option value="0">Do not allow</option>[% ELSE %]<option value="0" selected="selected">Do not allow</option>[% END %]
[% IF shelf.allow_delete_other %]<option value="1" selected="selected">Allow</option>[% ELSE %]<option value="1">Allow</option>[% END %]
</select>
&nbsp;<span>anyone to remove other contributed entries.</span>
</li>
[% END %]
@ -51,13 +35,13 @@
<li>Lists</li>
[% END %]
[% IF shelf and shelf.category == PRIVATE %]
[% IF shelf and shelf.is_private %]
[% IF op == 'view' OR op == 'edit_form' %]
<li><span class="divider">&rsaquo;</span> <a href="/cgi-bin/koha/opac-shelves.pl?op=list&amp;category=[% PRIVATE %]">Your lists</a></li>
[% ELSE %]
<li><span class="divider">&rsaquo;</span> Your lists</li>
[% END %]
[% ELSIF shelf AND shelf.category == PUBLIC %]
[% ELSIF shelf AND shelf.is_public %]
[% IF op == 'view' %]
<li><span class="divider">&rsaquo;</span> <a href="/cgi-bin/koha/opac-shelves.pl?op=list&amp;category=[% PUBLIC %]">Public lists</a></li>
[% ELSE %]
@ -586,25 +570,23 @@
<li>
<label for="category">Category:</label>
<select name="category" id="category">
[% IF shelf.category == PRIVATE %]
[% IF shelf.is_private %]
<option value="1" selected="selected">Private</option>
[% ELSE %]
<option value="1">Private</option>
[% END %]
[% IF shelf.category == PUBLIC %]
[% IF shelf.is_public %]
<option value="2" selected="selected">Public</option>
[% ELSE %]
<option value="2">Public</option>
[% END %]
</select>
[% IF shelf.category == PUBLIC AND NOT Koha.Preference('OpacAllowPublicListCreation') %]
[% IF shelf.is_public AND NOT Koha.Preference('OpacAllowPublicListCreation') %]
<span class="hint alert alert-info">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.</span>
[% END %]
</li>
[% END %]
[% IF shelf.category == PUBLIC OR shelf.is_shared %]
[% INCLUDE list_permissions %]
[% END %]
[% INCLUDE list_permissions %]
</ol>
[% UNLESS Koha.Preference('OpacAllowPublicListCreation') OR category == PUBLIC %]
<input type="hidden" name="category" value="[% PRIVATE %]" />
@ -671,7 +653,7 @@
<td><a href="/cgi-bin/koha/opac-shelves.pl?op=view&amp;shelfnumber=[% s.shelfnumber %]">[% s.shelfname |html %]</a></td>
<td>[% IF contents.count %][% contents.count %] [% IF contents.count == 1 %]item[% ELSE %]items[% END %][% ELSE %]Empty[% END %]</td>
<td>
[% IF s.category == PRIVATE %]
[% IF s.is_private %]
[% IF s.is_shared %]Shared[% ELSE %]Private[% END %]
[% ELSE %]
Public
@ -696,7 +678,7 @@
<input type="submit" class="deleteshelf" onclick="return confirmDelete(MSG_CONFIRM_DELETE_LIST);" value="Delete" />
</form>
[% END %]
[% IF s.category == PRIVATE AND s.can_be_managed( loggedinusernumber ) AND Koha.Preference('OpacAllowSharingPrivateLists') %]
[% IF s.is_private AND s.can_be_managed( loggedinusernumber ) AND Koha.Preference('OpacAllowSharingPrivateLists') %]
<a href="/cgi-bin/koha/opac-shareshelf.pl?op=invite&shelfnumber=[% s.shelfnumber |html %]" class="">Share</a>
[% END %]
[% IF s.is_shared_with( loggedinusernumber ) %]

17
opac/opac-addbybiblionumber.pl
View File

@ -107,25 +107,26 @@ if ($newvirtualshelf) {
my $private_shelves = Koha::Virtualshelves->search(
{ category => 1,
owner => $loggedinuser,
allow_change_from_owner => 1,
},
{ order_by => 'shelfname' }
);
my $shelves_shared_with_me = Koha::Virtualshelves->search(
{ category => 1,
'virtualshelfshares.borrowernumber' => $loggedinuser,
-or => {
allow_add => 1,
owner => $loggedinuser,
}
allow_change_from_others => 1,
},
{ join => 'virtualshelfshares', }
);
my $public_shelves = Koha::Virtualshelves->search(
{ category => 2,
-or => {
allow_add => 1,
owner => $loggedinuser,
}
-or => [
-and => {
allow_change_from_owner => 1,
owner => $loggedinuser,
},
allow_change_from_others => 1,
],
},
{ order_by => 'shelfname' }
);

17
opac/opac-shelves.pl
View File

@ -33,6 +33,8 @@ use Koha::Biblioitems;
use Koha::Virtualshelves;
use Koha::RecordProcessor;
use constant ANYONE => 2;
my $query = new CGI;
my $template_name = $query->param('rss') ? "opac-shelves-rss.tt" : "opac-shelves.tt";
@ -56,7 +58,8 @@ my $category = $query->param('category') || 1;
my ( $shelf, $shelfnumber, @messages );
if ( $op eq 'add_form' ) {
# Nothing to do
# Only pass default
$shelf = { allow_change_from_owner => 1 };
} elsif ( $op eq 'edit_form' ) {
$shelfnumber = $query->param('shelfnumber');
$shelf = Koha::Virtualshelves->find($shelfnumber);
@ -74,14 +77,14 @@ if ( $op eq 'add_form' ) {
}
} elsif ( $op eq 'add' ) {
if ( $loggedinuser ) {
my $allow_changes_from = $query->param('allow_changes_from');
eval {
$shelf = Koha::Virtualshelf->new(
{ shelfname => scalar $query->param('shelfname'),
sortfield => scalar $query->param('sortfield'),
category => scalar $query->param('category') || 1,
allow_add => scalar $query->param('allow_add'),
allow_delete_own => scalar $query->param('allow_delete_own'),
allow_delete_other => scalar $query->param('allow_delete_other'),
allow_change_from_owner => $allow_changes_from > 0,
allow_change_from_others => $allow_changes_from == ANYONE,
owner => scalar $loggedinuser,
}
);
@ -110,9 +113,9 @@ if ( $op eq 'add_form' ) {
if ( $shelf->can_be_managed( $loggedinuser ) ) {
$shelf->shelfname( scalar $query->param('shelfname') );
$shelf->sortfield( $sortfield );
$shelf->allow_add( scalar $query->param('allow_add') );
$shelf->allow_delete_own( scalar $query->param('allow_delete_own') );
$shelf->allow_delete_other( scalar $query->param('allow_delete_other') );
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->category( scalar $query->param('category') );
eval { $shelf->store };

17
virtualshelves/addbybiblionumber.pl
View File

@ -152,25 +152,26 @@ if ($newvirtualshelf) {
my $private_shelves = Koha::Virtualshelves->search(
{ category => 1,
owner => $loggedinuser,
allow_change_from_owner => 1,
},
{ order_by => 'shelfname' }
);
my $shelves_shared_with_me = Koha::Virtualshelves->search(
{ category => 1,
'virtualshelfshares.borrowernumber' => $loggedinuser,
-or => {
allow_add => 1,
owner => $loggedinuser,
}
allow_change_from_others => 1,
},
{ join => 'virtualshelfshares', }
);
my $public_shelves = Koha::Virtualshelves->search(
{ category => 2,
-or => {
allow_add => 1,
owner => $loggedinuser,
}
-or => [
-and => {
allow_change_from_owner => 1,
owner => $loggedinuser,
},
allow_change_from_others => 1,
],
},
{ order_by => 'shelfname' }
);

17
virtualshelves/shelves.pl
View File

@ -32,6 +32,8 @@ use Koha::Biblioitems;
use Koha::CsvProfiles;
use Koha::Virtualshelves;
use constant ANYONE => 2;
my $query = new CGI;
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
@ -49,7 +51,8 @@ my $category = $query->param('category') || 1;
my ( $shelf, $shelfnumber, @messages );
if ( $op eq 'add_form' ) {
# Nothing to do
# Only pass default
$shelf = { allow_change_from_owner => 1 };
} elsif ( $op eq 'edit_form' ) {
$shelfnumber = $query->param('shelfnumber');
$shelf = Koha::Virtualshelves->find($shelfnumber);
@ -66,14 +69,14 @@ if ( $op eq 'add_form' ) {
push @messages, { type => 'alert', code => 'does_not_exist' };
}
} elsif ( $op eq 'add' ) {
my $allow_changes_from = $query->param('allow_changes_from');
eval {
$shelf = Koha::Virtualshelf->new(
{ shelfname => scalar $query->param('shelfname'),
sortfield => scalar $query->param('sortfield'),
category => scalar $query->param('category'),
allow_add => scalar $query->param('allow_add'),
allow_delete_own => scalar $query->param('allow_delete_own'),
allow_delete_other => scalar $query->param('allow_delete_other'),
allow_change_from_owner => $allow_changes_from > 0,
allow_change_from_others => $allow_changes_from == ANYONE,
owner => scalar $query->param('owner'),
}
);
@ -100,9 +103,9 @@ if ( $op eq 'add_form' ) {
if ( $shelf->can_be_managed( $loggedinuser ) ) {
$shelf->shelfname( scalar $query->param('shelfname') );
$shelf->sortfield( $sortfield );
$shelf->allow_add( scalar $query->param('allow_add') );
$shelf->allow_delete_own( scalar $query->param('allow_delete_own') );
$shelf->allow_delete_other( scalar $query->param('allow_delete_other') );
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->category( scalar $query->param('category') );
eval { $shelf->store };