[wp-trac] [WordPress Trac] #44314: `user_confirmed_action_email_content` filter run on two different strings

[WordPress Trac]

#44314: `user_confirmed_action_email_content` filter run on two different strings
-------------------------------------------------+-------------------------
 Reporter:  desrosj                              |       Owner:  garrett-
                                                 |  eclipse
     Type:  defect (bug)                         |      Status:  accepted
 Priority:  normal                               |   Milestone:  5.8
Component:  Privacy                              |     Version:  4.9.6
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-dev-  |     Focuses:
  note early                                     |
-------------------------------------------------+-------------------------

Comment (by coffee2code):

 The latest [attachment:44314.cc.diff patch] by @peterwilsoncc overall
 looks good, except for the one point I brought up in my [comment:19
 previous comment] (which, admittedly, might not have been clear enough).

 Basically, DevHub can only know about one instance of a hook name.

 However, there are two instances of the
 `'user_confirmed_action_email_content'` filter. If you
 [https://developer.wordpress.org/reference/hooks/user_confirmed_action_email_content/
 check DevHub] currently, you may note that the docs for the hook were
 parsed from the version of the hook defined in
 `_wp_privacy_send_erasure_fulfillment_notification()`. That filter also
 appears in `_wp_privacy_send_request_confirmation_notification()`, but
 doesn't appear in DevHub even though it is documented differently.

 The aforementioned patch properly deprecates the duplicate hooks as if
 they were differently-named. But since you can't rely on which version of
 the hook docs will appear on DevHub for
 `'user_confirmed_action_email_content'`, the deprecation docs need to
 cover both cases.

 Also consider things from a developer's point of view: If someone had
 previously hooked `'user_confirmed_action_email_content'`, that callback
 will get invoked in either of 2 functions. WP can't accurately report to
 them whether they should be using
 `'user_erasure_fulfillment_email_content'` or
 `'user_request_confirmed_email_content'` because it doesn't know which was
 really intended. The error message should report both as possible
 alternatives to avoid recommending the wrong one.

 So there are two main aspects of the patch that need adjusting to convey
 the hook has two possible alternatives:

 == 1. Address how deprecation is conveyed via docs in DevHub

 Since only one version of the hook docs will be presented in DevHub, they
 both need to reference the possibility of two recommended alternatives.
 Or, one hook can get a `@ignore` added so that only the other instance of
 the hook needs to reference both hooks.

 The `@deprecated` notice should reference both alternatives:

 `@deprecated 5.7.0 Use {@see 'user_erasure_fulfillment_email_content'}
 instead. For user request confirmation email content use {@see
 'user_request_confirmed_email_content'} instead.`

 (This is for the instance of the hook in
 `_wp_privacy_send_erasure_fulfillment_notification()`. Something similar
 would need to be done in
 `_wp_privacy_send_request_confirmation_notification()`, unless that
 instance got an `@ignore` instead. But I think ignoring one would prevent
 it from appearing under the function's Used By section, so probably best
 not to do.)

 == 2. Address how deprecation is conveyed via error message

 Here is an example of how the aforementioned proposed patch formally
 deprecates an instance of the hook:

 `$content = apply_filters_deprecated(
 'user_confirmed_action_email_content', array( $content, $email_data ),
 '5.7.0', 'user_erasure_fulfillment_email_content' );`

 There are two possible approaches to clarifying the recommended
 alternatives:

 a. Overload the `$replacement` arg of the `apply_filters_deprecated()`
 call (the 4th arg) with both names, either as
 `"user_erasure_fulfillment_email_content|user_request_confirmed_email_content"`
 or `"user_erasure_fulfillment_email_content or
 user_request_confirmed_email_content"`[[br]][[br]]e.g. `$content =
 apply_filters_deprecated( 'user_confirmed_action_email_content', array(
 $content, $email_data ), '5.7.0', 'user_erasure_fulfillment_email_content
 or user_erasure_fulfillment_email_content' );`[[br]][[br]]Resulting error
 log message to dev:[[br]]`user_confirmed_action_email_content is
 <strong>deprecated</strong> since version 5.7.0! Use
 user_erasure_fulfillment_email_content or
 user_request_confirmed_email_content instead.`

 b. Add the other possible replacement as the `$message` arg of the
 `apply_filters_deprecated()` call (a new 5th arg):[[br]][[br]]e.g.
 `$content = apply_filters_deprecated(
 'user_confirmed_action_email_content', array( $content, $email_data ),
 '5.7.0', 'user_erasure_fulfillment_email_content', __( "If this was
 intended to hook within
 _wp_privacy_send_erasure_fulfillment_notification(), then use
 user_erasure_fulfillment_email_content instead." )
 );`[[br]][[br]]Resulting error log message to
 dev:[[br]]`user_confirmed_action_email_content is
 <strong>deprecated</strong> since version 5.7.0! Use
 user_erasure_fulfillment_email_content instead. If this was intended to
 hook within _wp_privacy_send_request_confirmation_notification(), then use
 user_request_confirmed_email_content instead.`


 -----


 Patch forthcoming that addresses 1 by having each deprecated instance of
 the hook refer to the other (without any use of `@ignore`) and addresses 2
 using the approach in 2a (using " or " instead of "|").

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