[wp-trac] [WordPress Trac] #44644: Personal data export request status changed to completed by admin download

[WordPress Trac]

#44644: Personal data export request status changed to completed by admin download
-------------------------------------------------+-------------------------
 Reporter:  knutsp                               |       Owner:  garrett-
                                                 |  eclipse
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.2
Component:  Privacy                              |     Version:  4.9.7
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-testing has-unit-    |     Focuses:
  tests                                          |  administration
-------------------------------------------------+-------------------------
Changes (by garrett-eclipse):

 * keywords:  needs-patch needs-refresh => has-patch needs-testing has-unit-
     tests
 * owner:  (none) => garrett-eclipse
 * status:  new => assigned
 * milestone:  Future Release => 5.2


Comment:

 Hello,

 Thank you for the initial report @knutsp and I greatly appreciate the
 discussion and feedback @JoshuaWold and @desrosj.

 In [https://core.trac.wordpress.org/attachment/ticket/44644/44644.diff
 44644.diff] I address the bug at hand ensuring admin downloads don't
 modify the request state. To accomplish this I moved the
 `_wp_privacy_completed_request` call into the `$send_as_email` check to
 ensure it's only triggered by the user via the email flow. I've also
 modified the associated comment to indicate the change.

 Testing this has so far worked nicely allowing me to download the export
 via admin multiple times throughout the process without affected the
 request state. Along with additional user testing we might want to ~~setup
 unit tests for the `wp_privacy_process_personal_data_export_page` function
 which check the `_wp_privacy_completed_request` call and change of request
 status is done when in `$send_as_email`.~~

 I've also created #46619 to branch off the enhancement request for
 introducing an admin action for 'Mark as Completed' either via bulk
 action, a button/link or both. @JoshuaWold I also answered your questions
 via that ticket. Happy to continue the conversation there.

 *After running `grunt test` I identified a unit testing issue which I've
 addressed in this patch. I struckthrough my above comment on unit tests
 because the below issue brought to light the pre-existing tests in
 `data_export_page_status_transitions`, and with my adjustment it provides
 the coverage of the two cases I was thinking would need unit tests (admin
 vs user as defined by `$send_as_email`.
 The error prior to my unit test adjustments;
 {{{
 1)
 Tests_Privacy_WpPrivacyProcessPersonalDataExportPage::test_request_status_transitions_correctly
 with data set #1 ('request-completed', 'last', 'last', 'last', false,
 'last')
 Failed asserting that two strings are identical.
 --- Expected
 +++ Actual
 @@ @@
 -'request-completed'
 +'request-pending'

 /WordPress/44644-admin-download-completes-
 request/tests/phpunit/tests/privacy/wpPrivacyProcessPersonalDataExportPage.php:597
 }}}

 Moving this into 5.2 for an easy win. Let me know if anyone runs into
 issues.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/44644#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform