Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve File download Log accuracy and remove PII from log meta #6548

Closed
cklosowski opened this issue May 8, 2018 · 3 comments
Closed

Improve File download Log accuracy and remove PII from log meta #6548

cklosowski opened this issue May 8, 2018 · 3 comments
Assignees
Milestone

Comments

@cklosowski
Copy link
Contributor

Enhancement Request

Explain your enhancement (please be detailed)

Currently the file download log uses a meta key of _edd_log_user_info to store information like
a:3:{s:5:"email";s:15:"admin@local.dev";s:2:"id";i:1;s:4:"name";s:15:"Chris Klosowski";}. This information is pulled from the customer associated with the purchase record.

While used to do some download count checks, we can be far more accurate, as well as remove PII from these log entry meta values by simply storing the customer ID. Since a customer can have more than one email address, and also set their primary email address from their profile editor, this could end up being in accurate when we do a download count using this function:
edd_count_file_downloads_of_user( $customer_obj->email );

My proposal will be to do the following:

  1. Remove the _edd_log_user_info for file download logs only, with a migration routine.
  2. Add in a new meta entry for file download logs of _edd_log_customer_id, with the above migration routine.
  3. Add a new function of edd_count_file_downloads_of_customer which accepts a customer Id or email (since the customer object can find a customer with any email address associated with the customer)
  4. Change any calls to get this count to the new method.

Justification or use case

I realized this was possibly an issue while researching the implementation of #6499. This data isn't necessary and can be looked up elsewhere.

@SeanTOSCD
Copy link
Contributor

Ran the migration with about 25,000 logs and everything looks to have updated correctly. If there's some real-world functionality you want me to test, just let me know. Otherwise, this looks neato to me. 👍

@pippinsplugins
Copy link
Contributor

@SDavisMedia could you please test on the EDD staging site?

@cklosowski cklosowski added this to the 2.9.2 milestone May 14, 2018
@cklosowski
Copy link
Contributor Author

@pippinsplugins yeah I'll get it on the staging site prior to merging

cklosowski added a commit that referenced this issue May 15, 2018
)

* Add upgrade routine to convert to customer ID, and update calls for download count #6548

* Adding new file #6548

* Update edd_record_download_in_log to insert data in new format #6548

* Make direct DB Queries for data to avoid any slow queries during migration #6548

* Use abbr tag around PII #6548

* Increase to 50 per step and continue of a customer ID is already defined #6548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants