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

WordPress Trac noreply at wordpress.org
Wed Nov 7 21:51:17 UTC 2018


#44314: `user_confirmed_action_email_content` filter run on two different strings
--------------------------+-----------------------------
 Reporter:  desrosj       |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Future Release
Component:  Privacy       |     Version:  4.9.6
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+-----------------------------

Comment (by coffee2code):

 As far as the debate around either renaming one (or both) of the filters
 or adding an additional context argument goes, I believe the uses of the
 filters are different enough, and they are documented differently enough,
 that a filter should be renamed and the original version deprecated.

 The issue of filter documentation deserves some consideration in the
 matter. The convention (if not requirement) for hook names has been that a
 hook can be reused in multiple places if it is applied identically and can
 use the same documentation. The latter is important for the Code Reference
 because only one canonical documentation of a hook is expected; duplicate
 instances of the hook should be documented with a special comment
 referencing the canonical documentation.

 That can't happen for the instances of this hook due to:

 * The hooks' purposes ("Filters the body of the user request confirmation
 email." vs "Filters the body of the data erasure fulfillment
 notification.")
 * The unique placeholder strings supported by each context (only 2 of 6
 overall placeholders overlap)
 * The differing elements supplied in the second argument (`$email_data`)
 passed to hooking functions

 With so much different (the documentation, the arguments, and how the text
 is handled immediately after filtering), the hooks deserve to be uniquely
 named. Adding a context argument should be a consideration when only the
 context of the hook being run is different and all else being the same (or
 trivially so), not when the type of data passed along differs and the
 handling of the filtered text differs.

 Worth noting: If only one hook is being renamed (and the old name
 deprecated), the deprecated version (if retaining its docblock) should
 also have `@ignore` added to its docblock to ensure only 1 docblock for
 `user_confirmed_action_email_content` gets parsed. If both instances get
 deprecated, then only one needs the `@ignore`.

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


More information about the wp-trac mailing list