See 18081
Created attachment 75765 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 75766 [details] [review] Bug 20819: DBIx schema changes No test plan.
Created attachment 75767 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t
More patches forthcoming tomorrow or so.
Created attachment 75818 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 75819 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 75820 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76781 [details] [review] Bug 20819: DBIx schema changes No test plan.
Created attachment 76782 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76783 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76784 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76785 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Trivial rebase
Do we need to keep a copy of the template in use during the consent in the database along with the borrower ID and date? The UK ICO page on consenting seems to say we do: https://ico.org.uk/for-organisations/guide-to-the-general-data-protection-regulation-gdpr/consent/how-should-we-obtain-record-and-manage-consent/ (second example).
(In reply to Jon Knight from comment #14) > Do we need to keep a copy of the template in use during the consent in the > database along with the borrower ID and date? The UK ICO page on consenting > seems to say we do: > > https://ico.org.uk/for-organisations/guide-to-the-general-data-protection- > regulation-gdpr/consent/how-should-we-obtain-record-and-manage-consent/ > > (second example). Thx for your interest in this patch. The link says: === If consent was given online, your records should include the data submitted as well as a timestamp to link it to the relevant version of the data capture form. You keep records that include an ID and the data submitted online together with a timestamp. You also keep a copy of the version of the data-capture form and any other relevant documents in use at that date. === This patch registers the borrower ID and the timestamp of consent given. What each library should keep somehow (not described), is: a version history of the template, esp. the exact text for the consent and a history of the referenced privacy page text. In that way you can 'prove' that the user gave consent for a specific version of template and privacy page. (Note: the privacy page is only specified in this patch by a preference called PrivacyPolicyURL. The library itself should create that local page.) I am no legal expert, but I can't imagine that we need to save these texts at an individual consent level. Hope this is clear enough?
I was focusing on the example box with the tick (in other words the right, approved way the ICO expect this to be handled) that says: "You keep records that include an ID and the data submitted online together with a timestamp. You also keep a copy of the version of the data-capture form and any other relevant documents in use at that date." So to me that does imply the need to save these texts at an individual consent level, as they all form part of the consent package for that user. Indeed the "crossed" part (wrong) of that example says, "You keep the time and date of consent linked to an IP address, with a web link to your current data-capture form and privacy policy" so they seem to think that just knowing what the PrivacyPolicyURL was set to at the time of consent is not enough.
I was looking at the same text as you. But I concluded that we need to save the consent tekst somewhere in a version history not on consent level (that would be unneeded redundancy). But not implying that saving a URL would be enough.
Probably changes on Member..
Created attachment 77264 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 77265 [details] [review] Bug 20819: DBIx schema changes No test plan.
Created attachment 77266 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 77267 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 77268 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 77269 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased. Especially last patch had conflicts due to AddMember changes.
Created attachment 78369 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78370 [details] [review] Bug 20819: DBIx schema changes No test plan.
Created attachment 78371 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78372 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78373 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78374 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Needed a rebase for removed NorwegianDB prefs.
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78779 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78781 [details] [review] Bug 20819: DBIx schema changes No test plan. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78783 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78784 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78786 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 78788 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
(In reply to sandboxes@biblibre.com from comment #39) > Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Thanks Séverine!
Created attachment 79059 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 79062 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79063 [details] [review] Bug 20819: DBIx schema changes No test plan. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79064 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79065 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79066 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79067 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Josef Moravec from comment #47) > Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Thanks Josef
Created attachment 79129 [details] [review] Bug 20819: Dbrev for new table patron_consent and sysprefs [1] We add table patron_consent. It allows for future extension. We could e.g. save consent for newsletter etc. [2] Add one field to borrower_modifications Datetime the patron who self-registered, gave consent. [3] Syspref PrivacyPolicyURL [4] Syspref GDPR_Policy: enforced, permissive or disabled. Test plan: [1] Run the dbrev or do a new install. [2] Look at the description of the new prefs in Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79130 [details] [review] Bug 20819: DBIx schema changes No test plan. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79131 [details] [review] Bug 20819: Add Koha object classes for patron consents Introduces Koha::Patron::Consent[s] for new table. Adds basic CRUD test. Test plan: Run t/db_dependent/Koha/Patron/Consents.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79132 [details] [review] Bug 20819: Add your consents tab to opac-user This patch adds a consents tab to the OPAC user account menu. We now add a GDPR section here, but it is open for future extensions. Think of a newsletter checkbox for instance. Script opac-patron-consent handles the tab. And now only includes some GDPR code but is also written for more general use too. Test plan: [1] Set GDPR_Policy pref to Disabled. Verify that OPAC operates as usual. [2] Set pref to Permissive. Try to save a consent or a refusal. Note that you are not logged out when saving a refusal. [3] Set pref to Enforced. Save a refusal. You should be logged out. Log in again and verify that the consents tab shows a No. Note: a follow-up patch will add further enforcements. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79133 [details] [review] Bug 20819: Add check in get_template_and_user to enforce GDPR If you choose to enforce GDPR policy, a user needs to give consent for data processing before he does something else in the OPAC while being logged in. Test plan: [1] Set GDPR_Policy to Disabled or Permissive. Usual behavior. [2] Set to Enforced. Save a refusal on your consents. Notice that you are logged out when saving. When you login again, all OPAC requests are redirected to your consents tab. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79134 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Resolved a checksum problem in patch 2 and a git problem for patch 6.
Created attachment 79135 [details] [review] Bug 20819: Add consent to self-registration process We add a section for the GDPR consent in opac-memberentry (only for the self-registration). Not when editing personal details. Test plan: [1] Enable selfregistration (with confirm) and GDPR policy. [2] Register a new account in OPAC. Verify that the GDPR checkbox is required. [3] After you submit, you should see a date in borrower_modifications field gdpr_proc_consent. [4] When you confirm, verify that the consent is visible on your consents. [5] Enable selfregistration without confirmation mail. Register again. [6] Check your consents tab again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Marcel de Rooy from comment #55) > Resolved a checksum problem in patch 2 and a git problem for patch 6. Patch 6 had no problem; forgot to apply dependency on 21374
Created attachment 79175 [details] [review] Bug 20819: (RM follow-up) Add missing filters
Awesome work all! Pushed to master for 18.11
I pushed this, the UI display for when a user has denied consent could be improved - it still seems to be waiting for approval, but doesn't make it clear that 'No' has been set - as others seemed fine with it I let it go but something to consider
(In reply to Nick Clemens from comment #58) > Created attachment 79175 [details] [review] [review] > Bug 20819: (RM follow-up) Add missing filters Thx
Created attachment 79190 [details] [review] Bug 20819: (Follow-up) Show more clearly that a user gave no consent See comment60 on Bugzilla. Although I intentionally did not want to show date and time of refusal, we can obviously add one line saying that we already have a no. Test plan: Login without consent and see no additional line. Agree and verify that you see the registered on xxx line. Disagree and verify that you now see "You indicated recently...". Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Nick Clemens from comment #60) > I pushed this, the UI display for when a user has denied consent could be > improved - it still seems to be waiting for approval, but doesn't make it > clear that 'No' has been set - as others seemed fine with it I let it go but > something to consider See attachment. Your consent please :)
(In reply to Marcel de Rooy from comment #62) > Created attachment 79190 [details] [review] [review] > Bug 20819: (Follow-up) Show more clearly that a user gave no consent > > See comment60 on Bugzilla. > Although I intentionally did not want to show date and time of refusal, > we can obviously add one line saying that we already have a no. > > Test plan: > Login without consent and see no additional line. > Agree and verify that you see the registered on xxx line. > Disagree and verify that you now see "You indicated recently...". > > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Followup pushed to master for 18.11, thank you!
Enhancement, will not be backported to 18.05.x series.
+ my $consent = Koha::Patron::Consents->search({ + borrowernumber => getborrowernumber($user), + type => 'GDPR_PROCESSING', + given_on => { '!=', undef }, + })->next; + if( !$consent ) { ->count could have been better.
(In reply to Jonathan Druart from comment #66) > + my $consent = Koha::Patron::Consents->search({ > + borrowernumber => getborrowernumber($user), > + type => 'GDPR_PROCESSING', > + given_on => { '!=', undef }, > + })->next; > + if( !$consent ) { > > > ->count could have been better. I tested that assumption a bit. And it seems not to be true in terms of performance at least? In short: 1.3480 sec for 1000 counts and 0.0094 for 1000 next's (on 1000 records). It seems that DBIx goes to the db for each count but the repeated nexts are handled much more efficiently (cache, prefetch, ..)? Script fragment on next comment.
my $set = Koha::Patron::Consents->search({ borrowernumber => $firstid }); #my $set = Koha::Patron::Consents->search; my $t1 = time; foreach(1..100) { my $i = $set->count; } my $t2 = time; is(1, 1, sprintf( "COUNT %6.4f\n", $t2 - $t1 ) ); $t1 = time; foreach(1..100) { my $i = $set->next; } #foreach(1..1000) { my $i = $set->next; #$set->_resultset->first unless $i; } $t2 = time; is(1, 1, sprintf( "NEXT %6.4f\n", $t2 - $t1 ) );
use Modern::Perl; use Data::Dumper qw/Dumper/; use Test::More tests => 1; use Time::HiRes qw/time/; use t::lib::Mocks; use t::lib::TestBuilder; use Koha::Patron::Consents; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; $schema->storage->txn_begin; subtest 'myTest' => sub { plan tests => 2; # Add 1000 consents my $firstid; foreach(1..100) { my $consent = $builder->build_object({ class => 'Koha::Patron::Consents' }); $firstid = $consent->borrowernumber if !$firstid; } my $set = Koha::Patron::Consents->search({ borrowernumber => $firstid }); #my $set = Koha::Patron::Consents->search; my $t1 = time; foreach(1..100) { my $i = $set->count; } my $t2 = time; is(1, 1, sprintf( "COUNT %6.4f\n", $t2 - $t1 ) ); $t1 = time; foreach(1..100) { my $i = $set->next; } #foreach(1..1000) { my $i = $set->next; #$set->_resultset->first unless $i; } $t2 = time; is(1, 1, sprintf( "NEXT %6.4f\n", $t2 - $t1 ) ); }; $schema->storage->txn_rollback;
Marcel, use DBIC_TRACE=1 to understand what's going on. You are comparing 100 queries vs 1 query: 100 "SELECT COUNT" vs 1 "SELECT *" The ->next will return something only on first pass.
Agreed, that test isn't testing what you think Marcel. The ->next is chained onto the resultset inside the loop in C4::Auth whereas the resultset is created outside of the loop in your test. I actually don't know which is faster, but I would imagine count is a bit faster and more memory efficient. Try moving the queries inside the loop and test again, I'd certainly be interested in the results. (a correlate subquery version might be interesting too)
Ignore the subquery bit.. that doesn't apply here (though I'd still be interested in comparing that at some point)
(In reply to Jonathan Druart from comment #70) > Marcel, use DBIC_TRACE=1 to understand what's going on. > > You are comparing 100 queries vs 1 query: 100 "SELECT COUNT" vs 1 "SELECT *" > > The ->next will return something only on first pass. Yeah I have been toggling a bit with comment chars. But I also tested the next on the set of 100 records.
# Subtest: myTest 1..2 ok 1 - COUNT 0.1278 # ok 2 - NEXT 0.0106 # 18021068316832683368346835683668376838683968406841684268436844684568466847684868496850685168526853685468556856685768586859686068616862686368646865686668676868686968706871687268736874687568766877687868796880688168826883688468856886688768886889689068916892689368946895689668976898689969006901690269036904690569066907690869096910691169126913691469156916691769186919692069216922692369246925692669276928 with a slight variation
use Modern::Perl; use Data::Dumper qw/Dumper/; use Test::More tests => 1; use Time::HiRes qw/time/; use t::lib::Mocks; use t::lib::TestBuilder; use Koha::Patron::Consents; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; $schema->storage->txn_begin; subtest 'myTest' => sub { plan tests => 2; # Add 1000 consents my $firstid; foreach(1..100) { my $consent = $builder->build_object({ class => 'Koha::Patron::Consents' }); $firstid = $consent->borrowernumber if !$firstid; } #my $set = Koha::Patron::Consents->search({ borrowernumber => $firstid }); my $set = Koha::Patron::Consents->search; my $t1 = time; foreach(1..100) { my $i = $set->count; } my $t2 = time; is(1, 1, sprintf( "COUNT %6.4f\n", $t2 - $t1 ) ); my $str = ''; $t1 = time; foreach(1..100) { my $i = $set->next; $str.= $i->id; } #foreach(1..1000) { my $i = $set->next; #$set->_resultset->first unless $i; } $t2 = time; is(1, 1, sprintf( "NEXT %6.4f\n", $t2 - $t1 ) ); print "$str\n"; }; $schema->storage->txn_rollback;
(In reply to Martin Renvoize from comment #71) > The ->next is chained onto the resultset inside the loop in C4::Auth whereas > the resultset is created outside of the loop in your test. Yeah deliberately. > I actually don't know which is faster, but I would imagine count is a bit > faster and more memory efficient. Thats the problem. I would expect something else.
On 1000 records: ok 1 - COUNT 1.3365 ok 2 - NEXT 0.0950 The printed string shows all 1000 primary keys too. What am I missing here?
(In reply to Marcel de Rooy from comment #76) > (In reply to Martin Renvoize from comment #71) > > The ->next is chained onto the resultset inside the loop in C4::Auth whereas > > the resultset is created outside of the loop in your test. > Yeah deliberately. OK, I'm confused then.. what are you trying to prove here then? > > > I actually don't know which is faster, but I would imagine count is a bit > > faster and more memory efficient. > Thats the problem. I would expect something else. When doing it inside the loop as per my suggestion what you're actually doing is SELECT COUNT(*) FROM whatever WHERE whatever; Cache this answer and print it 100 times vs SELECT * FROM whatever WHERE whatever; Do this 100 times, hitting the database for each call of 'Next' to get the next result given an offset from the database. (Unless you call prefetch which says 'get me all rows in one go and cache them to iterate in memory)
From IRC: See https://metacpan.org/source/RI[…]esultSet.pm#L1594 count is cached at the dbic level here which is why recursing count on the same resultset should be very very fast. https://metacpan.org/source/RI[…]esultSet.pm#L1229 next gets really clever ;) there's loads of optimizatino in there regarding caching rows see _construct_results It seems that count is not cached in my test (so recursing not fast) and next is cached. I am not triggered right now to replace the next in Auth.pm. And for one next we are talking about milliseconds.
Why do we have the following line: opac/opac-memberentry.pl: push @fields, 'gdpr_proc_consent' if C4::Context->preference('GDPR_Policy'); It causes bug 23150.
(In reply to Jonathan Druart from comment #80) > Why do we have the following line: > > opac/opac-memberentry.pl: push @fields, 'gdpr_proc_consent' if > C4::Context->preference('GDPR_Policy'); > > It causes bug 23150. See bug 23150. No bug, but design. The field should be handled as mandatory. If you do not want to give consent, you cannot register an account.
(In reply to Marcel de Rooy from comment #81) > (In reply to Jonathan Druart from comment #80) > > Why do we have the following line: > > > > opac/opac-memberentry.pl: push @fields, 'gdpr_proc_consent' if > > C4::Context->preference('GDPR_Policy'); > > > > It causes bug 23150. > > See bug 23150. No bug, but design. > The field should be handled as mandatory. If you do not want to give > consent, you cannot register an account. Not completely true: see 23150