[wp-trac] [WordPress Trac] #43985: Privacy: The user request email should be sent in the user language

WordPress Trac noreply at wordpress.org
Wed Aug 8 19:50:04 UTC 2018


#43985: Privacy: The user request email should be sent in the user language
-------------------------------------+-------------------------------------
 Reporter:  Chouby                   |       Owner:  desrosj
     Type:  defect (bug)             |      Status:  accepted
 Priority:  normal                   |   Milestone:  4.9.9
Component:  I18N                     |     Version:  4.9.6
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:  administration,
  has-unit-tests                     |  privacy
-------------------------------------+-------------------------------------

Comment (by desrosj):

 Replying to [comment:43 birgire]:

 Thanks for the thorough review!

 > - There are currently no {{{restore_previous_locale()}}}. Was that
 intentionally?

 I had been experimenting with making the tests faster. Removing that had
 seemed to make the tests run a bit faster. I added
 `restore_previous_locale()` to prevent the class from adversely affecting
 another one down the road.

 > - There is some discrepancy between test method descriptions and test
 method function names

 I fixed the discrepancies and updated some of the inline documentation. I
 was mixing up too many defaults. There is the core default language
 (en_US), and the site default language, which is selected on the user
 screen and switches the user to whichever language is selected for the
 site. They should match and be more clear now.

 > - There are two {{{wp_create_user_request()}}} and two
 {{{wp_set_current_user()}}} in
 {{{test_should_send_user_request_email_in_user_locale_when_admin_has_different_locale_than_site()}}}.

 Fixed! Copy paste error. This function's new name is
 `test_should_send_user_request_email_in_user_locale_when_admin_and_site_have_different_locales()`

 > - Are the global {{{$taxnow}}} and {{{$typenow}}} (implicitly) set in
 the test methods? It was always empty during my test runs.

 I `unset()` the three globals that can be altered in
 `set_current_screen()`, but I can see those two being a little
 overcautious. We can remove those `unset()`s if needed.

 > - Should the manual {{{remove_filter}}} be avoided in test methods if
 possible and rely on the built-in  {{{_restore_hooks()}}} during tear
 down? Sometimes it's nice though to see the scope of the filtering, i.e.
 when {{{add_filter}}} is added and when {{{remove_filter}}}. I'm just
 mentioning it here for future reference.

 I don't know that there is a standard for this. I removed
 `remove_filter()` calls that were not necessary for the remainder of the
 test method to succeed it in the interest of brevity. But, like the
 previous item, don't feel strongly that this must be changed.

 > - Should some of the tests be marked with {{{@group l10n}}}?

 Good idea! Added that tag to the appropriate tests.

 The other things you mentioned were fixed as well in
 [attachment:"43985.12.diff"]

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


More information about the wp-trac mailing list