Add missing unit tests for erasing personal data by username or email address

[desrosj]

This ticket picks up where #43546 left off to add missing tests for the functionality introduced and to keep when the enhancement and tests were committed clear.

See #43602.

[desrosj]

Marking as lower priority.

[audrasjb]

Hi,

As I said during today's bug scrub, I'll try to write the unit tests related to this ticket :)

Cheers,

Jb

[ocean90]

4.9.7 has been released, moving to next milestone.

[pbiron]

4.9.8 has hit RC, moving to 4.9.9.

[birgire]

@desrosj 44234.diff​ looks good.

44234-2.diff​ contains few suggestions:

• Adds test cases for the filters:
  • user_erasure_fulfillment_email_to,
  • user_erasure_complete_email_subject,
  • user_confirmed_action_email_content.

I only used the first input argument of those filter, so it's open for improvements to add the other arguments.

• Fixes the request type from 'earase_personal_data' to 'remove_personal_data'. It's defined as the latter in the WP_Privacy_Data_Removal_Requests_Table class. But in general I would have preferred the 'earase_personal_data' name of that action type :-)
• Adds :: for the global function in the @covers annotation for _wp_privacy_send_erasure_fulfillment_notification.
• Uses "send an email" instead of "send emails" in inline method descriptions, to make it clear that it only sends a single email.
• The unit tests are a useful documentation, so what about changing the order of methods; show first the methods that explain how it should work, then after that the methods for failing/error? This is applied in the patch.
• Adds inline comments within methods:
  • test_should_send_email_only_once(),
  • test_should_not_send_email_when_request_not_completed(),
  • test_should_not_send_email_when_not_wp_user_request().
• Adjusted the description of the test_should_not_send_email_when_not_wp_user_request() method.
• Changes 'erase-my-data@local.dev' to 'erase-my-data@local.test' as the .dev domain is no longer suitable for local.
• The $mailer->get_sent()->to is:

  [0] => Array
     (
  		[0] => erase-my-data@local.test
  		[1] =>
  	)
  )
  

so it seems like $mailer->get_sent()->to[0][0] should be used instead of $mailer->get_sent()->to[0].
I used $mailer->get_recipient( 'to' )->address instead, as it is a wrapper for $mailer->get_sent()->to.

• Moves

  wp_update_post(
  	array(
  		'ID'          => self::$request_id,
  		'post_status' => 'request-completed',
  	)
  );
  

into the wpSetUpBeforeClass() method to reduce code duplications, as I was also starting to use it in the new methods.

[desrosj]

I only used the first input argument of those filter, so it's open for improvements to add the other arguments.

I think tests for filters should just confirm that a filtered value effects the outcome of the function, so this should be sufficient.

The unit tests are a useful documentation, so what about changing the order of methods; show first the methods that explain how it should work, then after that the methods for failing/error?

I think this is just preference, but fine with your changes. I usually organize my tests in a top down manner, testing the scenarios from the first line of the function to the last.

All your changes look good, though. I think this one is ready.

[audrasjb]

5.0.3 is going to be released in few days. We are currently sorting the remaining tickets in the milestone. The ticket is sorted in milestone 5.0.3, but requires a @since refresh and a commit/backport to the associated branch.

[desrosj]

Let's punt this to 5.1. It's not necessary for 5.0.3.

[desrosj]

Refreshed patch with 5.1.0.

[desrosj]

​https://travis-ci.org/desrosj/wordpress-develop/builds/477630506 is the refreshed patch passing on Travis.

[desrosj]

In 44535:

Privacy: Add unit tests for privacy erasure fulfillment notifications.

Adds appropriate unit tests for _wp_privacy_send_erasure_fulfillment_notification().

Props birgire, desrosj.
Fixes #44234.