From 010181a6a007339edc4ed63d50cc8ba33fe0f5ab Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 9 Feb 2021 17:19:59 +0100
Subject: [PATCH] Bug 27652: Display patron's categories for sysprefs expecting
 them

This patch adds a new type of system preference entry option: patron
category. A preference with this type will show either a <select> with
patron categories to choose from or a multiple-select menu for choosing
one or more.

This prevents possible errors from incorrect category codes being
entered manually.

To test, apply the patch and run the database update.

- Go to Administration -> System preferences.
- Test each of the updated preferences to confirm that the available
  options are correct and that your selections are correctly saved.

  - Single category selections, PatronSelfRegistrationDefaultCategory
    and GoogleOpenIDConnectDefaultCategory. With each of these you
    should see a dropdown where you can select of all the existing
    patron categories.

  - Multiple selections, OPACHoldsIfAvailableAtPickup and
    BatchCheckouts: These preferences should have an option for
    selecting one or more system preferences at once.

- Test the pages affected by the updated system preferences:

- With BatchCheckouts enabled, confirm that the batch checkout page is
  correctly limited by patron category.

- Set the OPACHoldsIfAvailableAtPickup preference to "Don't allow" and
  select at least one patron category in the
  OPACHoldsIfAvailableAtPickupExceptions preference.
- Log in to the OPAC as a patron whose category was not selected in
  the OPACHoldsIfAvailableAtPickupExceptions preference.
- Try to place a hold on an item which is available at a library.
- When confirming the hold, the libraries where the item is available
  should be disabled in the "Pick up location" dropdown.
- Repeat the process as a patron whose category was selected in
  OPACHoldsIfAvailableAtPickupExceptions. There should be no disabled
  pickup locations.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 admin/preferences.pl                          | 27 +++++++++++++++++--
 circ/circulation.pl                           |  2 +-
 .../data/mysql/atomicupdate/bug_27652.perl    | 11 ++++++++
 .../prog/en/includes/circ-menu.inc            |  2 +-
 .../en/modules/admin/preferences/admin.pref   |  1 +
 .../admin/preferences/circulation.pref        |  6 +++--
 .../en/modules/admin/preferences/opac.pref    |  2 +-
 opac/opac-reserve.pl                          |  2 +-
 8 files changed, 45 insertions(+), 8 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_27652.perl

diff --git a/admin/preferences.pl b/admin/preferences.pl
index 8805e3dbfc..dc4bc0b5d3 100755
--- a/admin/preferences.pl
+++ b/admin/preferences.pl
@@ -91,6 +91,7 @@ sub _get_chunk {
         $chunk->{'languages'} = getTranslatedLanguages( $interface, $theme, undef, $current_languages );
         $chunk->{'type'} = 'languages';
     } elsif ( $options{ 'choices' } ) {
+        my $add_blank;
         if ( $options{'choices'} && ref( $options{ 'choices' } ) eq '' ) {
             if ( $options{'choices'} eq 'class-sources' ) {
                 my $sources = GetClassSources();
@@ -99,6 +100,9 @@ sub _get_chunk {
                 $options{'choices'} = { map { $_ => $_ } getallthemes( 'opac' ) }
             } elsif ( $options{'choices'} eq 'staff-templates' ) {
                 $options{'choices'} = { map { $_ => $_ } getallthemes( 'intranet' ) }
+            } elsif ( $options{choices} eq 'patron-categories' ) {
+                $options{choices} = { map { $_->categorycode => $_->description } Koha::Patron::Categories->search };
+                $add_blank = 1;
             } else {
                 die 'Unrecognized source of preference values: ' . $options{'choices'};
             }
@@ -106,12 +110,31 @@ sub _get_chunk {
 
         $value ||= 0;
 
-        $chunk->{'type'} = 'select';
+        $chunk->{'type'} = ( $options{class} && $options{class} eq 'multiple' ) ? 'multiple' : 'select';
+
+        my @values;
+        @values = split /,/, $value if defined($value);
         $chunk->{'CHOICES'} = [
             sort { $a->{'text'} cmp $b->{'text'} }
-            map { { text => $options{'choices'}->{$_}, value => $_, selected => ( $_ eq $value || ( $_ eq '' && ( $value eq '0' || !$value ) ) ) } }
+            map {
+                my $c = $_;
+                {
+                    text     => $options{'choices'}->{$c},
+                    value    => $c,
+                    selected => (
+                        grep { $_ eq $c || ( $c eq '' && ($value eq '0' || !$value ) ) } @values
+                    ) ? 1 : 0,
+                }
+              }
             keys %{ $options{'choices'} }
         ];
+
+        # Add a first blank value if needed
+        unshift @{ $chunk->{CHOICES} }, {
+            text  => '',
+            value => '',
+        } if $add_blank && $chunk->{type} eq 'select';
+
     } elsif ( $options{'multiple'} ) {
         my @values;
         @values = split /,/, $value if defined($value);
diff --git a/circ/circulation.pl b/circ/circulation.pl
index eef2d2f1a1..44f87b6281 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -116,7 +116,7 @@ my $batch = $query->param('batch');
 my $batch_allowed = 0;
 if ( $batch && C4::Context->preference('BatchCheckouts') ) {
     $template_name = q|circ/circulation_batch_checkouts.tt|;
-    my @batch_category_codes = split '\|', C4::Context->preference('BatchCheckoutsValidCategories');
+    my @batch_category_codes = split ',', C4::Context->preference('BatchCheckoutsValidCategories');
     my $categorycode = $patron->categorycode;
     if ( $categorycode && grep { $_ eq $categorycode } @batch_category_codes ) {
         $batch_allowed = 1;
diff --git a/installer/data/mysql/atomicupdate/bug_27652.perl b/installer/data/mysql/atomicupdate/bug_27652.perl
new file mode 100644
index 0000000000..13a1f75b91
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_27652.perl
@@ -0,0 +1,11 @@
+$DBversion = 'XXX'; # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+
+    $dbh->do(q{
+        UPDATE systempreferences
+        value=REPLACE(value, '|', ',')
+        WHERE variable="OPACHoldsIfAvailableAtPickupExceptions"
+           OR variable="BatchCheckoutsValidCategories"
+    });
+    NewVersion( $DBversion, 27652, "Separate values for OPACHoldsIfAvailableAtPickupExceptions and BatchCheckoutsValidCategories with comma");
+}
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
index e89544a71d..27b4783e8e 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
@@ -125,7 +125,7 @@
 <ul>
     [% IF ( CAN_user_circulate_circulate_remaining_permissions ) %]
         [% IF ( circview ) %]<li class="active">[% ELSE %]<li>[% END %]<a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% patron.borrowernumber | uri %]">Check out</a></li>
-        [% IF Koha.Preference('BatchCheckouts') && Koha.Preference('BatchCheckoutsValidCategories').split('\|').grep('^' _ patron.categorycode _ '$').size > 0 %]
+        [% IF Koha.Preference('BatchCheckouts') && Koha.Preference('BatchCheckoutsValidCategories').split(',').grep('^' _ patron.categorycode _ '$').size > 0 %]
           [% IF ( batch_checkout_view ) %]<li class="active">[% ELSE %]<li>[% END %]<a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% patron.borrowernumber | uri %]&amp;batch=1">Batch check out</a></li>
         [% END %]
     [% END %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
index 3327a3ce1c..77fe89a1ee 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
@@ -184,6 +184,7 @@ Administration:
         -
             - "Use this category code when automatically registering a Google Open ID patron: "
             - pref: GoogleOpenIDConnectDefaultCategory
+              choices: patron-categories
             - "."
         -
             - "Use this branch code when automatically registering a Google Open ID patron: "
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
index accf490d9b..3bb28f5541 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -880,7 +880,8 @@ Circulation:
         -
             - 'Patron categories not affected by <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=OPACHoldsIfAvailableAtPickup">OPACHoldsIfAvailableAtPickup</a>:'
             - pref: OPACHoldsIfAvailableAtPickupExceptions
-            - "(list of patron categories separated with a pipe '|')"
+              choices: patron-categories
+              class: multiple
         -
             - pref: CanMarkHoldsToPullAsLost
               choices:
@@ -1142,7 +1143,8 @@ Circulation:
         -
             - "Patron categories allowed to checkout in a batch:"
             - pref: BatchCheckoutsValidCategories
-            - "(list of patron categories separated with a pipe '|')."
+              choices: patron-categories
+              class: multiple
     Housebound module:
         -
             - pref: HouseboundModule
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
index df88ce6a31..67ef50a828 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
@@ -807,7 +807,7 @@ OPAC:
         -
             - "Use the patron category code"
             - pref: PatronSelfRegistrationDefaultCategory
-              class: short
+              choices: patron-categories
             - "as the default patron category for patrons registered via the OPAC."
             - "<br><strong>WARNING: Do not use a regular patron category for self registration.</strong> If the <code>misc/cronjobs/cleanup_database.pl</code> cronjob is setup to delete unverified and unfinished OPAC self registrations, it will permanently and unrecoverably delete all patrons that have registered more than <a href='/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=PatronSelfRegistrationExpireTemporaryAccountsDelay'>PatronSelfRegistrationExpireTemporaryAccountsDelay</a> days ago."
         -
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index b90a2061b2..9b7680759d 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -75,7 +75,7 @@ my $category = $patron->category;
 
 my $can_place_hold_if_available_at_pickup = C4::Context->preference('OPACHoldsIfAvailableAtPickup');
 unless ( $can_place_hold_if_available_at_pickup ) {
-    my @patron_categories = split '\|', C4::Context->preference('OPACHoldsIfAvailableAtPickupExceptions');
+    my @patron_categories = split ',', C4::Context->preference('OPACHoldsIfAvailableAtPickupExceptions');
     if ( @patron_categories ) {
         my $categorycode = $patron->categorycode;
         $can_place_hold_if_available_at_pickup = grep { $_ eq $categorycode } @patron_categories;
-- 
2.39.2