Make WordPress Core

Opened 12 years ago

Last modified 5 years ago

#20116 new defect (bug)

Welcome User Email in Multisite Can't Be Changed

Reported by: ipstenu's profile Ipstenu Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Login and Registration Keywords: has-patch fixed-minor needs-testing
Focuses: multisite Cc:

Description

Reproduced this on 3.3 and 3.4-aortic.

Go to /wp-admin/network/settings.php

Add 'New' to the sentance 'Welcome User' to make it 'Welcome New User'

Hit update.

Page refreshes, text does not change.

Attachments (5)

20116.patch (2.9 KB) - added by SergeyBiryukov 12 years ago.
20116.minimal.diff (1.3 KB) - added by duck_ 12 years ago.
20116.2.patch (3.7 KB) - added by SergeyBiryukov 12 years ago.
20116.minimal.2.diff (597 bytes) - added by nacin 12 years ago.
20116.3.patch (2.7 KB) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (22)

#1 @ocean90
12 years ago

Isn't it 'Dear User'? http://core.trac.wordpress.org/browser/trunk/wp-admin/includes/ms.php#L213

Tested in 3.3 and current trunk, works in both.

#2 @SergeyBiryukov
12 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.4

It's 'Dear User' indeed, but I guess Ipstenu refers to welcome_user_email option:
http://core.trac.wordpress.org/browser/tags/3.3.1/wp-includes/ms-functions.php#L1897

I can reproduce the bug in trunk. Seems introduced in [18588].

Since default welcome_user_email is implemented via a filter, update_site_option() doesn't call add_site_option() and then fails to update a non-existing option.

We should get rid of welcome_user_msg_filter() and make welcome_user_email a proper option, like welcome_email.

#3 @Ipstenu
12 years ago

Erf, yeah, Dear User, but that's what I meant. Sorry, trying to do trac and give directions on the phone is a bad idea.

#4 follow-up: @nacin
12 years ago

  • Milestone changed from 3.4 to 3.3.2

I think we could drop the ms-functions.php changes in the 3.3 branch. However, let's not remove a function entirely in case someone is using it. (In a branch, keep as is, I'd say.)

@duck_
12 years ago

#5 in reply to: ↑ 4 @SergeyBiryukov
12 years ago

Replying to nacin:

However, let's not remove a function entirely in case someone is using it.

20116.2.patch moves welcome_user_msg_filter() to ms-deprecated.php.

#6 @nacin
12 years ago

So I finally looked at the root issue. The option does not exist, but when update_site_option() calls get_site_option(), it is told that it has a value, thanks to the filter. So instead of passing things off to add_site_option() to do an insert, it does an update, and there's now row with that option name to update.

That is lame, and affects get_option/update_option as well. I think we need *option_exists() methods in the future as a way to skip the filters in get_*.

Regardless, I came up with a targeted fix for the 3.3 branch. Attached.

#7 @nacin
12 years ago

In [20543]:

Rolling upgrade to set the welcome_user_email network-wide option via the welcome_user_msg_filter() callback. Fixes ability to save the welcome user email when no entry exists in the database. see #20116 for the 3.3 branch.

#8 follow-up: @nacin
12 years ago

For trunk, I'd like to go along with something like 20116.2.diff. However, populate_network() only runs when the network is initially created. We should add some code to upgrade_network().

Since this is going to mean we are repeating this email three times, maybe we can just repurpose welcome_user_msg_filter(). Make $text default to false and then welcome_user_msg_filter() can just give us our email content. Perhaps too hacky, but it's an idea.

#9 in reply to: ↑ 8 @SergeyBiryukov
12 years ago

Replying to nacin:

However, populate_network() only runs when the network is initially created. We should add some code to upgrade_network().

Yeah, I forgot that without adding the option, the textarea on Network Settings screen would be empty, although wpmu_welcome_user_notification() would still get the proper text.

Make $text default to false and then welcome_user_msg_filter() can just give us our email content.

Done in 20116.3.patch.

#10 @nacin
12 years ago

  • Keywords fixed-minor added

#11 @nacin
12 years ago

  • Milestone changed from 3.3.2 to 3.4

#12 @ryan
12 years ago

Per bug scrub, port [20543] to trunk and punt anything that remains.

#13 @ryan
12 years ago

In [20697]:

Rolling upgrade to set the welcome_user_email network-wide option via the welcome_user_msg_filter() callback. Fixes ability to save the welcome user email when no entry exists in the database. see #20116

#14 @ryan
12 years ago

  • Milestone changed from 3.4 to Future Release

#15 @jeremyfelt
10 years ago

  • Component changed from Multisite to Login and Registration
  • Focuses multisite added

#16 @chriscct7
8 years ago

  • Keywords needs-testing added

What's left for this?

#17 @Ipstenu
8 years ago

This is working on trunk right now. This can be closed unless someone else finds a problem.

Note: See TracTickets for help on using tickets.