Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 28782: Use query param list instead of splitting elements using '/'

Browse source 
This removes the need to handle single and multiple cases separately,
thus removing bunch if-else cases and simplifying our code. This
coding style is also in line with our other .pl scripts.

To test:
 1) Make sure placing a hold still works from the following pages:

   /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX

   /cgi-bin/koha/catalogue/search.pl?q=a

   /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX

   /cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron
   there and through the clubs.pl create a hold to a bib)

Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Joonas Kylmälä 2021-07-29 17:23:01 +03:00 committed by Fridolin Somers
parent ee957fc812
commit 52b5a22edf
9 changed files with 59 additions and 87 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
koha-tmpl/intranet-tmpl/prog/en/includes/clubs-table.inc
View file

@ -24,10 +24,12 @@
<tbody>
[% FOREACH c IN clubs %]
[% IF destination == 'holds' %]
[% SET data_url = "/cgi-bin/koha/reserve/request.pl?club=" _ c.id %]
[% FOREACH biblionumber IN biblionumbers %]
[% SET data_url = data_url _ "&amp;biblionumber=" _ biblionumber %]
[% END %]
[% IF multi_hold %]
[% SET data_url = "/cgi-bin/koha/reserve/request.pl?club=" _ c.id _ "&amp;multi_hold=1&amp;biblionumbers=" _ biblionumbers %]
[% ELSE %]
[% SET data_url = "/cgi-bin/koha/reserve/request.pl?club=" _ c.id _ "&amp;biblionumber=" _ biblionumber %]
[% SET data_url = data_url _ "&amp;multi_hold=1" %]
[% END %]
<tr class="clickable" data-url="[% data_url | html %]">
<td><a href="[% data_url | url %]">[% c.name | html %]</a></td>

2
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
View file

@ -671,14 +671,12 @@
<form id="hold_form" method="get" action="/cgi-bin/koha/reserve/request.pl">
<!-- Value will be set here by placeHold() -->
<input id="hold_form_biblios" type="hidden" name="biblionumbers" value="" />
<input type="hidden" name="findborrower" id="holdFor" value="" />
<input type="hidden" name="club" id="holdForClub" value="" />
</form>
<form id="list_form" method="get" action="/cgi-bin/koha/reserve/request.pl">
<!-- Value will be set here by addToList() -->
<input id="list_form_biblios" type="hidden" name="biblionumbers" value="" />
<input type="hidden" name="multi_listadd" value="1"/>
</form>

51
koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
View file

@ -163,14 +163,12 @@
[% END %]
</ul>
<div id="holds_patronsearch_pane">
<form id="holds_patronsearch" action="request.pl?biblionumbers=[% biblionumbers | html %]" method="post">
<form id="holds_patronsearch" action="request.pl" method="post">
<div class="hint">Enter patron card number or partial name:</div>
<input type="text" size="40" id="patron" class="focus" name="findborrower" autocomplete="off" />
<input type="submit" value="Search" />
[% IF multi_hold %]
<input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
[% ELSE %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
[% FOREACH biblionumber IN biblionumbers %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]"/>
[% END %]
</form> <!-- /#holds_patronsearch -->
@ -180,14 +178,12 @@
</div>
[% IF clubcount %]
<div id="holds_clubsearch_pane">
<form id="holds_clubsearch" action="request.pl?biblionumbers=[% biblionumbers | html %]" method="post">
<form id="holds_clubsearch" action="request.pl" method="post">
<div class="hint">Enter club ID or partial name:</div>
<input type="text" size="40" id="club" class="focus" name="findclub" autocomplete="off" />
<input type="submit" value="Search" />
[% IF multi_hold %]
<input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
[% ELSE %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
[% FOREACH biblionumber IN biblionumbers %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]"/>
[% END %]
</form> <!-- /#holds_patronsearch -->
@ -206,7 +202,6 @@
<form action="/api/v1/clubs/[% club.id | html %]/holds" method="post" name="form" id="club-request-form">
[% IF ( multi_hold ) %]
<input type="hidden" name="biblionumbers" id="multi_hold_bibs" value="[% biblionumbers | html %]"/>
<input type="hidden" name="bad_bibs" id="bad_bibs" value=""/>
<input type="hidden" name="request" value="any"/>
[% FOREACH biblioloo IN biblioloop %]
@ -214,7 +209,6 @@
<input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/>
[% END %]
[% ELSE %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
<input type="hidden" name="title" value="[% biblio.title | html %]" />
<input type="hidden" name="rank-request" value="[% fixedRank | html %]" />
[% END # /IF multi_hold %]
@ -449,8 +443,10 @@
<input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]" />
<input type="hidden" name="type" value="str8" />
[% FOREACH biblionumber IN biblionumbers %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]"/>
[% END %]
[% IF ( multi_hold ) %]
<input type="hidden" name="biblionumbers" id="multi_hold_bibs" value="[% biblionumbers | html %]"/>
<input type="hidden" name="multi_holds" id="multi_holds" value="1" />
<input type="hidden" name="bad_bibs" id="bad_bibs" value=""/>
<input type="hidden" name="request" value="any"/>
@ -459,7 +455,6 @@
<input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/>
[% END %]
[% ELSE %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
<input type="hidden" name="title" value="[% biblio.title | html %]" />
<input type="hidden" name="rank-request" value="[% fixedRank | html %]" />
[% END # /IF multi_hold %]
@ -923,7 +918,9 @@
[% IF ( reserveloop ) %]
<form id="existing_holds" name="T[% time | html %]" action="modrequest.pl" method="post" style="display:block">
[% IF ( multi_hold ) %]
<input type = "hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
[% FOREACH biblionumber IN biblionumbers %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]"/>
[% END %]
[% END %]
[% IF enqueued %]
@ -1154,7 +1151,7 @@
[% Asset.js("js/holds.js") | $raw%]
<script>
var Sticky;
var biblionumber = "[% biblionumber | $raw %]";
var biblionumbers = [[% biblionumbers.join(', ') | $raw %]];
var borrowernumber = "[% patron.borrowernumber | $raw %]";
var patron_homebranch = "[% To.json( Branches.GetName( patron.branchcode ) ) | $raw %]";
var override_items = {[% FOREACH biblio IN biblioloop %][% FOREACH itemloo IN biblio.itemloop %][% IF ( itemloo.override ) %]
@ -1220,7 +1217,7 @@
var pickup = $("#pickup").val();
var url = "?pickup=" + pickup;
url += "&borrowernumber=" + borrowernumber;
url += "&biblionumber=" + biblionumber;
url += "&biblionumber=" + biblionumbers[0];
window.location.replace(url);
});
[% END %]
@ -1240,8 +1237,6 @@
$("#club-request-form").on("submit", function() {
let $t = $(this);
$('.clubalert, .holdalert').addClass('hide');
let biblionumbers = [biblionumber];
let biblionumbers_text;
const data = {
pickup_library_id: $('select[name="pickup"]').val()
};
@ -1257,12 +1252,13 @@
if($('input[name="default_patron_home"]:checked').length) {
data.default_patron_home = 1;
}
if($('input[name="biblionumbers"]').length) {
biblionumbers_text = $('input[name="biblionumbers"]').val();
biblionumbers = biblionumbers_text.replace(/\/$/, '').split('/')
}
const count = $('input[name="holds_to_place_count"]').length?$('input[name="holds_to_place_count"]').val():1;
var newloc = 'request.pl?';
biblionumbers.forEach(function (biblionumber) {
newloc += '&biblionumber=' + biblionumber;
});
biblionumbers.forEach(function(biblionumber) {
data.biblio_id = biblionumber;
let options = {
@ -1274,11 +1270,7 @@
for(let i = 0; i < count; i++) {
$.ajax(options)
.then(function(result) {
let url = 'request.pl?biblionumber='+biblionumber;
if(biblionumbers_text) {
url = 'request.pl?biblionumbers='+biblionumbers_text;
}
document.location = url;
document.location = newloc;
})
.fail(function(err) {
var message = err.responseJSON.error;
@ -1373,7 +1365,6 @@
function checkMultiHold() {
var biblionumbers = "";
var selected_bibs = $(".multi_hold_item_checkbox:checked");
if ( selected_bibs.length > 0 ) {
// there are biblios selected in the form!
@ -1386,7 +1377,6 @@
}
else {
var bibnum = $(this).attr("title");
biblionumbers += bibnum + "/";
}
});
if ( pickup_not_set > 0 ) {
@ -1406,7 +1396,6 @@
badBibs += bibnum + "/";
});
$("#multi_hold_bibs").val(biblionumbers);
$("#bad_bibs").val(badBibs);
$('#hold-request-form').preventDoubleFormSubmit();

8
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
View file

@ -435,8 +435,7 @@
[% END %]
<form id="hold_form" method="get" action="/cgi-bin/koha/reserve/request.pl">
<!-- Value will be set here by placeHold() -->
<input id="hold_form_biblios" type="hidden" name="biblionumbers" value="" />
<!-- Values will be set here by placeHold() -->
</form>
</main>
@ -762,11 +761,10 @@
alert(MSG_NO_ITEM_SELECTED);
return false;
}
var bibs = "";
$(checkedItems).each(function() {
bibs += $(this).val() + "/";
var bib_param = $("<input>").attr("type", "hidden").attr("name", "biblionumber").val($(this).val());
$('#hold_form').append(bib_param);
});
$("#hold_form_biblios").val(bibs);
$("#hold_form").submit();
return false;
}

21
koha-tmpl/intranet-tmpl/prog/js/cart.js
View file

@ -7,22 +7,17 @@ function placeHold () {
return false;
}
var newloc;
var bib_params = [];
$(checkedItems).each(function() {
var bib = $(this).val();
bib_params.push("biblionumber=" + bib);
});
if ($(checkedItems).size() > 1) {
var bibs = "";
$(checkedItems).each(function() {
var bib = $(this).val();
bibs += bib + "/";
});
newloc = "/cgi-bin/koha/reserve/request.pl?biblionumbers=" + bibs + "&multi_hold=1";
} else {
var bib = checkedItems[0].value;
newloc = "/cgi-bin/koha/reserve/request.pl?biblionumber=" + bib;
if (bib_params.length > 1) {
bib_params.push('multi_hold=1');
}
window.opener.location = newloc;
window.opener.location = "/cgi-bin/koha/reserve/request.pl?" + bib_params.join('&');
window.close();
}

9
koha-tmpl/intranet-tmpl/prog/js/pages/results.js
View file

@ -337,12 +337,15 @@ function placeHold () {
alert( __("Nothing is selected") );
return false;
}
var bibs = "";
var bibs = [];
$(checkedItems).each(function() {
var bib = $(this).val();
bibs += bib + "/";
bibs.push(bib);
});
bibs.forEach(function (bib) {
var bib_param = $("<input>").attr("type", "hidden").attr("name", "biblionumber").val(bib);
$('#hold_form').append(bib_param);
});
$("#hold_form_biblios").val(bibs);
$("#hold_form").submit();
return false;
}

5
reserve/modrequest.pl
View file

@ -24,6 +24,7 @@
use Modern::Perl;
use CGI qw ( -utf8 );
use URI;
use List::MoreUtils qw( uniq );
use Try::Tiny;
@ -104,7 +105,7 @@ if ( $from eq 'borrower'){
} elsif ( $from eq 'circ'){
print $query->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrower[0]");
} else {
my $url = "/cgi-bin/koha/reserve/request.pl?";
$url .= "biblionumbers=" . join('/', @biblionumber);
my $url = URI->new("/cgi-bin/koha/reserve/request.pl");
$url->query_form( biblionumber => @biblionumber);
print $query->redirect($url);
}

18
reserve/placerequest.pl
View file

@ -24,6 +24,7 @@
use Modern::Perl;
use CGI qw ( -utf8 );
use URI;
use C4::Reserves qw( CanItemBeReserved AddReserve CanBookBeReserved );
use C4::Auth qw( checkauth );
@ -35,7 +36,7 @@ my $input = CGI->new();
checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet');
my @reqbib = $input->multi_param('reqbib');
my $biblionumber = $input->param('biblionumber');
my @biblionumbers = $input->multi_param('biblionumber');
my $borrowernumber = $input->param('borrowernumber');
my $notes = $input->param('notes');
my $branch = $input->param('pickup');
@ -50,14 +51,11 @@ my $non_priority = $input->param('non_priority');
my $patron = Koha::Patrons->find( $borrowernumber );
my $biblionumbers = $input->param('biblionumbers');
$biblionumbers ||= $biblionumber . '/';
my $bad_bibs = $input->param('bad_bibs');
my $bad_bibs_param = $input->param('bad_bibs');
my @bad_bibs = split '/', $bad_bibs_param;
my $holds_to_place_count = $input->param('holds_to_place_count') || 1;
my %bibinfos = ();
my @biblionumbers = split '/', $biblionumbers;
foreach my $bibnum (@biblionumbers) {
my %bibinfo = ();
$bibinfo{title} = $input->param("title_$bibnum");
@ -150,10 +148,12 @@ if ( $type eq 'str8' && $patron ) {
}
}
if ($bad_bibs) {
$biblionumbers .= $bad_bibs;
if (@bad_bibs) {
push @biblionumbers, @bad_bibs;
}
print $input->redirect("request.pl?biblionumbers=$biblionumbers");
my $redirect_url = URI->new("request.pl");
$redirect_url->query_form( biblionumber => @biblionumbers);
print $input->redirect($redirect_url);
}
elsif ( $borrowernumber eq '' ) {
print $input->header();

24
reserve/request.pl
View file

@ -184,23 +184,13 @@ if($findclub) {
}
}
my @biblionumbers = ();
my $biblionumber = $input->param('biblionumber');
my $biblionumbers = $input->param('biblionumbers');
if ( $biblionumbers ) {
@biblionumbers = split '/', $biblionumbers;
} else {
push @biblionumbers, $input->multi_param('biblionumber');
}
my @biblionumbers = $input->multi_param('biblionumber');
my $multi_hold = @biblionumbers > 1;
$template->param(
multi_hold => $multi_hold,
);
# If we are coming from the search result and only 1 is selected
$biblionumber ||= $biblionumbers[0] unless $multi_hold;
# If we have the borrowernumber because we've performed an action, then we
# don't want to try to place another reserve.
if ($borrowernumber_hold && !$action) {
@ -706,17 +696,13 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
$template->param( no_reserves_allowed => $no_reserves_allowed );
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
# FIXME: getting just the first bib's result doesn't seem right
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumbers[0]));
} elsif ( ! $multi_hold ) {
my $biblio = Koha::Biblios->find( $biblionumber );
my $biblio = Koha::Biblios->find( $biblionumbers[0] );
$template->param( biblio => $biblio );
}
if ( $multi_hold ) {
$template->param( biblionumbers => join('/', @biblionumbers) );
} else {
$template->param( biblionumber => $biblionumber || $biblionumbers[0] );
}
$template->param( biblionumbers => \@biblionumbers );
# pass the userenv branch if no pickup location selected
$template->param( pickup => $pickup || C4::Context->userenv->{branch} );