Attachment 155002 Details for Bug 31503 – Bug 31503: Make opac-patron-consent more generic

[patch] Bug 31503: Make opac-patron-consent more generic

Bug-31503-Make-opac-patron-consent-more-generic.patch (text/plain), 18.86 KB, created by Marcel de Rooy on 2023-08-31 09:45:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2023-08-31 09:45:05 UTC

Size: 18.86 KB

patch

obsolete

>From 4468cbda39567a79f32055d9fb7bee6a7c8290fa Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Thu, 31 Aug 2023 08:40:29 +0000
>Subject: [PATCH] Bug 31503: Make opac-patron-consent more generic
>Content-Type: text/plain; charset=utf-8
>
>Adds $patron->consent and $consents->available_types.
>Incorporates them into script/template.
>Provides two unit tests.
>
>Note: A follow-up patch helps you test this with an
>example plugin.
>
>Test plan:
>Run t/db_dependent/Koha/Patron.t
>Run t/db_dependent/Koha/Patron/Consents.t
>Toggle the value of pref PrivacyPolicyConsent and look at
>OPAC account, tab Consents.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/Patron.pm                                | 24 +++++
> Koha/Patron/Consents.pm                       | 27 ++++++
> .../en/modules/opac-patron-consent.tt         | 94 ++++++++++---------
> opac/opac-patron-consent.pl                   | 57 +++++------
> t/db_dependent/Koha/Patron.t                  | 22 ++++-
> t/db_dependent/Koha/Patron/Consents.t         | 45 ++++++++-
> 6 files changed, 189 insertions(+), 80 deletions(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 6937ea904a..8c16da639b 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -44,6 +44,7 @@ use Koha::CurbsidePickups;
> use Koha::Old::Checkouts;
> use Koha::Patron::Attributes;
> use Koha::Patron::Categories;
>+use Koha::Patron::Consents;
> use Koha::Patron::Debarments;
> use Koha::Patron::HouseboundProfile;
> use Koha::Patron::HouseboundRole;
>@@ -2490,6 +2491,29 @@ sub get_savings {
>     )->next->get_column('total_savings') // 0;
> }
> 
>+=head3 consent
>+
>+    my $consent = $patron->consent(TYPE);
>+
>+    Returns the first consent of type TYPE (there should be only one) or a new instance
>+    of Koha::Patron::Consent.
>+
>+=cut
>+
>+sub consent {
>+    my ( $self, $type ) = @_;
>+    Koha::Exceptions::MissingParameter->throw('Missing consent type') if !$type;
>+    my $consents = Koha::Patron::Consents->search(
>+        {
>+            borrowernumber => $self->borrowernumber,
>+            type           => $type,
>+        }
>+    );
>+    return $consents && $consents->count
>+        ? $consents->next
>+        : Koha::Patron::Consent->new( { borrowernumber => $self->borrowernumber, type => $type } );
>+}
>+
> =head2 Internal methods
> 
> =head3 _type
>diff --git a/Koha/Patron/Consents.pm b/Koha/Patron/Consents.pm
>index fc75a55d41..7dcb14d6a9 100644
>--- a/Koha/Patron/Consents.pm
>+++ b/Koha/Patron/Consents.pm
>@@ -20,7 +20,10 @@ package Koha::Patron::Consents;
> use Modern::Perl;
> 
> use base qw(Koha::Objects);
>+
>+use C4::Context;
> use Koha::Patron::Consent;
>+use Koha::Plugins;
> 
> =head1 NAME
> 
>@@ -34,8 +37,32 @@ Koha::Objects class for handling patron consents
> 
> =head2 Class Methods
> 
>+=head3 available_types
>+
>+    Returns an HASHref of available consent types like:
>+        { type1 => {}, type2 => {}, .. }
>+
>+    Checks preferences OPACCustomConsentTypes and PrivacyPolicyConsent.
>+    Calls patron_consent_type plugins (if pref enabled).
>+
>+    Note: The plugins return an ARRAYref with type, title and description like:
>+        [ my_type => { title => { lang => 1, .. }, description => { lang => 2, .. } } ]
>+
> =cut
> 
>+sub available_types {
>+    my ($self) = shift;
>+    my $response = {};
>+    $response->{GDPR_PROCESSING} = 1 if C4::Context->preference('PrivacyPolicyConsent');
>+    if ( C4::Context->preference('OPACCustomConsentTypes') ) {
>+        foreach my $return ( Koha::Plugins->call('patron_consent_type') ) {
>+            next if ref($return) ne 'ARRAY' or @$return != 2;    # ignoring bad input
>+            $response->{ $return->[0] } = $return->[1];
>+        }
>+    }
>+    return $response;
>+}
>+
> =head3 _type
> 
> =cut
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-patron-consent.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-patron-consent.tt
>index d6fe1dcafe..6069cd2ac0 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-patron-consent.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-patron-consent.tt
>@@ -36,41 +36,45 @@
>             </div>
>             <div class="col-lg-10 order-first order-md-first order-lg-2">
>                 <div id="patronconsents" class="maincontent">
>-
>-                    [% IF Koha.Preference('PrivacyPolicyConsent') %]
>-                    <div class="alert alert-warning">
>-                            <p>In order to keep you logged in, we need your consent to process personal data as specified in the privacy policy linked below.</p>
>-                            <p>Please save your consent below or log out. Thank you!</p>
>-                    </div>
>-                    [% END %]
>-
>-                    <h1>Your consents</h1>
>-
>                     <form action="/cgi-bin/koha/opac-patron-consent.pl" method="post">
>-                        [% IF Koha.Preference('PrivacyPolicyConsent') %]
>-                        <legend><h2 id="GDPR_consents">Privacy policy consents</h2></legend>
>-                            <input type="hidden" name="op" value="gdpr_proc_save"/>
>-                            <input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]"/>
>+                        <input type="hidden" name="op" value="save"/>
>+                        <input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]"/>
>+                        <h1>Your consents</h1>
>+                        <br>
>+                        [% FOREACH consent IN consents %]
>+                            [% SET consent_type = consent.type %]
>+                            [% IF consent_type == 'GDPR_PROCESSING' %]
>+                                <legend><h2>Privacy policy consent</h2></legend>
>+                                <p>Please read the <a target="_blank" href="[% Koha.Preference('PrivacyPolicyURL') | url %]">privacy policy</a>.</p>
>+                                <p>In order to keep you logged in, we need your consent to process personal data as specified in the EU General Data Protection Regulation of May 25, 2018. If you would not agree, we will need to <strong>remove</strong> your account within a reasonable time.</p>
>+                                <p>Do you agree with our processing of your personal data as outlined in the policy?</p>
>+                            [% ELSIF consent_types.$consent_type %]
>+                                [% SET consent_title = ( consent_types.$consent_type.title.$lang || consent_types.$consent_type.title.en ) %]
>+                                [% SET consent_desc  = ( consent_types.$consent_type.description.$lang || consent_types.$consent_type.description.en )  %]
>+                                <legend><h2>[% consent_title | html %]</h2></legend>
>+                                <p>[% consent_desc | html %]</p>
>+                                <p>Do you agree?</p>
>+                            [% ELSE %]
>+                                <legend><h2>Consent for [% consent_type | html %]</h2></legend>
>+                                <p>Do you agree?</p>
>+                            [% END %]
>                             <fieldset>
>-                                <ul><li>
>-                                    <p>I have read the <a target="_blank" href="[% Koha.Preference('PrivacyPolicyURL') | url %]">privacy policy</a> and agree with your processing of my personal data as outlined therein.</p>
>-                                    <p><input type="radio" name="gdpr_processing" value="agreed"> Yes, I agree.<br>
>-                                    <input type="radio" name="gdpr_processing" value="disagreed"> No, I do not agree. Please remove my account within a reasonable time.</p>
>-                                    [% IF gdpr_proc_consent %]
>-                                        <p>Your consent was registered on [% gdpr_proc_consent | $KohaDates with_hours => 1 %].</p>
>-                                    [% ELSIF gdpr_proc_refusal %]
>-                                        <p>You indicated recently that you do not consent, and we will process your request soon.</p>
>-                                    [% END %]
>-                                </li></ul>
>-                            </fieldset>
>-                            <fieldset class="action">
>-                                <input id="saveconsent" type="submit" value="Save" class="btn btn-primary" />
>+                                <input type="radio" name="check_[% consent_type | html %]" value="1"> Yes<br>
>+                                <input type="radio" name="check_[% consent_type | html %]" value="0"> No
>                             </fieldset>
>+                            [% IF consent.given_on %]
>+                                <p class="consent_info">Your consent was registered on [% consent.given_on | html %].</p>
>+                            [% ELSIF consent.refused_on %]
>+                                <p class="dissent_info">We registered that you did not consent on [% consent.refused_on | html %].</p>
>+                            [% END %]
>+                            <br>
>                         [% END %]
>-
>+                        <fieldset class="action">
>+                            <input id="saveconsent" type="submit" value="Save" class="btn btn-primary" />
>+                        </fieldset>
>                     </form>
> 
>-                </div> 
>+                </div> 
>             </div> 
>         </div> 
>     </div> 
>@@ -79,27 +83,29 @@
> [% INCLUDE 'opac-bottom.inc' %]
> [% BLOCK jsinclude %]
>     <script>
>-        var consent = null;
>         $(document).ready(function() {
>-            [% IF gdpr_proc_consent %]
>-                consent=1;
>-                $("input[type='radio'][value='agreed']").prop('checked',true);
>-                $(".alert").hide();
>-            [% ELSIF gdpr_proc_refusal %]
>-                consent=0;
>-                $("input[type='radio'][value='disagreed']").prop('checked',true);
>-            [% ELSE %]
>+            [% FOREACH consent IN consents %]
>+                [% IF consent.given_on %]
>+                    $("input[type='radio'][name='check_[% consent.type | html %]'][value='1']").prop('checked',true);
>+                [% ELSIF consent.refused_on %]
>+                    $("input[type='radio'][name='check_[% consent.type | html %]'][value='0']").prop('checked',true);
>+                 [% END %]
>             [% END %]
>+            // Initially no choice is made or no change, so disable button
>             $("#saveconsent").prop('disabled', true);
> 
>             $("input[type='radio']").click(function() {
>-                var radio = $(this).val();
>-                if(radio=='agreed' && (consent==null || consent==0)) $("#saveconsent").prop('disabled', false);
>-                if(radio=='disagreed' && (consent==null || consent==1)) $("#saveconsent").prop('disabled', false);
>-                if(radio=='agreed') $(".alert").hide();
>-                if(radio=='disagreed') $(".alert").show();
>+                $("#saveconsent").prop('disabled', false);
>+                var v = $(this).val();
>+                // If former registration info present, toggle
>+                if( v == 1 ) {
>+                    $(this).parent().siblings('.consent_info').show();
>+                    $(this).parent().siblings('.dissent_info').hide();
>+                } else {
>+                    $(this).parent().siblings('.consent_info').hide();
>+                    $(this).parent().siblings('.dissent_info').show();
>+                }
>             });
>-
>         });
>     </script>
> [% END %]
>diff --git a/opac/opac-patron-consent.pl b/opac/opac-patron-consent.pl
>index 49c121250e..565417d6fa 100755
>--- a/opac/opac-patron-consent.pl
>+++ b/opac/opac-patron-consent.pl
>@@ -23,14 +23,12 @@ use CGI qw/-utf8/;
> use C4::Auth qw( get_template_and_user );
> use C4::Output qw( output_html_with_http_headers );
> use Koha::DateUtils qw( dt_from_string );
>-use Koha::Patron::Consents;
>+use Koha::Exceptions::Patron;
> use Koha::Patrons;
> 
>-use constant GDPR_PROCESSING => 'GDPR_PROCESSING';
>-
> my $query = CGI->new;
> my $op = $query->param('op') // q{};
>-my $gdpr_check = $query->param('gdpr_processing') // q{};
>+my $vars = $query->Vars;
> 
> my ( $template, $borrowernumber, $cookie ) = get_template_and_user({
>     template_name   => "opac-patron-consent.tt",
>@@ -38,44 +36,37 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user({
>     type            => "opac",
> });
> 
>-my $patron = Koha::Patrons->find($borrowernumber);
>-my $gdpr_proc_consent;
>-if( C4::Context->preference('PrivacyPolicyConsent') ) {
>-    $gdpr_proc_consent = Koha::Patron::Consents->search({
>-        borrowernumber => $borrowernumber,
>-        type => GDPR_PROCESSING,
>-    })->next;
>-    $gdpr_proc_consent //= Koha::Patron::Consent->new({
>-        borrowernumber => $borrowernumber,
>-        type => GDPR_PROCESSING,
>-    });
>+my $patron = Koha::Patrons->find($borrowernumber)
>+    or Koha::Exceptions::Patron->throw("Patron id $borrowernumber not found");
>+
>+# Get consent types and values
>+my @consents;
>+my $consent_types = Koha::Patron::Consents->available_types;
>+foreach my $consent_type ( sort keys %$consent_types) {
>+    push @consents, $patron->consent($consent_type);
> }
> 
> # Handle saves here
>-if( $op eq 'gdpr_proc_save' && $gdpr_proc_consent ) {
>-    if( $gdpr_check eq 'agreed' ) {
>-        $gdpr_proc_consent->given_on( dt_from_string() );
>-        $gdpr_proc_consent->refused_on( undef );
>-    } elsif( $gdpr_check eq 'disagreed' ) {
>-        $gdpr_proc_consent->given_on( undef );
>-        $gdpr_proc_consent->refused_on( dt_from_string() );
>-    }
>-    $gdpr_proc_consent->store;
>+my $needs_redirect;
>+foreach my $consent ( @consents ) {
>+    my $check = $vars->{ "check_".$consent->type };
>+    next if !defined($check);    # no choice made
>+    $needs_redirect = 1
>+        if $consent->type eq q/GDPR_PROCESSING/ && !$check && C4::Context->preference('PrivacyPolicyConsent') eq 'Enforced';
>+    next if $consent->given_on && $check || $consent->refused_on && !$check;
>+        # No update if no consent change
>+    $consent->set({
>+        given_on => $check ? dt_from_string() : undef,
>+        refused_on => $check ? undef : dt_from_string(),
>+    })->store;
> }
> 
> # If user refused GDPR consent and we enforce GDPR, logout (when saving)
>-if( $op =~ /save/ && C4::Context->preference('PrivacyPolicyConsent') eq 'Enforced' && $gdpr_proc_consent->refused_on )
>-{
>+if( $needs_redirect ) {
>     print $query->redirect('/cgi-bin/koha/opac-main.pl?logout.x=1');
>     exit;
> }
> 
>-$template->param( patron => $patron );
>-if( $gdpr_proc_consent ) {
>-    $template->param(
>-        gdpr_proc_consent => $gdpr_proc_consent->given_on // q{},
>-        gdpr_proc_refusal => $gdpr_proc_consent->refused_on // q{},
>-    );
>-}
>+$template->param( patron => $patron, consents => \@consents, consent_types => $consent_types );
> 
> output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 };
>diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
>index 5d05a16efd..92ca508ac6 100755
>--- a/t/db_dependent/Koha/Patron.t
>+++ b/t/db_dependent/Koha/Patron.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 24;
>+use Test::More tests => 25;
> use Test::Exception;
> use Test::Warn;
> 
>@@ -1471,6 +1471,7 @@ subtest 'update privacy tests' => sub {
> 
>     plan tests => 5;
> 
>+    $schema->storage->txn_begin;
>     my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { privacy => 1 } });
> 
>     my $old_checkout = $builder->build_object({ class => 'Koha::Old::Checkouts', value => { borrowernumber => $patron->id } });
>@@ -1497,4 +1498,23 @@ subtest 'update privacy tests' => sub {
> 
>     is( $old_checkout->borrowernumber, $anon_patron->id, "Checkout is successfully anonymized");
>     is( $patron->privacy(), 2, "Patron privacy is successfully updated");
>+    $schema->storage->txn_rollback;
>+};
>+
>+subtest 'test patron_consent' => sub {
>+    plan tests => 4;
>+    $schema->storage->txn_begin;
>+
>+    my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
>+    throws_ok { $patron->consent } 'Koha::Exceptions::MissingParameter', 'missing type';
>+
>+    my $consent = $patron->consent('GDPR_PROCESSING');
>+    is( ref $consent, 'Koha::Patron::Consent', 'return type check' );
>+    $consent->given_on('2021-02-03')->store;
>+    undef $consent;
>+    is( $patron->consent('GDPR_PROCESSING')->given_on, '2021-02-03 00:00:00', 'check date' );
>+
>+    is( $patron->consent('NOT_EXIST')->refused_on, undef, 'New empty object for new type' );
>+
>+    $schema->storage->txn_rollback;
> };
>diff --git a/t/db_dependent/Koha/Patron/Consents.t b/t/db_dependent/Koha/Patron/Consents.t
>index ab9ed669d4..4d2bf54d8f 100755
>--- a/t/db_dependent/Koha/Patron/Consents.t
>+++ b/t/db_dependent/Koha/Patron/Consents.t
>@@ -18,9 +18,12 @@
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>+use Data::Dumper qw/Dumper/;
>+use Test::More tests => 2;
>+use Test::MockModule;
>+use Test::MockObject;
> 
>-use Test::More tests => 1;
>-
>+use t::lib::Mocks;
> use t::lib::TestBuilder;
> 
> use Koha::Database;
>@@ -46,3 +49,41 @@ subtest 'Basic tests for Koha::Patron::Consent' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'Method available_types' => sub {
>+    plan tests => 7;
>+    $schema->storage->txn_begin;
>+
>+    # Mock get_enabled_plugins
>+    my $plugins        = [];
>+    my $plugins_module = Test::MockModule->new('Koha::Plugins');
>+    $plugins_module->mock( 'get_enabled_plugins', sub { return @$plugins } );
>+    my $plugin_1 = Test::MockObject->new;
>+    my $data_1   = [ 'plugin_1', { title => { en => 'Title1' }, description => { en => 'Desc1' } } ];
>+    $plugin_1->mock( 'patron_consent_type', sub { return $data_1; } );
>+    my $plugin_2 = Test::MockObject->new;
>+    my $data_2   = [ 'plugin_2', { title => { en => 'Title2' }, description => { en => 'Desc2' } } ];
>+    $plugin_2->mock( 'patron_consent_type', sub { return $data_2; } );
>+    $plugins = [ $plugin_1, $plugin_2 ];
>+
>+    t::lib::Mocks::mock_preference( 'PrivacyPolicyConsent',   'Enforced' );
>+    t::lib::Mocks::mock_preference( 'OPACCustomConsentTypes', 0 );
>+    my $types = Koha::Patron::Consents->available_types;
>+    is( keys %$types, 1, 'Expect one plugin for privacy policy' );
>+    t::lib::Mocks::mock_preference( 'OPACCustomConsentTypes', 1 );
>+    $types = Koha::Patron::Consents->available_types;
>+    is( keys %$types, 3, 'Expect three plugins when allowing custom consents' );
>+    t::lib::Mocks::mock_preference( 'PrivacyPolicyConsent', '' );
>+    $types = Koha::Patron::Consents->available_types;
>+    is( keys %$types,              2,     'Expect two plugins, when pref disabled' );
>+    is( $types->{GDPR_PROCESSING}, undef, 'GDPR key should not be found' );
>+    is_deeply( $types->{plugin_2}, $data_2->[1], 'Check type hash' );
>+
>+    # Let plugin_2 return bad data (hashref)
>+    $data_2 = { not_expected => 1 };
>+    $types  = Koha::Patron::Consents->available_types;
>+    is( keys %$types, 1, 'Expect one plugin, when plugin_2 fails' );
>+    is_deeply( $types->{plugin_1}, $data_1->[1], 'Check type hash' );
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 31503: 140047 | 140048 | 140049 | 140050 | 140080 | 140081 | 140082 | 140083 | 140084 | 140085 | 140087 | 140088 | 140089 | 140090 | 140666 | 140667 | 140668 | 140669 | 140670 | 140671 | 140672 | 141216 | 141217 | 141218 | 141219 | 141220 | 141221 | 141222 | 141263 | 141264 | 141265 | 141266 | 141267 | 141268 | 141269 | 141302 | 141570 | 155000 | 155001 | 155002 | 155003 | 155177 | 155178 | 155179 | 155180 | 155189 | 155190 | 155191 | 155656 | 155657 | 155658 | 155659 | 155660 | 155661 | 155691 | 155694 | 155783 | 155784 | 155785 | 155786 | 155787 | 155788 | 155789 | 155790 | 155791 | 157117 | 157118 | 157119 | 157120 | 157121 | 157122 | 157123 | 157124 | 157125 | 157126 | 157773 | 157774 | 157775 | 157776 | 157777 | 157778 | 157779 | 157780 | 157781 | 157782 | 157783