Make WordPress Core

Opened 9 years ago

Last modified 2 years ago

#37297 reviewing enhancement

Deprecate get_blogaddress_by_id function

Reported by: spacedmonkey's profile spacedmonkey Owned by: spacedmonkey's profile spacedmonkey
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: needs-patch
Focuses: multisite Cc:

Description

The get_blogaddress_by_id function is pretty old and should be replaced with get_home_url, as this is filtered.

Attachments (1)

37297.diff (3.0 KB) - added by spacedmonkey 9 years ago.

Download all attachments as: .zip

Change History (13)

@spacedmonkey
9 years ago

#1 @thomaswm
9 years ago

You cannot replace get_blogaddress_by_id with get_home_url in install_blog(), because the home option hasn't been defined yet at that point. It is set in line 1360 in ms-functions.php.

So, in install_blog(), you'll have to do something like

$site = get_site( $blog_id );
$url = 'http://' . $site->domain . $site->path;

to get the value of $url.

#2 @spacedmonkey
9 years ago

  • Keywords has-patch 4.7-early added

#3 @jeremyfelt
8 years ago

  • Keywords 4.7-early removed
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to jeremyfelt
  • Status changed from new to reviewing

Noting #26855 in here as well. Let's see if we can deprecate this early. :)

#4 @jorbin
8 years ago

Early is coming to a an end. (whatever early means). @jeremyfelt - Think this can go in, or do you need to do #26855 first?

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#6 @jeremyfelt
8 years ago

  • Keywords needs-patch added; has-patch removed
  • Owner jeremyfelt deleted

I spent some time with 37297.diff the other day and ran into some uncertainty.

As @thomaswm mentioned, get_home_url() is not appropriate solution for install_blog(). get_blogaddress_by_id() attempts to use the home option to determine the URL's scheme, but it will always be empty at this point. We then know it will be HTTP combined with the site's domain and path.

It does seem to be the right option for wpmu_welcome_notification(). I'd like this to be tested more though.

The use in wp-activate.php has already been removed via #26855.

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#11 @flixos90
8 years ago

  • Milestone changed from 4.7 to Future Release

This needs further thought and should be tested earlier in a release cycle. Let's reconsider it in one of the next releases.

#12 @spacedmonkey
2 years ago

  • Milestone set to Future Release
  • Owner set to spacedmonkey
Note: See TracTickets for help on using tickets.