@geekdenz opened this Pull Request on September 4th 2021 Contributor

fixes #16529

Description:

Reuse logic from DataSubject and refactor into core.

To test this manually, please see https://github.com/matomo-org/test-examples/blob/trackingformlogs/logdeleter/general/index.php . You can configure it with https://github.com/matomo-org/test-examples/blob/trackingformlogs/config/config.example.php .

Review

@geekdenz commented on September 5th 2021 Contributor

You'll need to apply this patch, otherwise it won't get into the code.

See https://github.com/matomo-org/test-examples/blob/trackingformlogs/logdeleter/general/index.php .

diff --git a/plugins/PrivacyManager/PrivacyManager.php b/plugins/PrivacyManager/PrivacyManager.php
index f11ff26b43..98055c9150 100644
--- a/plugins/PrivacyManager/PrivacyManager.php
+++ b/plugins/PrivacyManager/PrivacyManager.php
@@ -672,6 +672,7 @@ class PrivacyManager extends Plugin
         $deleteIntervalDays = $settings[$setting];
         $deleteIntervalSeconds = $this->getDeleteIntervalInSeconds($deleteIntervalDays);

+        return true;
         if ($lastDelete === false ||
             $lastDelete === '' ||
             ((int)$lastDelete + $deleteIntervalSeconds) <= time()
@sgiehl commented on September 7th 2021 Member

@geekdenz Wouldn't it make sense to add/change some tests in LogDeleterTest to prove that the new logic works as expected. Ideally the new/changed tests would fail on 4.x-dev but work with the changes of this PR.

@tsteur commented on September 7th 2021 Member

@sgiehl it's not really needed since the logic we use itself is already tested.

@geekdenz commented on September 8th 2021 Contributor

idvisit IN(1,2,3,4,4)

will be more efficient, though just by a constant factor (strlen('idvisit = 1 OR ') - strlen('1,')), even just because the text is shorter. The query optimiser might optimise this automatically though, but it adds another step for it. It'll add up with lots of IDs. I'll do it as it is not difficult.

@geekdenz commented on September 9th 2021 Contributor

OK. Double checked manually and got expected results:

mysql> SELECT distinct b.idvisit, c.visit_last_action_time, a.idlogform, a.idformview, a.field_name FROM matomo_log_form_field as a join matomo_log_form as b on a.idlogform=b.idlogform JOIN matomo_log_visit AS c ON c.idvisit=b.idvisit;
+---------+------------------------+-----------+------------+------------+
| idvisit | visit_last_action_time | idlogform | idformview | field_name |
+---------+------------------------+-----------+------------+------------+
|   10444 | 2020-08-01 00:00:00    |        24 | zaODNJ     | input1     |
|   10444 | 2020-08-01 00:00:00    |        24 | zaODNJ     | input2     |
|   10444 | 2020-08-01 00:00:00    |        24 | zaODNJ     | input3     |
|   10449 | 2020-08-01 00:00:00    |        28 | 9GlRNX     | input1     |
|   10449 | 2020-08-01 00:00:00    |        28 | 9GlRNX     | input2     |
|   10449 | 2020-08-01 00:00:00    |        28 | 9GlRNX     | input3     |
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input1     |
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input2     |
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input3     |
+---------+------------------------+-----------+------------+------------+
9 rows in set (0.01 sec)

mysql> SELECT distinct b.idvisit, c.visit_last_action_time, a.idlogform, a.idformview, a.field_name FROM matomo_log_form_field as a join matomo_log_form as b on a.idlogform=b.idlogform JOIN matomo_log_visit AS c ON c.idvisit=b.idvisit;
+---------+------------------------+-----------+------------+------------+
| idvisit | visit_last_action_time | idlogform | idformview | field_name |
+---------+------------------------+-----------+------------+------------+
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input1     |
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input2     |
|   10443 | 2021-09-07 03:05:54    |        23 | ZDa9lV     | input3     |
+---------+------------------------+-----------+------------+------------+
3 rows in set (0.01 sec)
@tsteur commented on September 13th 2021 Member

Fyi tweaked the test cases to make sure it works fine when idvisit and idvisit+idsite is mixed and also to know it works when multiple deletions are requested. Was the easiest way for me to test the generated queries work.

Screenshot 2021-09-13 at 3 04 19 PMScreenshot 2021-09-13 at 3 20 57 PM
@tsteur commented on September 13th 2021 Member

Generally tested for quite a while and looks all good to me. Waiting for automated tests to finish before merging 🎉

@geekdenz commented on September 13th 2021 Contributor

Thanks @tsteur for the added test and info!

This Pull Request was closed on September 13th 2021
Powered by GitHub Issue Mirror