Bug 31503 - Allow several consent types on the consents tab of OPAC account page
Summary: Allow several consent types on the consents tab of OPAC account page
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 20380 31570
  Show dependency treegraph
 
Reported: 2022-09-01 07:14 UTC by Marcel de Rooy
Modified: 2023-10-26 07:10 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds plugin hooks to allow plugin authors to add their own consent requirements. Consent types introduced will be added to the OPAC account page (Consents tab) using the new `patron_consent_type` hook. A very simple example is given on the Bugzilla report.
Version(s) released in:
23.11.00


Attachments
Bug 31503: Add $patron->consent (3.02 KB, patch)
2022-09-01 14:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Use patron->consent in opac-patron-consent (8.22 KB, patch)
2022-09-01 14:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.97 KB, patch)
2022-09-01 14:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.50 KB, patch)
2022-09-01 14:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Adjust db column patron_consent.type (1.35 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $patron->consent (3.02 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Use patron->consent in opac-patron-consent (8.22 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.97 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.55 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (Experimental) Configurable consent text (7.00 KB, patch)
2022-09-02 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition (1.30 KB, patch)
2022-09-02 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.97 KB, patch)
2022-09-02 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.55 KB, patch)
2022-09-02 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (Experimental) Configurable consent text (7.00 KB, patch)
2022-09-02 08:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Adjust db column patron_consent.type (2.10 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $patron->consent (3.02 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Use patron->consent in opac-patron-consent (8.22 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition (1.30 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.97 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.75 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Configurable consent text (4.23 KB, patch)
2022-09-15 13:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Adjust db column patron_consent.type (2.10 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $patron->consent (3.26 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Use patron->consent in opac-patron-consent (8.22 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition (1.30 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.97 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.75 KB, patch)
2022-10-03 07:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Configurable consent text (4.12 KB, patch)
2022-10-03 07:01 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Adjust db column patron_consent.type (2.11 KB, patch)
2022-10-03 14:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Add $patron->consent (3.27 KB, patch)
2022-10-03 14:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Use patron->consent in opac-patron-consent (8.23 KB, patch)
2022-10-03 14:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition (1.31 KB, patch)
2022-10-03 14:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Add $av_cat->values (1.98 KB, patch)
2022-10-03 14:15 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (10.75 KB, patch)
2022-10-03 14:15 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Configurable consent text (4.13 KB, patch)
2022-10-03 14:15 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: (follow-up) Check return of GetByCode (2.87 KB, patch)
2022-10-04 11:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (QA follow-up) Fix POD warning (857 bytes, patch)
2022-10-10 11:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (1.93 KB, patch)
2023-08-31 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.18 KB, patch)
2023-08-31 09:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (18.86 KB, patch)
2023-08-31 09:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
DO NOT PUSH: Add example plugin (2.22 KB, patch)
2023-08-31 09:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.44 KB, patch)
2023-09-03 06:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (1.94 KB, patch)
2023-09-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.19 KB, patch)
2023-09-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (18.87 KB, patch)
2023-09-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.54 KB, patch)
2023-09-04 09:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
DO NOT PUSH: Add example plugin (2.22 KB, patch)
2023-09-04 09:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: DO NOT PUSH - Add example plugin (2.23 KB, patch)
2023-09-04 09:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (2.00 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.32 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (19.04 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.56 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Styling and pre-selection (6.90 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: DO NOT PUSH - Add example plugin (2.26 KB, patch)
2023-09-15 10:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins (2.36 KB, patch)
2023-09-15 15:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Use KohaPlugins.feature_enabled in place of preference lookup (2.90 KB, patch)
2023-09-15 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (2.01 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.33 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (19.05 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.56 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Styling and pre-selection (6.90 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins (4.19 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled (3.60 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: (follow-up) Remove OPACCustomConsentTypes (6.29 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: DO NOT PUSH - Add example plugin (2.26 KB, patch)
2023-09-18 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (2.04 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.36 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (19.47 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.60 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Styling and pre-selection (6.94 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins (4.39 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled (3.12 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Remove OPACCustomConsentTypes (6.32 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (QA follow-up) Improve count of available_types (1.48 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: DO NOT PUSH - Add example plugin (2.29 KB, patch)
2023-10-13 15:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change patron_consent.type (2.04 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Change usermenu condition, add preference (5.36 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: Make opac-patron-consent more generic (19.38 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Update description for system preference (1.60 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Styling and pre-selection (6.94 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins (4.39 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled (3.12 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (follow-up) Remove OPACCustomConsentTypes (6.32 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (QA follow-up) Improve count of available_types (1.48 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: (QA follow-up) Fix number of tests in rebase (761 bytes, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31503: DO NOT PUSH - Add example plugin (2.29 KB, patch)
2023-10-25 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2022-09-01 07:14:03 UTC

    
Comment 1 Marcel de Rooy 2022-09-01 14:26:51 UTC
Created attachment 140047 [details] [review]
Bug 31503: Add $patron->consent

Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2022-09-01 14:26:55 UTC
Created attachment 140048 [details] [review]
Bug 31503: Use patron->consent in opac-patron-consent

Includes a few small additional changes in template:
[1] op == save (more generic)
[2] agreed/disagreed replaced by 1/0
[3] toggling display of consent_info paragraph

Test plan:
Enable GDPR_Policy.
Test adding/removing consent. Check what happens with
alert, consent_info paragraph and save button.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2022-09-01 14:26:58 UTC
Created attachment 140049 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2022-09-01 14:27:02 UTC
Created attachment 140050 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
In order to simplify the alert for GDPR was moved to the description
text. Note that we could move this also to a AC HTML block at some
point.

Test plan:
First check the form without PATRON_CONSENT category defined.
Add AV category PATRON_CONSENT. Add GDPR_PROCESSING and another one
(e.g. NEWSLETTER).
Verify that you can toggle consent/dissent and that everything is
saved correctly. Note only a real change updates the timestamp.
Comment 5 Marcel de Rooy 2022-09-01 14:28:54 UTC
What I am still adding here, is probably a text block for a consent type from Additional contents.
What is here now already, allows you to define new consent types and register them on the consents page.
Comment 6 Marcel de Rooy 2022-09-02 08:02:17 UTC
Created attachment 140080 [details] [review]
Bug 31503: Adjust db column patron_consent.type

We want to control this field now via authorised values. An ENUM
is too restrictive.
A foreign key on the values of authorised_value for category
'PATRON_CONSENT' is not possible.
This will do for now.

Test plan:
Run dbrev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2022-09-02 08:02:21 UTC
Created attachment 140081 [details] [review]
Bug 31503: Add $patron->consent

Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2022-09-02 08:02:24 UTC
Created attachment 140082 [details] [review]
Bug 31503: Use patron->consent in opac-patron-consent

Includes a few small additional changes in template:
[1] op == save (more generic)
[2] agreed/disagreed replaced by 1/0
[3] toggling display of consent_info paragraph

Test plan:
Enable GDPR_Policy.
Test adding/removing consent. Check what happens with
alert, consent_info paragraph and save button.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2022-09-02 08:02:28 UTC
Created attachment 140083 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2022-09-02 08:02:32 UTC
Created attachment 140084 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
In order to simplify the alert for GDPR was moved to the description
text. Note that we could move this also to a AC HTML block at some
point.

Test plan:
First check the form without PATRON_CONSENT category defined.
Add AV category PATRON_CONSENT. Add GDPR_PROCESSING and another one
(e.g. NEWSLETTER).
Verify that you can toggle consent/dissent and that everything is
saved correctly. Note only a real change updates the timestamp.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2022-09-02 08:02:35 UTC
Created attachment 140085 [details] [review]
Bug 31503: (Experimental) Configurable consent text

This patch allows you to add title and content from the
Additional Contents module but it is still slightly
experimental since we depend on setting the code column
to a value that is not yet supported. Will open a report
for that.

Note however that the template contains a fallback in
case you might not be able to add contents yet.

Test plan:
[1] Add a NEWSLETTER value under AV category PATRON_CONSENT.
[2] Add a PatronConsent record under html customizations. Only
add title and content for Default.
[3] Change code via SQL (awaiting further AC development):
    update additional_contents
        set code='PatronConsent_NEWSLETTER'
    where location='PatronConsent' and idnew=[YOUR_ID]
Note: YOUR_ID refers to the record you added. After this
change, you will be able to add translations under Additional
contents. The code will be copied.
[4] Finally, check patron consent page for your text.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2022-09-02 08:59:19 UTC
Created attachment 140087 [details] [review]
Bug 31503: Change usermenu condition

Test plan:
Check if you see Consents tab depending on GDPR_Policy or having
PATRON_CONSENT authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2022-09-02 08:59:22 UTC
Created attachment 140088 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2022-09-02 08:59:26 UTC
Created attachment 140089 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
In order to simplify the alert for GDPR was moved to the description
text. Note that we could move this also to a AC HTML block at some
point.

Test plan:
First check the form without PATRON_CONSENT category defined.
Add AV category PATRON_CONSENT. Add GDPR_PROCESSING and another one
(e.g. NEWSLETTER).
Verify that you can toggle consent/dissent and that everything is
saved correctly. Note only a real change updates the timestamp.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2022-09-02 08:59:29 UTC
Created attachment 140090 [details] [review]
Bug 31503: (Experimental) Configurable consent text

This patch allows you to add title and content from the
Additional Contents module but it is still slightly
experimental since we depend on setting the code column
to a value that is not yet supported. Will open a report
for that.

Note however that the template contains a fallback in
case you might not be able to add contents yet.

Test plan:
[1] Add a NEWSLETTER value under AV category PATRON_CONSENT.
[2] Add a PatronConsent record under html customizations. Only
add title and content for Default.
[3] Change code via SQL (awaiting further AC development):
    update additional_contents
        set code='PatronConsent_NEWSLETTER'
    where location='PatronConsent' and idnew=[YOUR_ID]
Note: YOUR_ID refers to the record you added. After this
change, you will be able to add translations under Additional
contents. The code will be copied.
[4] Finally, check patron consent page for your text.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2022-09-02 09:10:37 UTC
(In reply to Marcel de Rooy from comment #15)

> This patch allows you to add title and content from the
> Additional Contents module but it is still slightly
> experimental since we depend on setting the code column
> to a value that is not yet supported. Will open a report
> for that.

Bug 31509

> Note however that the template contains a fallback in
> case you might not be able to add contents yet.
Comment 17 Marcel de Rooy 2022-09-08 06:44:53 UTC
The last patch needs a bit more attention on selecting the right record and could benefit from development on 29144.
Furthermore we could think about including opac-memberentry and opac-registration-verify.
But I am bit hesitant to continue here since 29144 is still stuck in PQA ;)
Comment 18 Marcel de Rooy 2022-09-15 05:48:26 UTC
Having another look here
Comment 19 Marcel de Rooy 2022-09-15 13:56:08 UTC
Created attachment 140666 [details] [review]
Bug 31503: Adjust db column patron_consent.type

We want to control this field now via authorised values. An ENUM
is too restrictive.
A foreign key on the values of authorised_value for category
'PATRON_CONSENT' is not possible.
This will do for now.

Test plan:
Run dbrev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 20 Marcel de Rooy 2022-09-15 13:56:13 UTC
Created attachment 140667 [details] [review]
Bug 31503: Add $patron->consent

Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2022-09-15 13:56:17 UTC
Created attachment 140668 [details] [review]
Bug 31503: Use patron->consent in opac-patron-consent

Includes a few small additional changes in template:
[1] op == save (more generic)
[2] agreed/disagreed replaced by 1/0
[3] toggling display of consent_info paragraph

Test plan:
Enable GDPR_Policy.
Test adding/removing consent. Check what happens with
alert, consent_info paragraph and save button.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Marcel de Rooy 2022-09-15 13:56:21 UTC
Created attachment 140669 [details] [review]
Bug 31503: Change usermenu condition

Test plan:
Check if you see Consents tab depending on GDPR_Policy or having
PATRON_CONSENT authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 23 Marcel de Rooy 2022-09-15 13:56:26 UTC
Created attachment 140670 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2022-09-15 13:56:30 UTC
Created attachment 140671 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
Note: the alert for GDPR was moved to the description.

Test plan:
First check the form without any PATRON_CONSENT values in AV. Toggle
the value of pref GDPR_Policy.
Now add GDPR_PROCESSING and another one (e.g. NEWSLETTER).
Refresh the OPAC form. Verify that you can toggle consent/dissent
and that everything is saved correctly. Note only a real change updates
the timestamp.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Marcel de Rooy 2022-09-15 13:56:33 UTC
Created attachment 140672 [details] [review]
Bug 31503: Configurable consent text

Although I planned to implement this via Additional
contents (AC), I am using the AV description here after
all, since AC does not yet provide enough flexibility.

The title on the consents page is constructed from the
consent type now still, and the description is from the
OPAC description in AV (PATRON_CONSENT).

Note: AV allows you to add html in the description, although
it was not meant to do so. When we switch to AC later, we
will have the possibility of html, languages, title and
content.

Second note: We still need AV translations too. See
Bugzilla.

Test plan:
[1] Add a NEWSLETTER value under AV category PATRON_CONSENT.
[2] Add no OPAC description. Save. Hit OPAC consents page.
[3] Add description. Check page again.
[4] Add html to description. Check again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 26 Marcel de Rooy 2022-09-15 13:59:41 UTC
The experimental patch with Additional Contents is obsoleted. The feature is not flexible enough yet. Will open another report to move to AC later.
Comment 27 Martin Renvoize 2022-09-20 13:09:38 UTC
This is looking like a great piece of functionality.

Couple of things to note:
1) I think I expected these new consent options to appear in the self-registration form alongside the existing gdpr processing consent.. or for something to direct the user to consent to them immediately after registration?
2) I find the exposure of the AV code a on the page a little strange.. Some AV's have been abused to use 'Staff' and 'OPAC' or 'Title' and 'Description'.. perhaps this might be a suitable resolution?

I've not dug to deeply yet.. but is there any notification going on anywhere when a consent is later removed?  I could imagine a newsletter consent being removed but that being missed by librarians?

Otherwise, I really like this enhancement.
Comment 28 Marcel de Rooy 2022-09-20 13:39:17 UTC
(In reply to Martin Renvoize from comment #27)

Thx for taking a look. Good questions.

> 1) I think I expected these new consent options to appear in the
> self-registration form alongside the existing gdpr processing consent.. or
> for something to direct the user to consent to them immediately after
> registration?

I treated GDPR as a special one (for historic reasons and when enforced, it rather aggressively asks for consent). Yes, I thought also of adding them on self reg, but later moved away from it. A future extension is allowing to specify opt-in or opt-out. In that case we should add implicit consents for opt-out when finishing a self reg. And think of the best way to communicate that to the user. Apart from GDPR the other consents are now handled as opt-in and not (yet) enforced in some way. The omnibus report also mentions a 'mandatory' type.
To keep it simple here, I did not add them.

> 2) I find the exposure of the AV code a on the page a little strange.. Some
> AV's have been abused to use 'Staff' and 'OPAC' or 'Title' and
> 'Description'.. perhaps this might be a suitable resolution?

AV was actually my fallback. I wanted it on Additional contents but we need to make that a bit more flexible before moving. So I considered this a bit as temporary measure to get it moving. We could do as you say but it is also a bit confusing..

> I've not dug to deeply yet.. but is there any notification going on anywhere
> when a consent is later removed?  I could imagine a newsletter consent being
> removed but that being missed by librarians?

This need is mentioned too on the omnibus. I was thinking of the Reporting module first here. But you could have a job that exports them etc.
If newsletter processing is in Koha, you could simply check real-time.
Comment 29 David Nind 2022-09-30 18:00:12 UTC
The patch no longer applies 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31503: Adjust db column patron_consent.type
Applying: Bug 31503: Add $patron->consent
Using index info to reconstruct a base tree...
M	Koha/Patron.pm
M	t/db_dependent/Koha/Patron.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Koha/Patron.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/Patron.t
Auto-merging Koha/Patron.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 31503: Add $patron->consent
Comment 30 Marcel de Rooy 2022-10-03 07:00:34 UTC
Created attachment 141216 [details] [review]
Bug 31503: Adjust db column patron_consent.type

We want to control this field now via authorised values. An ENUM
is too restrictive.
A foreign key on the values of authorised_value for category
'PATRON_CONSENT' is not possible.
This will do for now.

Test plan:
Run dbrev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 31 Marcel de Rooy 2022-10-03 07:00:39 UTC
Created attachment 141217 [details] [review]
Bug 31503: Add $patron->consent

Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 32 Marcel de Rooy 2022-10-03 07:00:43 UTC
Created attachment 141218 [details] [review]
Bug 31503: Use patron->consent in opac-patron-consent

Includes a few small additional changes in template:
[1] op == save (more generic)
[2] agreed/disagreed replaced by 1/0
[3] toggling display of consent_info paragraph

Test plan:
Enable GDPR_Policy.
Test adding/removing consent. Check what happens with
alert, consent_info paragraph and save button.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 33 Marcel de Rooy 2022-10-03 07:00:47 UTC
Created attachment 141219 [details] [review]
Bug 31503: Change usermenu condition

Test plan:
Check if you see Consents tab depending on GDPR_Policy or having
PATRON_CONSENT authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2022-10-03 07:00:51 UTC
Created attachment 141220 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 35 Marcel de Rooy 2022-10-03 07:00:56 UTC
Created attachment 141221 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
Note: the alert for GDPR was moved to the description.

Test plan:
First check the form without any PATRON_CONSENT values in AV. Toggle
the value of pref GDPR_Policy.
Now add GDPR_PROCESSING and another one (e.g. NEWSLETTER).
Refresh the OPAC form. Verify that you can toggle consent/dissent
and that everything is saved correctly. Note only a real change updates
the timestamp.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Marcel de Rooy 2022-10-03 07:01:01 UTC
Created attachment 141222 [details] [review]
Bug 31503: Configurable consent text

Although I planned to implement this via Additional
contents (AC), I am using the AV description here after
all, since AC does not yet provide enough flexibility.

The title on the consents page is constructed from the
consent type now still, and the description is from the
OPAC description in AV (PATRON_CONSENT).

Note: AV allows you to add html in the description, although
it was not meant to do so. When we switch to AC later, we
will have the possibility of html, languages, title and
content.

Second note: We still need AV translations too. See
Bugzilla.

Test plan:
[1] Add a NEWSLETTER value under AV category PATRON_CONSENT.
[2] Add no OPAC description. Save. Hit OPAC consents page.
[3] Add description. Check page again.
[4] Add html to description. Check again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 37 Marcel de Rooy 2022-10-03 07:04:29 UTC
(In reply to David Nind from comment #29)
> The patch no longer applies 8-(..
> 
> Apply? [(y)es, (n)o, (i)nteractive] y
> Applying: Bug 31503: Adjust db column patron_consent.type
> Applying: Bug 31503: Add $patron->consent
> Using index info to reconstruct a base tree...
> M	Koha/Patron.pm
> M	t/db_dependent/Koha/Patron.t
> Falling back to patching base and 3-way merge...
> Auto-merging t/db_dependent/Koha/Patron.t
> CONFLICT (content): Merge conflict in t/db_dependent/Koha/Patron.t
> Auto-merging Koha/Patron.pm
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 31503: Add $patron->consent

Try again please David.
Note that this test now fails on master in subtest 19 -  notify_library_of_registration() with and without this patch set. So can be ignored, should be addressed somewhere else.
Comment 38 David Nind 2022-10-03 14:14:39 UTC
Created attachment 141263 [details] [review]
Bug 31503: Adjust db column patron_consent.type

We want to control this field now via authorised values. An ENUM
is too restrictive.
A foreign key on the values of authorised_value for category
'PATRON_CONSENT' is not possible.
This will do for now.

Test plan:
Run dbrev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 39 David Nind 2022-10-03 14:14:45 UTC
Created attachment 141264 [details] [review]
Bug 31503: Add $patron->consent

Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 David Nind 2022-10-03 14:14:51 UTC
Created attachment 141265 [details] [review]
Bug 31503: Use patron->consent in opac-patron-consent

Includes a few small additional changes in template:
[1] op == save (more generic)
[2] agreed/disagreed replaced by 1/0
[3] toggling display of consent_info paragraph

Test plan:
Enable GDPR_Policy.
Test adding/removing consent. Check what happens with
alert, consent_info paragraph and save button.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 41 David Nind 2022-10-03 14:14:56 UTC
Created attachment 141266 [details] [review]
Bug 31503: Change usermenu condition

Test plan:
Check if you see Consents tab depending on GDPR_Policy or having
PATRON_CONSENT authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 David Nind 2022-10-03 14:15:02 UTC
Created attachment 141267 [details] [review]
Bug 31503: Add $av_cat->values

Test plan:
Run t/db_dependent/AuthorisedValues.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 43 David Nind 2022-10-03 14:15:08 UTC
Created attachment 141268 [details] [review]
Bug 31503: Make opac-patron-consent more generic

We now allow multiple consent types as defined under AV
(read: authorised values) category 'PATRON_CONSENT'.
The description is used on the form too.

This is the base patch for multiple consents. We can do some more
polishing on a follow-up or another report even.
Note: the alert for GDPR was moved to the description.

Test plan:
First check the form without any PATRON_CONSENT values in AV. Toggle
the value of pref GDPR_Policy.
Now add GDPR_PROCESSING and another one (e.g. NEWSLETTER).
Refresh the OPAC form. Verify that you can toggle consent/dissent
and that everything is saved correctly. Note only a real change updates
the timestamp.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 44 David Nind 2022-10-03 14:15:14 UTC
Created attachment 141269 [details] [review]
Bug 31503: Configurable consent text

Although I planned to implement this via Additional
contents (AC), I am using the AV description here after
all, since AC does not yet provide enough flexibility.

The title on the consents page is constructed from the
consent type now still, and the description is from the
OPAC description in AV (PATRON_CONSENT).

Note: AV allows you to add html in the description, although
it was not meant to do so. When we switch to AC later, we
will have the possibility of html, languages, title and
content.

Second note: We still need AV translations too. See
Bugzilla.

Test plan:
[1] Add a NEWSLETTER value under AV category PATRON_CONSENT.
[2] Add no OPAC description. Save. Hit OPAC consents page.
[3] Add description. Check page again.
[4] Add html to description. Check again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 45 David Nind 2022-10-03 14:23:20 UTC
Testing notes (using KTD).

The only thing I noted was:
1. If there is no GPPR_PROCESSING authorized value for PATRON_CONSENT, then in the OPAC > your consents the text displayed is "Consent for GDPR_PROCESSING" with Yes and No options.
2. If a GPR_PROCESSING authorized value is added without any descriptions, then the text in [1] is displayed.
3. Is that text editable, or is that just a default that should be changed/customised by updating the descriptions?

My attemopt at a consolidated test plan:
1. Apply patches.
2. Update database.
3. Restart all the things (flush_memcached and restart_all).
4. Run the tests - these should pass:
   - prove t/db_dependent/Koha/Patron.t
   - prove t/db_dependent/AuthorisedValues.t
5. Enable the GDPR_Policy system preference and test (add any URL for PrivacyPolicyURL system preference, test with enforced and permissive options):
   - Go to the OPAC
   - Log in
   - You are prompted (under your consents) to "Consent for GDPR_PROCESSING"
   - Answer Yes
   - Response is "Your consent was registered on [Date/time stamp]
   - Change consent to No
   - Response is "We registered that you did not consent on [Date/time stamp]"
6. Disable the GDPR_Policy system preference - check that there is no "your consents" section link for your account (note that there is still a page that is accessible if you access http://127.0.0.1:8080/cgi-bin/koha/opac-patron-consent.pl - but that you can't do anything and the Save button is disabled).
7. Re-enable the GDPR_Policy system preference - check that there is now a "your consents" section for your account. The previous consent should be shown.
8. Check for the new authorized value category PATRON_CONSENT. Note: Without any values, "Consent for GDPR_PROCESSING" is the only option under your account > your consents in the OPAC.
9. Add GDPR_PROCESSING without a Description or Description (OPAC).
10. Refresh the OPAC form. 
11. Verify that you can toggle consent/dissent and that everything is saved correctly:
    - Note that only a real change updates the timestamp.
    - Note the text displayed for the consent [1].
12. Add a NEWSLETTER value under the authorized value category PATRON_CONSENT without any descriptions.
13. Add a Description. Save. Refresh the OPAC consents page.
14. Add a Description (OPAC). Refresh and check the OPAC consents page again.
15. Add HTML to the OPAC description. Refresh and check the OPAC consents page again. For example: This is some text for the <strong>newsletter consent</strong> to <br/> display in the OPAC.
16. Verify that you can toggle consent/dissent for the NEWSLETTER and that everything is saved correctly.

[1] Text displayed for GDPR_PROCESSING authorized value if no text enterted into Description or Description (OPAC) in the PATRON_CONSENT authorized values.

Consent for GDPR_PROCESSING 

Please read the privacy policy. [privacy policy is linked to page URL set in PrivacyPolicyURL]

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 remove your account within a reasonable time.

Do you agree with our processing of your personal data as outlined in the policy?

Yes
No
Comment 46 Marcel de Rooy 2022-10-04 11:39:48 UTC
(In reply to David Nind from comment #45)

Thanks David! Great.

> The only thing I noted was:
> 1. If there is no GPPR_PROCESSING authorized value for PATRON_CONSENT, then
> in the OPAC > your consents the text displayed is "Consent for
> GDPR_PROCESSING" with Yes and No options.
> 2. If a GPR_PROCESSING authorized value is added without any descriptions,
> then the text in [1] is displayed.
> 3. Is that text editable, or is that just a default that should be
> changed/customised by updating the descriptions?

You found a small flaw! I expected AV.GetByCode to return nothing when there is no description in authorised values. But it just returns the code you were looking for.. Will fix that now.

The idea was: If there is no description, use the historic template text (not editable). Do you want to change, add something in AV. Or in Additional Contents on a follow-up report.
Comment 47 Marcel de Rooy 2022-10-04 11:50:13 UTC
Created attachment 141302 [details] [review]
Bug 31503: (follow-up) Check return of GetByCode

AV.GetByCode returns code when there is no description. So we should
check the return.

This patch also makes the use of the Do you agree-question a bit more
consistent. No need to include it in the description.

Test plan:
Check the OPAC consents page when you have an AV description for a
consent type and when you do not. For GDPR you should see the original
GDPR text when there is no description.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 48 Marcel de Rooy 2022-10-04 11:51:31 UTC
(In reply to Marcel de Rooy from comment #46)
> (In reply to David Nind from comment #45)
> 
> The idea was: If there is no description, use the historic template text
> (not editable). Do you want to change, add something in AV. Or in Additional
> Contents on a follow-up report.

This only talks about GDPR.
Comment 49 David Nind 2022-10-04 12:00:49 UTC
(In reply to Marcel de Rooy from comment #46)

> The idea was: If there is no description, use the historic template text
> (not editable). Do you want to change, add something in AV. Or in Additional
> Contents on a follow-up report.

Thanks Marcel! I wasn't sure exactly what was expected.
Comment 50 Marcel de Rooy 2022-10-04 13:38:24 UTC
From IRC:

[15:34] <ashimema> 28 has all good answers.. looks like you've thought about most of it and are planning them for a continuation in the tree of bugs..
[15:34] <ashimema> so don't let me comment block anything 🙂
Comment 51 Nick Clemens 2022-10-05 13:00:42 UTC
Minor QA tool failure

Why use AV for consents? Looking ahead to bug 20380 it seems the consents will have more options - maybe we could create a 'consent_types' table and use that?

Currently too - branch limits are half implemented - I don't see the tab if my only consent is branch limited - but I can force go to the page and see the consent options - was this a plan, or a side effect of choosing AV?

The description displays in the form, but the title is still the AV - can let go in light of 31570, but it is lees friendly to see "Consent for GDPR_PROCESSING"


+use constant AV_CONSENT => 'PATRON_CONSENT';
Why not just use the string in the search? Same question for existing:
 use constant GDPR_PROCESSING => 'GDPR_PROCESSING';
Comment 52 Marcel de Rooy 2022-10-10 11:10:10 UTC
(In reply to Nick Clemens from comment #51)

Thx for QA

> Minor QA tool failure
Fixed. Renamed sub, forgot POD header. Adding follow-up.

> Why use AV for consents? Looking ahead to bug 20380 it seems the consents
> will have more options - maybe we could create a 'consent_types' table and
> use that?
Could be done later indeed. Decided here to start with AV and its existing functionality.

> Currently too - branch limits are half implemented - I don't see the tab if
> my only consent is branch limited - but I can force go to the page and see
> the consent options - was this a plan, or a side effect of choosing AV?
Good catch. This is the most serious issue imo. And a blocker.
Would addressing this be sufficient for you to pass QA? If not, I will probably have to switch first to Additional contents fixing. And try this again for 23.05 or 23.11.

> The description displays in the form, but the title is still the AV - can
> let go in light of 31570, but it is lees friendly to see "Consent for
> GDPR_PROCESSING"
Correct. Should be addressed later on.

> +use constant AV_CONSENT => 'PATRON_CONSENT';
> Why not just use the string in the search? Same question for existing:
>  use constant GDPR_PROCESSING => 'GDPR_PROCESSING';
Using constants is good practice. When we add another instance, we can refer to the constant.
Comment 53 Marcel de Rooy 2022-10-10 11:10:52 UTC
Created attachment 141570 [details] [review]
Bug 31503: (QA follow-up) Fix POD warning

FAIL   pod coverage
    POD is missing for 'values'

I renamed the sub in the process :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 54 Martin Renvoize 2022-10-10 15:52:58 UTC
I asked myself the same question regards the use of AV here.. and would likely ask it again if it was switch to additional_contents.. it feels like this is a big enough feature tree that starting with a fresh new table instead of trying to plumb extra functionality into other tables is the right way to go... 

The account types stuff is a clear example of where/why this makes sense.. we added lots of functionality to it really quite quickly once we'd converted from AV to distinct tables.. AV's and Additional Contents end up very restrictive if you are actually trying to tie functionality to this stuff...

You talk about future enhancements Marcel.. any change you can write up a tree of bugs to describe where your headed with the big picture?
Comment 55 Marcel de Rooy 2022-10-11 11:06:50 UTC
(In reply to Martin Renvoize from comment #54)
> I asked myself the same question regards the use of AV here.. and would
> likely ask it again if it was switch to additional_contents.. it feels like
> this is a big enough feature tree that starting with a fresh new table
> instead of trying to plumb extra functionality into other tables is the
> right way to go... 
> 
> The account types stuff is a clear example of where/why this makes sense..
> we added lots of functionality to it really quite quickly once we'd
> converted from AV to distinct tables.. AV's and Additional Contents end up
> very restrictive if you are actually trying to tie functionality to this
> stuff...
> 
> You talk about future enhancements Marcel.. any change you can write up a
> tree of bugs to describe where your headed with the big picture?

Thanks, Martin.
Reworking this along some other lines will certainly need more time.
I was certainly not planning to make this as large as Accounts ;) Baby steps..
Will come back here later.
Comment 56 Marcel de Rooy 2023-08-29 14:58:19 UTC
I will try to get this further now again with a plugin approach (with the courage of despair..)

It will use plugin hook patron_consent_type.
Comment 57 Marcel de Rooy 2023-08-31 09:44:59 UTC
Created attachment 155000 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 58 Marcel de Rooy 2023-08-31 09:45:02 UTC
Created attachment 155001 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 59 Marcel de Rooy 2023-08-31 09:45:05 UTC
Created attachment 155002 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Comment 60 Marcel de Rooy 2023-08-31 09:45:08 UTC
Created attachment 155003 [details] [review]
DO NOT PUSH: Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 61 Marcel de Rooy 2023-08-31 10:11:10 UTC
This is another approach to get this further using plugins instead of Authorised values, Additional Contents or database translations.
These three areas do currently not offer enough flexibility, translation options, etc. Some developments in that area are stuck for longer time already.
Comment 62 Marcel de Rooy 2023-08-31 10:12:40 UTC
David: Could you please test this change? The looks are quite the same, but the internal implementation is quite different.
Comment 63 David Nind 2023-09-03 06:29:50 UTC
Created attachment 155177 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

Signed-off-by: David Nind <david@davidnind.com>
Comment 64 David Nind 2023-09-03 06:34:21 UTC
Created attachment 155178 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 65 David Nind 2023-09-03 06:34:24 UTC
Created attachment 155179 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 66 David Nind 2023-09-03 06:34:27 UTC
Created attachment 155180 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Signed-off-by: David Nind <david@davidnind.com>
Comment 67 David Nind 2023-09-03 06:38:45 UTC
Testing notes (using KTD):

1. Apply patches - got a weird message (see [1]).
2. Update database (updatedatabase).
3. Restart everything (flush_memcached and restart_all).
4. Run the tests - these should pass:
   - prove t/db_dependent/Koha/Patron.t
   - prove t/db_dependent/AuthorisedValues.t
5. Install and enable the plugin included with the patches:
perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
6. Restart everything (flush_memcached and restart_all) (I had to do this, otherwise I got an error trace when accessing the OPAC account consents page). 
7. Enable the new system preference OPACCustomConsentTypes.
8. Go to the OPAC > [user account] > Consents tab.
9. If the PrivacyPolicyConsent system preference is disabled, then only the Newsletter consent is displayed:
    9.1 If you select Yes + Save = "Your consent was registered on [date and timestamp]."
    9.2 If you select No + Save = "We registered that you did not consent on [date and timestamp]."
10. The standard PrivacyPolicyConsent system preference options should continue to work as expected if OPACCustomConsentTypes is set to 'Don't allow' (enter a URL in  PrivacyPolicyURL, for example http://example.com):
   10.1 disabled: No consent tab in OPAC account area)
   10.2 enforced: Verify that you can agree (Yes) or disagree (No) with the consent and press Save:
         . If Yes, a comment is added: "Your consent was registered on [date and timestamp]."
         . If No, you are logged out. (A comment is added under consents: "We registered that you did not consent on [date and timestamp]." - you only see this if you log in again)
   10.3 permissive: Verify that you can agree (Yes) or disagree (No) with the consent and press Save:
         .  If Yes, a comment is added: "Your consent was registered on [date and timestamp]."
         . If No, a comment is added and you are not logged out: "We registered that you did not consent on [date and timestamp]."
11. If both OPACCustomConsentTypes and PrivacyPolicyConsent system prferences are enabled, in addition to the default 'Privacy policy consent', there is now the  Newsletter consent - the options for PrivacyPolicyConsent should continue to work as expected:
   11.1 disabled: the default privacy policy consent doesn't appear, but the Newsletter consent (from the plugin) appears as per step 9: 
   11.2 enforced: same as for step 10.2 for the privacy policy consent, and step 9 for the newsletter consent
   11.3 permissive: same as for step 10.3 for the privacy policy consent, and step 9 for the newsletter consent

I hope I tested this correctly!

I added a follow-up patch to amend the OPACCustomConsentTypes description - note in CAPS and bold (the same as other Notes), and linked to PrivacyPolicyConsent.

I had trouble attaching the patches because of the example plugin patch - I hope I haven't screwed anything up!


[1] Output I get when applying the patches:

git bz apply 31503

Bug 31503 - Allow several consent types on the consents tab of OPAC account page

155000 - Bug 31503: Change patron_consent.type
155001 - Bug 31503: Change usermenu condition, add preference
155002 - Bug 31503: Make opac-patron-consent more generic
155003 - DO NOT PUSH: Add example plugin

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31503: Change patron_consent.type
Applying: Bug 31503: Change usermenu condition, add preference
Applying: Bug 31503: Make opac-patron-consent more generic
Applying: DO NOT PUSH: Add example plugin
Moving to starting point
Adding bug reference   6b2e2e2 DO NOT PUSH: Add example plugin
Cleaning up back to original state on error
Traceback (most recent call last):
  File "/usr/bin/git-bz", line 2834, in <module>
    applied = do_apply(bug_ref)
  File "/usr/bin/git-bz", line 1977, in do_apply
    add_url(bug, commits)
  File "/usr/bin/git-bz", line 1719, in add_url
    add_url_to_head_commit(commit, bug)
  File "/usr/bin/git-bz", line 1682, in add_url_to_head_commit
    git.commit(file="-", amend=True, _input=input)
  File "/usr/bin/git-bz", line 197, in f
    o = git_run(command, *args, **kwargs)
  File "/usr/bin/git-bz", line 167, in git_run
    output, error = process.communicate(input)
  File "/usr/lib/python3.9/subprocess.py", line 1134, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/usr/lib/python3.9/subprocess.py", line 1961, in _communicate
    input_view = memoryview(self._input)
TypeError: memoryview: a bytes-like object is required, not 'str'
Comment 68 Marcel de Rooy 2023-09-04 09:06:52 UTC
Created attachment 155189 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 69 Marcel de Rooy 2023-09-04 09:06:55 UTC
Created attachment 155190 [details] [review]
DO NOT PUSH: Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 70 Marcel de Rooy 2023-09-04 09:09:44 UTC
(In reply to David Nind from comment #66)
> Signed-off-by: David Nind <david@davidnind.com>

Thx David for your quick signoff.
Comment 71 Marcel de Rooy 2023-09-04 09:12:00 UTC
(In reply to David Nind from comment #67)
> I had trouble attaching the patches because of the example plugin patch - I
> hope I haven't screwed anything up!

Not sure what was going wrong there exactly. But I restored the example patch.

> Applying: DO NOT PUSH: Add example plugin
> Moving to starting point
> Adding bug reference   6b2e2e2 DO NOT PUSH: Add example plugin
> Cleaning up back to original state on error
> Traceback (most recent call last):
>   File "/usr/bin/git-bz", line 2834, in <module>
>     applied = do_apply(bug_ref)
>   File "/usr/bin/git-bz", line 1977, in do_apply
>     add_url(bug, commits)
>   File "/usr/bin/git-bz", line 1719, in add_url
>     add_url_to_head_commit(commit, bug)
>   File "/usr/bin/git-bz", line 1682, in add_url_to_head_commit
>     git.commit(file="-", amend=True, _input=input)
>   File "/usr/bin/git-bz", line 197, in f
>     o = git_run(command, *args, **kwargs)
>   File "/usr/bin/git-bz", line 167, in git_run
>     output, error = process.communicate(input)
>   File "/usr/lib/python3.9/subprocess.py", line 1134, in communicate
>     stdout, stderr = self._communicate(input, endtime, timeout)
>   File "/usr/lib/python3.9/subprocess.py", line 1961, in _communicate
>     input_view = memoryview(self._input)
> TypeError: memoryview: a bytes-like object is required, not 'str'

Hmm. I can reproduce this too now. Still looking a bit further.
Comment 72 Marcel de Rooy 2023-09-04 09:14:21 UTC
Created attachment 155191 [details] [review]
Bug 31503: DO NOT PUSH - Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 73 Marcel de Rooy 2023-09-04 09:15:51 UTC
(In reply to Marcel de Rooy from comment #71)
> Hmm. I can reproduce this too now. Still looking a bit further.

Changed commit title. And cannot reproduce any more..
Comment 74 David Nind 2023-09-04 10:05:05 UTC
(In reply to Marcel de Rooy from comment #73)
> (In reply to Marcel de Rooy from comment #71)
> > Hmm. I can reproduce this too now. Still looking a bit further.
> 
> Changed commit title. And cannot reproduce any more..

It now applies cleanly for me now as well - weird!
Comment 75 Marcel de Rooy 2023-09-13 11:30:40 UTC
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/usermenu.inc


Comes from 27378. Waiting a bit for feedback on that report.
Comment 76 Marcel de Rooy 2023-09-13 12:16:09 UTC
I just hoped that it would come further now, lets see ;)
Comment 77 Martin Renvoize 2023-09-14 16:08:41 UTC
This is a really cool feature.. I love the move to plugins as you can now really add functionality to the system and add a user consent step as part of that functionality improvement.

I've rebased it locally and am playing around with it.. some small things I'd like to see to get it over the finish line, but actually I think it's really close now.
Comment 78 Martin Renvoize 2023-09-15 07:15:51 UTC
There are some system preference renames not accounted for yet in my local rebase.  I'm working on those.

I'm not sure I like how we've dropped the alert warning for privacy policy. I think it's important to highlight with that particular feature that the account is deleted and you are logged out if you do not accept and I think the reduction to 'just another consent' like the plugin one's your introducing here is a reduction of functionality.

I'll work on a follow-up to keep it moving.

Talking system preferences.. do you think libraries will want to independantly turn off the plugin based consents?  I think perhaps that the system preference introduced is superflous to requirements. I think I'd just check for the occurrence of any plugins carrying the consent method instead.
Comment 79 Martin Renvoize 2023-09-15 10:28:13 UTC
Created attachment 155656 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 80 Martin Renvoize 2023-09-15 10:28:16 UTC
Created attachment 155657 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 81 Martin Renvoize 2023-09-15 10:28:19 UTC
Created attachment 155658 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 82 Martin Renvoize 2023-09-15 10:28:22 UTC
Created attachment 155659 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 83 Martin Renvoize 2023-09-15 10:28:24 UTC
Created attachment 155660 [details] [review]
Bug 31503: (follow-up) Styling and pre-selection

This patch restores some of the wording and styling from the original
GDPR/Privacy policy feature including highlighting in a warning box.

I also remove the jQuery approach for pre-selecting already selected
radio options and rely on standard TT processing instead.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 84 Martin Renvoize 2023-09-15 10:28:27 UTC
Created attachment 155661 [details] [review]
Bug 31503: DO NOT PUSH - Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 85 Martin Renvoize 2023-09-15 10:35:46 UTC
I've added a minor follow-up to restore a little of the original styling and wording from the Privacy Policy feature.

Doing the above made me wonder if the 'description' display for plugin generated consent types should not be wrapped in <p>.. allowing for plugin authors to have more control over the final display of the description (for example adding a warning box as we do for the privacy policy one again now).

I'm not 100% sure about the Koha::Patron->consent($type) accessor introduced here.. but I can see why you've done it that way and this patchset has been through a few iterations already.. so not a blocker.

I'm also not in total agreement around the new system preference introduced. I believe you've done this for performance reasons but I tihnk we could work around it with caching instead.. again.. I don't think this should be a blocker.

Finally.. and this one may be more of a blocker.  I'm not sure how one would go about translating the description blocks introduced by plugins. We could pass language (or rely on context) through the available types method.. in fact, perhaps that's the answer.. we just leave it to the plugin author to look at lang from context and return the right thing when 'patron_consent_type' is called.  I'd love to see this in the example plugin to highlight it's possible and works.. I may add that shortly if I can find a few more minutes.

Generally, though, I'm happy with this and feel it can move forward. I'd love one final QA set of eyes.. I'll ask around for you.
Comment 86 Marcel de Rooy 2023-09-15 11:24:09 UTC
(In reply to Martin Renvoize from comment #85)
> Finally.. and this one may be more of a blocker.  I'm not sure how one would
> go about translating the description blocks introduced by plugins. We could
> pass language (or rely on context) through the available types method.. in
> fact, perhaps that's the answer.. we just leave it to the plugin author to
> look at lang from context and return the right thing when
> 'patron_consent_type' is called.  I'd love to see this in the example plugin
> to highlight it's possible and works.. I may add that shortly if I can find
> a few more minutes.

Thx Martin. Yes, I had a nl-NL key in my testing. Works fine. And will start using this approach in 22.11 soon.
It feels like translation should be handled elsewhere. But we are waiting for long already on translation for authorised values, database translation, etc. So this is sufficient for now. Normally a local plugin will not contain many translations.
Comment 87 Martin Renvoize 2023-09-15 15:12:19 UTC
Created attachment 155691 [details] [review]
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins

This patch adds a 'feature_enabled' accessor to the Koha::Plugins class.
Passing the plugin method name for the plugin feature you're looking for
will return a boolean denoting whether any enabled plugins carry said
method.

Incuded in the patch is an addition to the TT KohaPlugins plugin to
enable quick access to this function.
Comment 88 Martin Renvoize 2023-09-15 15:37:27 UTC
OK, I've had a go at providing a performant method to allow us to drop the new system preference.. I'll continue on Monday.. 

I also think I've worked out the translation stuff, adding in translations for the descriptions into the plugin method shouldn't be too hard :)

Thanks for your patience here Marcel.
Comment 89 Martin Renvoize 2023-09-15 15:43:04 UTC
Created attachment 155694 [details] [review]
Bug 31503: Use KohaPlugins.feature_enabled in place of preference lookup
Comment 90 Martin Renvoize 2023-09-18 09:57:02 UTC
Created attachment 155783 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 91 Martin Renvoize 2023-09-18 09:57:05 UTC
Created attachment 155784 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Martin Renvoize 2023-09-18 09:57:08 UTC
Created attachment 155785 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Martin Renvoize 2023-09-18 09:57:11 UTC
Created attachment 155786 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Martin Renvoize 2023-09-18 09:57:14 UTC
Created attachment 155787 [details] [review]
Bug 31503: (follow-up) Styling and pre-selection

This patch restores some of the wording and styling from the original
GDPR/Privacy policy feature including highlighting in a warning box.

I also remove the jQuery approach for pre-selecting already selected
radio options and rely on standard TT processing instead.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Martin Renvoize 2023-09-18 09:57:17 UTC
Created attachment 155788 [details] [review]
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins

This patch adds a 'feature_enabled' accessor to the Koha::Plugins class.
Passing the plugin method name for the plugin feature you're looking for
will return a boolean denoting whether any enabled plugins carry said
method.

Included in the patch is an addition to the TT KohaPlugins plugin to
enable quick access to this function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Martin Renvoize 2023-09-18 09:57:20 UTC
Created attachment 155789 [details] [review]
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled

This patch switches the code from using
Koha.preference('OPACCustomConsentTypes') to using
KohaPlugins.feature_enabled('patron_consent_type').

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Martin Renvoize 2023-09-18 09:57:23 UTC
Created attachment 155790 [details] [review]
Bug 31503: (follow-up) Remove OPACCustomConsentTypes

This patch removes all trace of the original OPACCustomConsentTypes
preference.  We now use Koha::Plugins->feature_enabled in preference.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Martin Renvoize 2023-09-18 09:57:25 UTC
Created attachment 155791 [details] [review]
Bug 31503: DO NOT PUSH - Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 99 Marcel de Rooy 2023-09-18 10:10:28 UTC
(In reply to Martin Renvoize from comment #98)
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Ah great Martin. Thanks. Will have a look at your changes.
Comment 100 Marcel de Rooy 2023-10-13 14:38:35 UTC
Does not apply anymore ):
Comment 101 Marcel de Rooy 2023-10-13 14:45:26 UTC
    #   Failed test 'Article requests in 10 days'
    #   at /usr/share/koha/t/db_dependent/Koha/Patron.t line 401.
     #          got: '0'
    #     expected: '1'
     # Looks like you failed 1 test of 19.

Waiting for 35041 ;)
Comment 102 Marcel de Rooy 2023-10-13 14:45:47 UTC
    #   Failed test 'Expect one type for privacy policy'
    #   at /usr/share/koha/t/db_dependent/Koha/Patron/Consents.t line 59.
    #          got: '2'
    #     expected: '1'
     # Looks like you failed 1 test of 7.

More trouble
Comment 103 Marcel de Rooy 2023-10-13 14:47:38 UTC
 WARN   t/db_dependent/Koha/Patron/Consents.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 9, now: 10)

 WARN   t/db_dependent/Koha/Plugins/Plugins.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 104, now: 110)

Bah, dirty!
Comment 104 Marcel de Rooy 2023-10-13 15:36:10 UTC
(In reply to Marcel de Rooy from comment #102)
>     #   Failed test 'Expect one type for privacy policy'
>     #   at /usr/share/koha/t/db_dependent/Koha/Patron/Consents.t line 59.
>     #          got: '2'
>     #     expected: '1'
>      # Looks like you failed 1 test of 7.
> 
> More trouble

Fixed in follow-up

(In reply to Marcel de Rooy from comment #103)
>  WARN   t/db_dependent/Koha/Patron/Consents.t
>    WARN   tidiness
>                 The file is less tidy than before (bad/messy lines before:
> 9, now: 10)
> 
>  WARN   t/db_dependent/Koha/Plugins/Plugins.t
>    WARN   tidiness
>                 The file is less tidy than before (bad/messy lines before:
> 104, now: 110)
> 
> Bah, dirty!

Fixed inline
Comment 105 Marcel de Rooy 2023-10-13 15:37:01 UTC
Created attachment 157117 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 106 Marcel de Rooy 2023-10-13 15:37:04 UTC
Created attachment 157118 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 107 Marcel de Rooy 2023-10-13 15:37:07 UTC
Created attachment 157119 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 108 Marcel de Rooy 2023-10-13 15:37:10 UTC
Created attachment 157120 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 109 Marcel de Rooy 2023-10-13 15:37:12 UTC
Created attachment 157121 [details] [review]
Bug 31503: (follow-up) Styling and pre-selection

This patch restores some of the wording and styling from the original
GDPR/Privacy policy feature including highlighting in a warning box.

I also remove the jQuery approach for pre-selecting already selected
radio options and rely on standard TT processing instead.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Marcel de Rooy 2023-10-13 15:37:15 UTC
Created attachment 157122 [details] [review]
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins

This patch adds a 'feature_enabled' accessor to the Koha::Plugins class.
Passing the plugin method name for the plugin feature you're looking for
will return a boolean denoting whether any enabled plugins carry said
method.

Included in the patch is an addition to the TT KohaPlugins plugin to
enable quick access to this function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

[EDIT] Tidied t/db_dependent/Koha/Plugins/Plugins.t
[EDIT] Tidied Koha/Template/Plugin/KohaPlugins.pm

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 111 Marcel de Rooy 2023-10-13 15:37:18 UTC
Created attachment 157123 [details] [review]
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled

This patch switches the code from using
Koha.preference('OPACCustomConsentTypes') to using
KohaPlugins.feature_enabled('patron_consent_type').

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Marcel de Rooy 2023-10-13 15:37:21 UTC
Created attachment 157124 [details] [review]
Bug 31503: (follow-up) Remove OPACCustomConsentTypes

This patch removes all trace of the original OPACCustomConsentTypes
preference.  We now use Koha::Plugins->feature_enabled in preference.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Marcel de Rooy 2023-10-13 15:37:24 UTC
Created attachment 157125 [details] [review]
Bug 31503: (QA follow-up) Improve count of available_types

Tidied one other line.

Test plan:
Run t/db_dependent/Koha/Patron/Consents.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 114 Marcel de Rooy 2023-10-13 15:37:27 UTC
Created attachment 157126 [details] [review]
Bug 31503: DO NOT PUSH - Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 115 Marcel de Rooy 2023-10-13 15:37:50 UTC
Tomas: Its yours again. Roadmap :)
Comment 116 Marcel de Rooy 2023-10-13 15:41:11 UTC
RM: Needs DBIx schema change
Comment 117 Tomás Cohen Arazi 2023-10-24 18:39:22 UTC
Please rebase ASAP. Thanks!
Comment 118 Marcel de Rooy 2023-10-25 11:26:12 UTC
(In reply to Tomás Cohen Arazi from comment #117)
> Please rebase ASAP. Thanks!

Heh    Parse errors: Bad plan.  You planned 29 tests but ran 30.
Comment 119 Marcel de Rooy 2023-10-25 11:32:02 UTC
Created attachment 157773 [details] [review]
Bug 31503: Change patron_consent.type

Test plan:
Run db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 120 Marcel de Rooy 2023-10-25 11:32:05 UTC
Created attachment 157774 [details] [review]
Bug 31503: Change usermenu condition, add preference

Adds preference OPACCustomConsentTypes.

Test plan:
Run dbrev.
Toggle value of OPACCustomConsentTypes / PrivacyPolicyConsent.
Check user page for Consents tab.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 121 Marcel de Rooy 2023-10-25 11:32:08 UTC
Created attachment 157775 [details] [review]
Bug 31503: Make opac-patron-consent more generic

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>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 122 Marcel de Rooy 2023-10-25 11:32:11 UTC
Created attachment 157776 [details] [review]
Bug 31503: (follow-up) Update description for system preference

Update new OPACCustomConsentType system preference description - bold
and capitalize NOTE for consistency, link to PrivacyPolicyConsent
system preference for consistency, and other minor changes.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 123 Marcel de Rooy 2023-10-25 11:32:14 UTC
Created attachment 157777 [details] [review]
Bug 31503: (follow-up) Styling and pre-selection

This patch restores some of the wording and styling from the original
GDPR/Privacy policy feature including highlighting in a warning box.

I also remove the jQuery approach for pre-selecting already selected
radio options and rely on standard TT processing instead.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 124 Marcel de Rooy 2023-10-25 11:32:17 UTC
Created attachment 157778 [details] [review]
Bug 31503: (follow-up) Add 'feature_enabled' to Koha::Plugins

This patch adds a 'feature_enabled' accessor to the Koha::Plugins class.
Passing the plugin method name for the plugin feature you're looking for
will return a boolean denoting whether any enabled plugins carry said
method.

Included in the patch is an addition to the TT KohaPlugins plugin to
enable quick access to this function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

[EDIT] Tidied t/db_dependent/Koha/Plugins/Plugins.t
[EDIT] Tidied Koha/Template/Plugin/KohaPlugins.pm

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 125 Marcel de Rooy 2023-10-25 11:32:20 UTC
Created attachment 157779 [details] [review]
Bug 31503: (follow-up) Use KohaPlugins.feature_enabled

This patch switches the code from using
Koha.preference('OPACCustomConsentTypes') to using
KohaPlugins.feature_enabled('patron_consent_type').

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 126 Marcel de Rooy 2023-10-25 11:32:23 UTC
Created attachment 157780 [details] [review]
Bug 31503: (follow-up) Remove OPACCustomConsentTypes

This patch removes all trace of the original OPACCustomConsentTypes
preference.  We now use Koha::Plugins->feature_enabled in preference.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 127 Marcel de Rooy 2023-10-25 11:32:26 UTC
Created attachment 157781 [details] [review]
Bug 31503: (QA follow-up) Improve count of available_types

Tidied one other line.

Test plan:
Run t/db_dependent/Koha/Patron/Consents.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 128 Marcel de Rooy 2023-10-25 11:32:29 UTC
Created attachment 157782 [details] [review]
Bug 31503: (QA follow-up) Fix number of tests in rebase

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 129 Marcel de Rooy 2023-10-25 11:32:32 UTC
Created attachment 157783 [details] [review]
Bug 31503: DO NOT PUSH - Add example plugin

Test plan:
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->install"
Enable OPACCustomConsentTypes.
Test consenting and refusing consent on OPAC account page.
Run perl -MKoha::Plugin::Test_Newsletter -e"Koha::Plugin::Test_Newsletter->new->uninstall"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 130 Marcel de Rooy 2023-10-25 11:33:06 UTC
(In reply to Tomás Cohen Arazi from comment #117)
> Please rebase ASAP. Thanks!

Done.
Comment 131 Tomás Cohen Arazi 2023-10-25 13:36:12 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 132 Fridolin Somers 2023-10-26 07:10:04 UTC
Enhancement not pushed to 23.05.x