Bug 33363: Add separate permissions for creating and deleting suggestions

There are a number of libraries that would like for their staff to be able to submit (and view existing) purchase suggestions from the borrower record, but not give the staff the ability to edit/manage/delete purchase suggestions.

Test Plan:
1) Apply this patch
2) Run restart all the things!
3) Run updatedatabase
4) Verify anyone with the suggestions manage permissions now has the create and delete permissions as well
5) Verify anyone without the suggestions manage permission has not recieved the new permissions
6) Enable only the create permission for a librarian
7) Verify that librarian can only create new suggestions ( not manage or delete )
8) Enable only the manage permission for a librarian
9) Verify that librarian can only manage existing suggestions ( not create or delete )
10) Enable only the delete permission for a librarian
11) Verify that librarian can only delete suggestions ( not create or manage )

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Sponsored-by: Cuyahoga County Public Library
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Kyle M Hall 2023-04-26 15:21:39 +00:00 committed by Katrin Fischer
parent a3f4c9ec8a
commit 87b3399812
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
13 changed files with 181 additions and 63 deletions

View File

@ -41,7 +41,10 @@
$ref: "../swagger.yaml#/definitions/error"
x-koha-authorization:
permissions:
suggestions: suggestions_manage
suggestions:
- suggestions_manage
- suggestions_delete
- suggestions_create
post:
x-mojo-to: Suggestions#add
operationId: addSuggestions
@ -100,7 +103,7 @@
$ref: "../swagger.yaml#/definitions/error"
x-koha-authorization:
permissions:
suggestions: suggestions_manage
suggestions: suggestions_create
"/suggestions/{suggestion_id}":
get:
x-mojo-to: Suggestions#get
@ -139,7 +142,10 @@
$ref: "../swagger.yaml#/definitions/error"
x-koha-authorization:
permissions:
suggestions: suggestions_manage
suggestions:
- suggestions_manage
- suggestions_delete
- suggestions_create
put:
x-mojo-to: Suggestions#update
operationId: updateSuggestion
@ -229,7 +235,7 @@
$ref: "../swagger.yaml#/definitions/error"
x-koha-authorization:
permissions:
suggestions: suggestions_manage
suggestions: suggestions_delete
/suggestions/managers:
get:
x-mojo-to: Suggestions#list_managers
@ -281,4 +287,7 @@
$ref: "../swagger.yaml#/definitions/error"
x-koha-authorization:
permissions:
suggestions: suggestions_manage
suggestions:
- suggestions_manage
- suggestions_delete
- suggestions_create

View File

@ -0,0 +1,25 @@
use Modern::Perl;
return {
bug_number => "33363",
description =>
"Split suggestions_manage into three separate permissions for creating, updating, and deleting suggetions",
up => sub {
my ($args) = @_;
my ( $dbh, $out ) = @$args{qw(dbh out)};
$dbh->do(
q{INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (12, 'suggestions_create', 'Create purchase suggestions')}
);
$dbh->do(
q{INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (12, 'suggestions_delete', 'Update purchase suggestions')}
);
$dbh->do(
q{INSERT IGNORE INTO user_permissions (borrowernumber, module_bit, code) SELECT borrowernumber, 12, 'suggestions_create' FROM borrowers WHERE flags & (1 << 2)}
);
$dbh->do(
q{INSERT IGNORE INTO user_permissions (borrowernumber, module_bit, code) SELECT borrowernumber, 12, 'suggestions_delete' FROM borrowers WHERE flags & (1 << 2)}
);
},
};

View File

@ -91,7 +91,9 @@ INSERT INTO permissions (module_bit, code, description) VALUES
(11, 'delete_invoices', 'Delete invoices'),
(11, 'merge_invoices', 'Merge invoices'),
(11, 'delete_baskets', 'Delete baskets'),
(12, 'suggestions_create', 'Create purchase suggestions'),
(12, 'suggestions_manage', 'Manage purchase suggestions'),
(12, 'suggestions_delete', 'Delete purchase suggestions'),
(13, 'edit_additional_contents', 'Write additional contents for the OPAC and staff interfaces (news and HTML customizations)'),
(13, 'label_creator', 'Create printable labels and barcodes from catalog and patron data'),
(13, 'edit_calendar', 'Define days when the library is closed'),

View File

@ -6,7 +6,7 @@
<li><a href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions home</a></li>
<li><a href="/cgi-bin/koha/acqui/histsearch.pl">Advanced search</a></li>
[% IF ( CAN_user_acquisition_order_receive ) %]<li><a href="/cgi-bin/koha/acqui/lateorders.pl">Late orders</a></li>[% END %]
[% IF ( CAN_user_suggestions_suggestions_manage ) %]<li><a href="/cgi-bin/koha/suggestion/suggestion.pl">Suggestions</a></li>[% END %]
[% IF ( suggestion && ( CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete ) ) %]<li><a href="/cgi-bin/koha/suggestion/suggestion.pl">Suggestions</a></li>[% END %]
<li><a href="/cgi-bin/koha/acqui/invoices.pl">Invoices</a></li>
[% IF Koha.Preference('EDIFACT') && CAN_user_acquisition_edi_manage %]
<li><a href="/cgi-bin/koha/acqui/edifactmsgs.pl">EDIFACT messages</a></li>

View File

@ -226,7 +226,7 @@
[% END %]
[% END %]
[% IF CAN_user_suggestions_suggestions_manage %]
[% IF CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete %]
[% IF ( suggestionsview ) %]<li class="active">[% ELSE %]<li>[% END %]
<a href="/cgi-bin/koha/members/purchase-suggestions.pl?borrowernumber=[% patron.borrowernumber | uri %]">Purchase suggestions</a>
</li>

View File

@ -59,7 +59,7 @@
[% IF ( CAN_user_reports ) %]
<li><a href="/cgi-bin/koha/reports/reports-home.pl">Reports</a></li>
[% END %]
[% IF ( CAN_user_suggestions_suggestions_manage ) %]
[% IF ( CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete ) %]
<li><a href="/cgi-bin/koha/suggestion/suggestion.pl">Suggestions</a></li>
[% END %]
[% IF ( CAN_user_tools || CAN_user_clubs ) %]

View File

@ -127,7 +127,7 @@
[% END %]
[% IF filter == 'suggestions_managers' %]
<div class="note">Only staff with superlibrarian or suggestions_manage permissions are returned in the search results</div>
<div class="note">Only staff with superlibrarian or full suggestions permissions are returned in the search results</div>
[% ELSIF filter == 'orders_managers' OR filter == 'baskets_managers' %]
<div class="note">Only staff with superlibrarian or acquisitions permissions (or order_manage permission if granular permissions are enabled) are returned in the search results</div>
[% ELSIF filter == 'funds_owners' OR filter == 'funds_users' %]

View File

@ -437,11 +437,21 @@
Add manual credits to a patron account
</span>
<span class="permissioncode">([% name | html %])</span>
[%- CASE 'suggestions_create' -%]
<span class="sub_permission suggestions_create_subpermission">
Create purchase suggestions
</span>
<span class="permissioncode">([% name | html %])</span>
[%- CASE 'suggestions_manage' -%]
<span class="sub_permission suggestions_manage_subpermission">
Manage purchase suggestions
</span>
<span class="permissioncode">([% name | html %])</span>
[%- CASE 'suggestions_delete' -%]
<span class="sub_permission suggestions_delete_subpermission">
Delete purchase suggestions
</span>
<span class="permissioncode">([% name | html %])</span>
[%- CASE 'budget_add_del' -%]
<span class="sub_permission budget_add_del_subpermission">
Add and delete funds (but can't modify funds)

View File

@ -44,7 +44,9 @@
<h1>Purchase suggestions</h1>
<div id="toolbar" class="btn-toolbar">
<a class="btn btn-default" id="newsuggestion" href="/cgi-bin/koha/suggestion/suggestion.pl?op=add_form&amp;suggestedby=[% patron.borrowernumber | html %]&amp;redirect=purchase_suggestions&amp;borrowernumber=[% patron.borrowernumber | html %]"><i class="fa fa-plus"></i> New purchase suggestion</a>
[% IF CAN_user_suggestions_suggestions_create %]
<a class="btn btn-default" id="newsuggestion" href="/cgi-bin/koha/suggestion/suggestion.pl?op=add_form&amp;suggestedby=[% patron.borrowernumber | html %]&amp;redirect=purchase_suggestions&amp;borrowernumber=[% patron.borrowernumber | html %]"><i class="fa fa-plus"></i> New purchase suggestion</a>
[% END %]
</div>
[% IF suggestions.size %]

View File

@ -97,13 +97,17 @@
[% INCLUDE 'messages.inc' %]
<div id="toolbar" class="btn-toolbar">
<a class="btn btn-default" id="editsuggestion" href="suggestion.pl?op=edit_form&amp;suggestionid=[% suggestionid | html %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a>
<form class="delete_form" method="post" action="/cgi-bin/koha/suggestion/suggestion.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="op" value="cud-delete" />
<input type="hidden" name="suggestionid" value="[% suggestionid | html %]" />
<button type="submit" class="btn btn-default deletesuggestion"><i class="fa fa-trash-can"></i> Delete</button>
</form>
[% IF CAN_user_suggestions_suggestions_manage %]
<a class="btn btn-default" id="editsuggestion" href="suggestion.pl?op=edit_form&amp;suggestionid=[% suggestionid | html %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a>
[% END %]
[% IF CAN_user_suggestions_suggestions_delete %]
<form class="delete_form" method="post" action="/cgi-bin/koha/suggestion/suggestion.pl">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="op" value="cud-delete" />
<input type="hidden" name="suggestionid" value="[% suggestionid | html %]" />
<button type="submit" class="btn btn-default deletesuggestion"><i class="fa fa-trash-can"></i> Delete</button>
</form>
[% END %]
</div>
<fieldset class="rows">
@ -338,6 +342,12 @@
<span>A similar document already exists: <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% m.id | uri %]">[% m.title | html %]</a>. Click on "Confirm your suggestion" to ignore this message.</span>
[% CASE 'manager_not_enough_permissions' %]
<span>The manager you selected does not have sufficient permissions.</span>
[% CASE 'no_manage_permission' %]
<span>You do not have permissions to manage suggestions</span>
[% CASE 'no_create_permission' %]
<span>You do not have permissions to create suggestions</span>
[% CASE 'no_delete_permission' %]
<span>You do not have permissions to delete suggestions</span>
[% CASE %]
<span>[% m.code | html %]</span>
[% END %]
@ -558,7 +568,11 @@
<label for="managedby_name">by:</label>
<div>
<span id="managedby_name" name="managedby_name">
<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% logged_in_user.borrowernumber | uri %]">You</a>
[% IF CAN_user_suggestions_suggestions_manage %]
<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% logged_in_user.borrowernumber | uri %]">You</a>
[% ELSE %]
Nobody
[% END %]
</span>
[% IF managedby_patron.borrowernumber && logged_in_user.borrowernumber != managedby_patron.borrowernumber %]
| Previously was [% INCLUDE 'patron-title.inc' patron=managedby_patron hide_patron_infos_if_needed=1 %] [% Branches.GetName( managedby_patron.branchcode ) | html %] ([% managedby_patron.category.description | html %])
@ -568,7 +582,9 @@
[% IF managedby_patron.borrowernumber && logged_in_user.borrowernumber != managedby_patron.borrowernumber %]
<a id="restore_previous_manager" href="#"><i class="fa fa-trash-can"></i> Keep existing manager</a>
[% END %]
<input type="hidden" name="managedby" id="managedby" value="[% logged_in_user.borrowernumber | html %]" />
[% IF CAN_user_suggestions_suggestions_manage %]
<input type="hidden" name="managedby" id="managedby" value="[% logged_in_user.borrowernumber | html %]" />
[% END %]
<br/>
<label for="notify">Notify manager:</label>
@ -649,7 +665,9 @@
[% IF op == 'else' %]
<div id="toolbar" class="btn-toolbar">
<a class="btn btn-default" id="newsuggestion" href="suggestion.pl?op=add_form&branchcode=[% branchcode | uri %]"><i class="fa fa-plus"></i> New purchase suggestion</a>
[% IF CAN_user_suggestions_suggestions_create %]
<a class="btn btn-default" id="newsuggestion" href="suggestion.pl?op=add_form&branchcode=[% branchcode | uri %]"><i class="fa fa-plus"></i> New purchase suggestion</a>
[% END %]
</div>
<h1>Suggestions management</h1>
@ -843,23 +861,29 @@
[% END %]
</td>
<td class="actions">
<div class="btn-group dropup">
<a class="btn btn-default btn-xs" role="button" href="suggestion.pl?suggestionid=[% s.suggestionid | html %]&amp;op=edit_form"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a><a class="btn btn-default btn-xs dropdown-toggle" id="more_actions_[% s.suggestionid | html %]" role="button" data-toggle="dropdown" href="#"><b class="caret"></b></a>
<ul class="dropdown-menu pull-right" role="menu" aria-labelledby="more_actions_[% s.suggestionid | html %]">
<li>
<button data-op="cud-delete" data-suggestionid="[% s.suggestionid | html %]" class="btn btn-default trigger_action">Delete</button>
</li>
[% UNLESS s.archived %]
<li>
<button class="btn btn-default trigger_action" data-op="cud-archive" data-suggestionid="[% s.suggestionid | html %]">Archive</button>
</li>
[% ELSE %]
<li>
<button class="btn btn-default trigger_action" data-op="cud-unarchive" data-suggestionid="[% s.suggestionid | html %]">Unarchive</button>
</li>
[% END %]
</ul>
</div>
[% IF CAN_user_suggestions_suggestions_manage %]
<div class="btn-group dropup">
<a class="btn btn-default btn-xs" role="button" href="suggestion.pl?suggestionid=[% s.suggestionid | html %]&amp;op=edit_form"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a><a class="btn btn-default btn-xs dropdown-toggle" id="more_actions_[% s.suggestionid | html %]" role="button" data-toggle="dropdown" href="#"><b class="caret"></b></a>
<ul class="dropdown-menu pull-right" role="menu" aria-labelledby="more_actions_[% s.suggestionid | html %]">
[% IF CAN_user_suggestions_suggestions_delete %]
<li>
<button data-op="cud-delete" data-suggestionid="[% s.suggestionid | html %]" class="btn btn-default trigger_action">Delete</button>
</li>
[% END %]
[% UNLESS s.archived %]
<li>
<button class="btn btn-default trigger_action" data-op="cud-archive" data-suggestionid="[% s.suggestionid | html %]">Archive</button>
</li>
[% ELSE %]
<li>
<button class="btn btn-default trigger_action" data-op="cud-unarchive" data-suggestionid="[% s.suggestionid | html %]">Unarchive</button>
</li>
[% END %]
</ul>
</div>
[% ELSIF CAN_user_suggestions_suggestions_delete %]
<button data-op="cud-delete" data-suggestionid="[% s.suggestionid | html %]" class="btn btn-default trigger_action">Delete</button>
[% END %]
</td>
</tr>
[% END # /FOREACH s %]
@ -872,6 +896,7 @@
</div>
</div>
<div class="suggestions_batch_ops row">
[% IF CAN_user_suggestions_suggestions_manage %]
<div class="col-sm-4">
<fieldset class="brief">
<div id="select-reason[% loop.index | html %]">
@ -964,7 +989,9 @@
<button type="submit" data-op="cud-update_manager" class="btn btn-primary">Submit</button>
</fieldset>
</div> <!-- /.col-sm-2 -->
[% END %]
[% IF CAN_user_suggestions_suggestions_delete %]
<div class="col-sm-2">
<fieldset class="brief">
<span class="label">Delete selected</span>
@ -975,6 +1002,9 @@
<button type="submit" data-op="cud-delete" class="btn btn-primary">Delete</button>
</fieldset>
</div> <!-- /.col-sm-2 -->
[% END %]
[% IF CAN_user_suggestions_suggestions_manage %]
<div class="col-sm-2">
<fieldset class="brief">
<span class="label">Archive selected</span>
@ -985,6 +1015,7 @@
</fieldset>
</fieldset>
</div> <!-- /.col-sm-2 -->
[% END %]
</div> <!-- /.row -->
[% ELSE %]

View File

@ -32,7 +32,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
{ template_name => "members/purchase-suggestions.tt",
query => $input,
type => "intranet",
flagsrequired => { suggestions => 'suggestions_manage' },
flagsrequired => { suggestions => '*' },
}
);

View File

@ -125,13 +125,15 @@ unless ( $op eq 'cud-save' ) {
}
my ( $template, $borrowernumber, $cookie, $userflags ) = get_template_and_user(
{
template_name => "suggestion/suggestion.tt",
query => $input,
type => "intranet",
flagsrequired => { suggestions => 'suggestions_manage' },
}
);
{
template_name => "suggestion/suggestion.tt",
query => $input,
type => "intranet",
flagsrequired => { suggestions => '*' },
}
);
my $librarian = Koha::Patrons->find($borrowernumber);
$borrowernumber = $input->param('borrowernumber') if ( $input->param('borrowernumber') );
$template->param('borrowernumber' => $borrowernumber);
@ -141,8 +143,12 @@ my $branchfilter = $input->param('branchcode') || C4::Context->userenv->{'branch
## Operations
##
my @messages;
if ( $op =~ /cud-save/ ) {
output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' });
my @messages;
my $biblio = MarcRecordFromNewSuggestion({
title => $suggestion_only->{title},
author => $suggestion_only->{author},
@ -201,7 +207,12 @@ if ( $op =~ /cud-save/ ) {
if exists $suggestion_only->{branchcode}
&& $suggestion_only->{branchcode} eq "";
&ModSuggestion($suggestion_only);
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) {
&ModSuggestion($suggestion_only);
} else {
push @messages, { type => 'error', code => 'no_manage_permission' };
$template->param( messages => \@messages, );
}
if ( $notify ) {
my $patron = Koha::Patrons->find( $suggestion_only->{managedby} );
@ -241,7 +252,12 @@ if ( $op =~ /cud-save/ ) {
}
else {
## Adding some informations related to suggestion
Koha::Suggestion->new($suggestion_only)->store();
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_create' } ) ) {
Koha::Suggestion->new($suggestion_only)->store();
} else {
push @messages, { type => 'error', code => 'no_delete_permission' };
$template->param( messages => \@messages );
}
}
# empty fields, to avoid filter in "SearchSuggestion"
}
@ -303,17 +319,28 @@ elsif ($op eq "cud-update_status" ) {
$suggestion->{reason} = $reason;
}
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
$suggestion->{suggestionid} = $suggestionid;
&ModSuggestion($suggestion);
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) {
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
$suggestion->{suggestionid} = $suggestionid;
&ModSuggestion($suggestion);
}
redirect_with_params($input);
} else {
push @messages, { type => 'error', code => 'no_manage_permission' };
$template->param( messages => \@messages, );
}
redirect_with_params($input);
}elsif ($op eq "cud-delete" ) {
foreach my $delete_field (@editsuggestions) {
&DelSuggestion( $borrowernumber, $delete_field,'intranet' );
} elsif ($op eq "cud-delete" ) {
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_delete' } ) ) {
foreach my $delete_field (@editsuggestions) {
&DelSuggestion( $borrowernumber, $delete_field, 'intranet' );
}
redirect_with_params($input);
} else {
push @messages, { type => 'error', code => 'no_delete_permission' };
$template->param( messages => \@messages, );
}
redirect_with_params($input);
}
elsif ($op eq "cud-archive" ) {
Koha::Suggestions->find($_)->update({ archived => 1 }) for @editsuggestions;
@ -324,18 +351,28 @@ elsif ($op eq "cud-unarchive" ) {
redirect_with_params($input);
}
elsif ( $op eq 'cud-update_itemtype' ) {
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
&ModSuggestion({ suggestionid => $suggestionid, itemtype => $new_itemtype });
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) {
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
&ModSuggestion({ suggestionid => $suggestionid, itemtype => $new_itemtype });
}
redirect_with_params($input);
} else {
push @messages, { type => 'error', code => 'no_manage_permission' };
$template->param( messages => \@messages, );
}
redirect_with_params($input);
}
elsif ( $op eq 'cud-update_manager' ) {
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
&ModSuggestion({ suggestionid => $suggestionid, managedby => $sugg_managedby });
if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) {
foreach my $suggestionid (@editsuggestions) {
next unless $suggestionid;
&ModSuggestion({ suggestionid => $suggestionid, managedby => $sugg_managedby });
}
redirect_with_params($input);
} else {
push @messages, { type => 'error', code => 'no_manage_permission' };
$template->param( messages => \@messages, );
}
redirect_with_params($input);
}
elsif ( $op eq 'show' ) {
$suggestion_ref=&GetSuggestion($$suggestion_ref{'suggestionid'});

View File

@ -244,7 +244,9 @@ subtest 'superlibrarian tests' => sub {
'CAN_user_stockrotation_manage_rota_items' => 1,
'CAN_user_stockrotation_manage_rotas' => 1,
'CAN_user_stockrotation' => 1,
'CAN_user_suggestions_suggestions_create' => 1,
'CAN_user_suggestions_suggestions_manage' => 1,
'CAN_user_suggestions_suggestions_delete' => 1,
'CAN_user_suggestions' => 1,
'CAN_user_superlibrarian' => 1,
'CAN_user_tools_access_files' => 1,