Bug 4137: Fix the OPACViewOthersSuggestions behavior

This pref does not work at all, the interface let the user choose to
list all suggestions, but whatever he chooses the suggestion list is the
same.

This patch cleans a bit the suggestedby management.

There are a lot of cases to test, because linked to 2 prefs:
 AnonSuggestions and OPACViewOthersSuggestions.
1/ AnonSuggestions = 0 and OPACViewOthersSuggestions = 0
  - A non logged in user is not able to make a suggestion.
  - A logged in user is not able to see suggestions made by someone else.
2/ AnonSuggestions = 0 and OPACViewOthersSuggestions = 1
  - A non logged in user is not able to make a suggestion.
  - A logged in user is able to see suggestions made by someone else.
3/ AnonSuggestions = 1 and OPACViewOthersSuggestions = 0
  - A non logged in user is able to make a suggestion.
  The suggestedby field will be filled with the AnonymousPatron pref value.
  He is not able to see suggestions, even the ones made by AnonymousPatron.
  - A logged in user is not able to see suggestions made by someone else.
4/ AnonSuggestions = 1 and OPACViewOthersSuggestions = 1
  - A non logged in user is able to make a suggestion.
  He is able to see all suggestions.
  - A logged in user is able to see suggestions made by someone else.

In all cases a logged in user should be able to search for suggestions
(except if he is not able to see them).

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All use cases tested, work as expected
No errors

Only comment is perhaps (in the future) a gracefull failure
when AnonymousPatron is not set, or has '0' value

Message is DBIx::Class::ResultSet::create(): Column 'suggestedby' cannot be null at ...

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
(cherry picked from commit be35039b55)
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
This commit is contained in:
Jonathan Druart 2015-03-24 17:01:30 +01:00 committed by Chris Cormack
parent c8e48b280a
commit f7dd4d6aef
4 changed files with 72 additions and 44 deletions

View File

@ -483,7 +483,6 @@ sub get_template_and_user {
OPACShelfBrowser => "" . C4::Context->preference("OPACShelfBrowser"),
OPACURLOpenInNewWindow => "" . C4::Context->preference("OPACURLOpenInNewWindow"),
OPACUserCSS => "" . C4::Context->preference("OPACUserCSS"),
OPACViewOthersSuggestions => "" . C4::Context->preference("OPACViewOthersSuggestions"),
OpacAuthorities => C4::Context->preference("OpacAuthorities"),
opac_css_override => $ENV{'OPAC_CSS_OVERRIDE'},
opac_search_limit => $opac_search_limit,

View File

@ -68,14 +68,12 @@
[% END # / opacreadinghistory %]
[% IF Koha.Preference( 'suggestion' ) == 1 %]
[% UNLESS ( Koha.Preference( 'AnonSuggestions' ) == 1 ) %]
[% IF ( suggestionsview ) %]
<li class="active">
[% ELSE %]
<li>
[% END %]
<a href="/cgi-bin/koha/opac-suggestions.pl">your purchase suggestions</a></li>
[% IF ( suggestionsview ) %]
<li class="active">
[% ELSE %]
<li>
[% END %]
<a href="/cgi-bin/koha/opac-suggestions.pl">your purchase suggestions</a></li>
[% END %]
[% IF ( EnhancedMessagingPreferences ) %]

View File

@ -92,7 +92,7 @@
</ol>
</fieldset>
<fieldset class="action">
<input type="hidden" name="suggestedby" value="[% suggestedbyme %]" />
<input type="hidden" name="suggested_by_anyone" value="[% suggested_by_anyone %]" />
<input type="hidden" name="op" value="add_confirm" />
<input type="submit" onclick="Check(this.form); return false;" class="btn" value="Submit your suggestion" /> <a class="action" href="/cgi-bin/koha/opac-suggestions.pl">Cancel</a>
</fieldset>
@ -127,29 +127,36 @@
[% IF ( deleted ) %]<div class="alert alert-info">The selected suggestions have been deleted.</div>[% END %]
[% IF ( suggestions_loop ) %]
[% IF Koha.Preference( 'OPACViewOthersSuggestions' ) == 1 %]
<form action="/cgi-bin/koha/opac-suggestions.pl" class="form-inline" method="get">
<fieldset>
<label for="title">Search for:</label>
<input type="text" name="title" id="title" value="[% title |html %]" />
<label for="suggestedby">Suggested by:</label>
<div class="input-append">
<select name="suggestedby" id="suggestedby">
<option value="0">Anyone</option>
<option value="1">Me</option>
</select>
<button type="submit" class="btn">Go</button>
</div>
</fieldset>
</form>
[% END %]
<form action="/cgi-bin/koha/opac-suggestions.pl" class="form-inline" method="get">
<fieldset>
<label for="title">Search for:</label>
<input type="text" name="title" id="title" value="[% title |html %]" />
[% IF Koha.Preference( 'OPACViewOthersSuggestions' ) == 1 %]
[% IF loggedinusername %]
<label for="suggested_by_anyone">Suggested by:</label>
<div class="input-append">
<select name="suggested_by_anyone" id="suggested_by_anyone">
[% IF suggested_by_anyone %]
<option value="0">Me</option>
<option value="1" selected="selected">Anyone</option>
[% ELSE %]
<option value="0" selected="selected">Me</option>
<option value="1">Anyone</option>
[% END %]
</select>
<button type="submit" class="btn">Go</button>
</div>
[% END %]
[% END %]
</fieldset>
</form>
<form action="/cgi-bin/koha/opac-suggestions.pl" method="post" id="myform">
<input type="hidden" name="op" value="delete_confirm" />
<div id="toolbar" class="toolbar clearfix">
[% IF ( loggedinusername || ( Koha.Preference( 'AnonSuggestions' ) == 1 ) ) %]
[% IF ( loggedinusername || ( Koha.Preference( 'AnonSuggestions' ) == 1 ) ) %]
<div id="toolbar" class="toolbar clearfix">
<a class="new" href="/cgi-bin/koha/opac-suggestions.pl?op=add">New purchase suggestion</a>
[% END %]
</div>
</div>
[% END %]
[% IF ( loggedinusername ) %]
<div id="selections-toolbar" class="toolbar">
@ -237,7 +244,11 @@
[% END %]
</form>
[% ELSE %]
<p>There are no pending purchase suggestions.</p>
[% UNLESS Koha.Preference( 'OPACViewOthersSuggestions' ) or $loggedinusername %]
<p>You are not authorized to see pending purchase suggestions.</p>
[% ELSE %]
<p>There are no pending purchase suggestions.</p>
[% END %]
[% IF ( loggedinusername || ( Koha.Preference( 'AnonSuggestions' ) == 1 ) ) %]
<p><a class="new" href="/cgi-bin/koha/opac-suggestions.pl?op=add">New purchase suggestion</a></p>
[% END %]
@ -280,7 +291,7 @@
[% IF ( loggedinusername ) %]null,[% END %]
{ "sType": "anti-the" },
null,
[% IF ( OPACViewOthersSuggestions ) %]null,[% END %]
[% IF Koha.Preference( 'OPACViewOthersSuggestions' ) == 1 %]null,[% END %]
[% IF Koha.Preference( 'OpacSuggestionManagedBy' ) %]null,[% END %]
null
]

View File

@ -33,11 +33,11 @@ use C4::Scrubber;
use Koha::DateUtils qw( dt_from_string );
my $input = new CGI;
my $allsuggestions = $input->param('showall');
my $op = $input->param('op');
my $suggestion = $input->Vars;
delete $suggestion->{negcap};
my $negcaptcha = $input->param('negcap');
my $suggested_by_anyone = $input->param('suggested_by_anyone') || 0;
# If a spambot accidentally populates the 'negcap' field in the sugesstions form, then silently skip and return.
if ($negcaptcha ) {
@ -51,14 +51,14 @@ if ( ! C4::Context->preference('suggestion') ) {
exit;
}
delete $$suggestion{$_} foreach qw<op suggestedbyme>;
delete $suggestion->{$_} foreach qw<op suggested_by_anyone>;
$op = 'else' unless $op;
my ( $template, $borrowernumber, $cookie, @messages );
my $deleted = $input->param('deleted');
my $submitted = $input->param('submitted');
if ( C4::Context->preference("AnonSuggestions") ) {
if ( C4::Context->preference("AnonSuggestions") or ( C4::Context->preference("OPACViewOthersSuggestions") and $op eq 'else' ) ) {
( $template, $borrowernumber, $cookie ) = get_template_and_user(
{
template_name => "opac-suggestions.tt",
@ -67,9 +67,6 @@ if ( C4::Context->preference("AnonSuggestions") ) {
authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ),
}
);
if ( !$$suggestion{suggestedby} ) {
$$suggestion{suggestedby} = C4::Context->preference("AnonymousPatron");
}
}
else {
( $template, $borrowernumber, $cookie ) = get_template_and_user(
@ -81,13 +78,36 @@ else {
}
);
}
if ($allsuggestions){
delete $$suggestion{suggestedby};
if ( $op eq 'else' ) {
if ( C4::Context->preference("OPACViewOthersSuggestions") ) {
if ( $borrowernumber ) {
# A logged in user is able to see suggestions from others
$suggestion->{suggestedby} = $suggested_by_anyone
? undef
: $borrowernumber;
}
else {
# Non logged in user is able to see all suggestions
$suggestion->{suggestedby} = undef;
}
}
else {
if ( $borrowernumber ) {
$suggestion->{suggestedby} = $borrowernumber;
}
else {
$suggestion->{suggestedby} = -1;
}
}
} else {
if ( $borrowernumber ) {
$suggestion->{suggestedby} = $borrowernumber;
}
else {
$suggestion->{suggestedby} = C4::Context->preference("AnonymousPatron");
}
}
else {
$$suggestion{suggestedby} ||= $borrowernumber unless ($allsuggestions);
}
# warn "bornum:",$borrowernumber;
my $suggestions_loop =
&SearchSuggestion( $suggestion);
@ -172,11 +192,11 @@ $template->param(
itemtypeloop=> $supportlist,
suggestions_loop => $suggestions_loop,
patron_reason_loop => $patron_reason_loop,
showall => $allsuggestions,
"op_$op" => 1,
$op => 1,
messages => \@messages,
suggestionsview => 1,
suggested_by_anyone => $suggested_by_anyone,
);
output_html_with_http_headers $input, $cookie, $template->output;