Adds message about data retention on gdpr overview #12878

sgiehl · 2018-05-09T21:17:21Z

tsteur · 2018-05-09T21:36:02Z

FYI: We still have to mention the data retention period they should enter in a privacy notice, consent notice, or privacy policy.

This would be usually whatever is higher - aggregated reports or raw data retention rate. If the delete reports delete all reports except the numeric reports, then we only show the raw/log data retention rate to be used for this. That's because aggregated reports still contain personal information and therefore need to be included in the data retention rate.

mattab · 2018-05-10T02:10:33Z

Maybe we should add a sentence below the bullet points "Data retention policy can be set by a Super User." with the link only there for Super Users.
(IIRC the page "Anonymise data" is not visible to admin users)

@tsteur I would only mention both and let users pick the one they need depending on their needs and requirements. we can't really go into more details for them, because they may not have PII (user id, custom dim) in their aggregate reports and then would only need to mention the raw data retention.

tsteur · 2018-05-10T02:30:15Z

@tsteur I would only mention both and let users pick the one they need depending on their needs and requirements. we can't really go into more details for them, because they may not have PII (user id, custom dim) in their aggregate reports and then would only need to mention the raw data retention.

the thing be to at least educate them on helping to make a decision. Nobody can realistically know which one to pick so we need to help our users.

sgiehl · 2018-05-10T09:53:16Z

I'm fine with changing it again, just let me know what exactly to change/add.

tsteur · 2018-05-10T22:28:16Z

We would only need to add a message. Like "The overall data retention rate for your privacy policy is the raw data retention rate. Please note that aggregated reports may contain personal data as well. If you are using features like User ID, Custom Variables, Custom Dimension, or track personal data through other events such as events, page URLs or page titles, etc, then the overall data retention rate is the data retention rate that is higher. "

In reality, people sometimes track different websites and for some pages, they may have personal data in aggregated reports and for some not.

btw: I don't know if you use "Log data" somewhere, but lets maybe always use raw data instead as this is better understandable for people that don't know our tables etc are called "log" and also in general?

sgiehl · 2018-05-13T18:28:53Z

we use log data in some other translations in PrivacyManager. Should we replace it with raw data at all occurrences?

tsteur · 2018-05-13T19:06:53Z

Makes sense I would say @mattab ?

mattab · 2018-05-13T19:43:19Z

Sounds good to replace wording by raw data 👍

diosmosis · 2018-05-18T01:36:35Z

plugins/PrivacyManager/templates/gdprOverview.twig

+        </ul>
+        <p>
+            <br />
+            The overall data retention rate for your privacy policy is the raw data retention rate. Please note that aggregated reports may contain personal data as well. If you are using features like User ID, Custom Variables, Custom Dimension, or track personal data through other events such as events, page URLs or page titles, etc, then the overall data retention rate is the data retention rate that is higher.


Think it would be clearer here to write "the higher of the two" instead of "the data retention rate that is higher".

Another suggestion: "through other events" => "in other ways"

diosmosis · 2018-05-18T01:43:01Z

plugins/PrivacyManager/templates/gdprOverview.twig

+            {% if deleteLogsEnable %}
+                <li>all aggregated reports are deleted after <strong>{{ reportRetention }}</strong>.</li>
+            {% else %}
+                <li>aggregated report are <strong>never</strong> deleted.</li>


Typo, should be aggregated reports

diosmosis · 2018-05-18T01:43:37Z

plugins/PrivacyManager/templates/gdprOverview.twig

+            {% if deleteReportsEnable %}
+                <li>all visitors and actions raw data are deleted after <strong>{{ rawDataRetention }}</strong>.</li>
+            {% else %}
+                <li>visitors and actions raw data are <strong>never</strong> deleted.</li>


Think it should be "visit and action raw data", since you can't technically delete the visitor themselves :P

diosmosis · 2018-05-18T01:47:27Z

plugins/PrivacyManager/Controller.php

+        }
+        if ($purgeDataSettings['delete_reports_older_than'] % 30 > 0) {
+            $rawDataRetention .= floor($purgeDataSettings['delete_reports_older_than']%30) . ' ' . Piwik::translate('Intl_PeriodDays');
+        }


Not sure how beneficial it is, but it would look a bit better to use the singular translation if the year/day/month is 1. (ie, 1 year 1 month instead of 1 years 1 months)

diosmosis · 2018-05-18T01:53:50Z

Not sure why but I'm getting weird results when I test this. If I set the "Delete old visitor logs" entry to 180 days & don't delete report data:

It says it will never delete raw data & will delete report data if > 1 year 5 months:

I think for the aggregated data section at least, it's not checking if the report deletion feature is enabled?

diosmosis · 2018-05-18T01:57:04Z

If both features are enabled:

then the gdpr overview says it will delete raw data, but it gets the number wrong (13 days instead of 182, I think it's using the wrong setting):

sgiehl · 2018-05-20T14:16:48Z

@diosmosis I've updated the PR with your suggestions and fixed some stuff.

diosmosis

Good to go 👍 . Not sure if @tsteur or @mattab has to do a final approval since it's GDPR related, so just approving.

diosmosis · 2018-05-21T00:00:20Z

Oh forgot about the UI tests, they will need to be updated before merging.

mattab · 2018-05-21T08:35:07Z

Feedback

Could you please add a UI test which shows both bullet point values?

Maybe @tsteur you have some more feedback?

sgiehl · 2018-05-21T12:35:14Z

@mattab I've added a UI test. See https://builds-artifacts.matomo.org/matomo-org/matomo/gdprretention/27727/PrivacyManager_gdpr_overview_no_retention.png and https://builds-artifacts.matomo.org/matomo-org/matomo/gdprretention/27727/PrivacyManager_gdpr_overview.png

tsteur · 2018-05-21T19:21:39Z

I would change "then the overall data retention rate" => " then the overall data retention rate for your privacy policy ".

For some reason I would have personally listed the raw data retention before the aggregated report but that is totally fine either way.

I don't know if we already have an FAQ, but if so it would be great to link raw data and aggregated data to an FAQ entry so it is clear what they mean. All good though if we don't have them.

sgiehl · 2018-05-21T19:30:01Z

changed the order and updated the text.
Regarding FAQ: I didn't find some matching entries, but maybe @mattab knows if there are any.

diosmosis · 2018-05-21T19:35:42Z

plugins/PrivacyManager/templates/gdprOverview.twig

            {% if deleteReportsEnable %}
                <li>all visits and actions raw data are deleted after <strong>{{ rawDataRetention }}</strong>.</li>
            {% else %}
                <li>visits and actions raw data are <strong>never</strong> deleted.</li>
            {% endif %}
+            {% if deleteLogsEnable %}


I just noticed, but are these variables named correctly? The deleteReportsEnable block talks about raw data & the deleteLogsEnable block talks about aggregated reports.

seems I somehow screwed it up 🙈

* Adds message about data retention on gdpr overview * Adds additional information about gdpr relevant data retention * replace log data with raw data * review adjustments * adds ui test * review adjustments * update UI files

sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels May 9, 2018

sgiehl added this to the 3.5.1 milestone May 9, 2018

sgiehl force-pushed the gdprretention branch from 6fbd9e3 to 62f1d12 Compare May 13, 2018 18:31

diosmosis reviewed May 18, 2018

View reviewed changes

sgiehl force-pushed the gdprretention branch from ce29878 to f5f715c Compare May 20, 2018 13:59

diosmosis approved these changes May 20, 2018

View reviewed changes

sgiehl added 4 commits May 21, 2018 12:01

Adds message about data retention on gdpr overview

00e37be

Adds additional information about gdpr relevant data retention

549098f

replace log data with raw data

2865ddd

review adjustments

65e02fa

sgiehl force-pushed the gdprretention branch from f5f715c to 29ced15 Compare May 21, 2018 10:02

adds ui test

797fb26

sgiehl force-pushed the gdprretention branch from 29ced15 to 797fb26 Compare May 21, 2018 12:34

diosmosis reviewed May 21, 2018

View reviewed changes

review adjustments

49a79b5

sgiehl force-pushed the gdprretention branch from 1efa12f to 49a79b5 Compare May 21, 2018 19:57

update UI files

17d0015

diosmosis merged commit 05d5b30 into 3.x-dev May 21, 2018

diosmosis deleted the gdprretention branch May 21, 2018 23:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds message about data retention on gdpr overview #12878

Adds message about data retention on gdpr overview #12878

sgiehl commented May 9, 2018

tsteur commented May 9, 2018

mattab commented May 10, 2018

tsteur commented May 10, 2018

sgiehl commented May 10, 2018

tsteur commented May 10, 2018

sgiehl commented May 13, 2018

tsteur commented May 13, 2018

mattab commented May 13, 2018

diosmosis May 18, 2018

diosmosis May 18, 2018

diosmosis May 18, 2018

diosmosis May 18, 2018

diosmosis May 18, 2018

diosmosis commented May 18, 2018

diosmosis commented May 18, 2018

sgiehl commented May 20, 2018

diosmosis left a comment

diosmosis commented May 21, 2018

mattab commented May 21, 2018

sgiehl commented May 21, 2018

tsteur commented May 21, 2018

sgiehl commented May 21, 2018

diosmosis May 21, 2018

sgiehl May 21, 2018

Adds message about data retention on gdpr overview #12878

Adds message about data retention on gdpr overview #12878

Conversation

sgiehl commented May 9, 2018

tsteur commented May 9, 2018

mattab commented May 10, 2018

tsteur commented May 10, 2018

sgiehl commented May 10, 2018

tsteur commented May 10, 2018

sgiehl commented May 13, 2018

tsteur commented May 13, 2018

mattab commented May 13, 2018

diosmosis May 18, 2018

Choose a reason for hiding this comment

diosmosis May 18, 2018

Choose a reason for hiding this comment

diosmosis May 18, 2018

Choose a reason for hiding this comment

diosmosis May 18, 2018

Choose a reason for hiding this comment

diosmosis May 18, 2018

Choose a reason for hiding this comment

diosmosis commented May 18, 2018

diosmosis commented May 18, 2018

sgiehl commented May 20, 2018

diosmosis left a comment

Choose a reason for hiding this comment

diosmosis commented May 21, 2018

mattab commented May 21, 2018

sgiehl commented May 21, 2018

tsteur commented May 21, 2018

sgiehl commented May 21, 2018

diosmosis May 21, 2018

Choose a reason for hiding this comment

sgiehl May 21, 2018

Choose a reason for hiding this comment