Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#32503 closed enhancement (fixed)

Remove the "Update siteurl and home as well" checkbox option on multisite "Edit Site" screen.

Reported by: hugobaeta's profile hugobaeta Owned by: jeremyfelt's profile jeremyfelt
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch
Focuses: ui, multisite Cc:

Description

In #22383 there is a discussion about what a possible use-case might be for this checkbox. No one seems to understand why it's there, specially with the combined "home" and "siteurl".

This ticket serves to try and understand a use-case for it and, if none is found, simply remove the checkbox from the UI for simplification.

https://cldup.com/Y3jne2Hoe6.png

(This idea is mocked up as if #22383 is already implemented)

Attachments (3)

32503.diff (2.0 KB) - added by earnjam 9 years ago.
32503.1.diff (2.7 KB) - added by earnjam 9 years ago.
32503.2.diff (3.3 KB) - added by jeremyfelt 9 years ago.

Download all attachments as: .zip

Change History (24)

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


9 years ago

#2 follow-ups: @earnjam
9 years ago

It looks like the origin is from https://mu.trac.wordpress.org/ticket/920 with the addition of the checkbox occuring in https://mu.trac.wordpress.org/changeset/1688.

The URL on this screen is the combination of domain and path and should match the value in home, but it might not match site_url if WP is installed in a subdirectory. I can't think of a scenario where the URL (domain/path) and home would ever be different.

If we keep the checkbox, perhaps it should be reduced to only "Update site_url as well" and have home be updated every time.

@earnjam
9 years ago

#3 @earnjam
9 years ago

  • Keywords has-patch added

In 32503.diff switch checkbox to only handle updating of site_url and force home to be updated by default on changes to domain and/or path.

#4 in reply to: ↑ 2 @hugobaeta
9 years ago

Replying to earnjam:

If we keep the checkbox, perhaps it should be reduced to only "Update site_url as well" and have home be updated every time.

So, if you don't select the checkbox, what happens? Where would you update site_url?

#5 in reply to: ↑ 2 ; follow-up: @jeremyfelt
9 years ago

Replying to earnjam:

The URL on this screen is the combination of domain and path and should match the value in home, but it might not match site_url if WP is installed in a subdirectory. I can't think of a scenario where the URL (domain/path) and home would ever be different.

I think I'm actually flipped on imagination here. :)

If I have a site that I edit using mysite.company-admin.test/wp-admin/, I want the siteurl set to https://mysite.company-admin.test/ and the domain/path set to mysite.company-admin.test and /.

I can imagine in this same scenario that my front-end actually lives at https://company.com and that all home uses should generate the URL accordingly.

In reality, that would probably require extensive filtering. It's just where my imagination has gone.

#6 in reply to: ↑ 5 ; follow-up: @earnjam
9 years ago

Replying to hugobaeta:

So, if you don't select the checkbox, what happens? Where would you update site_url?

You can edit both home and site_url on site-settings.php.

Replying to jeremyfelt:

If I have a site that I edit using mysite.company-admin.test/wp-admin/, I want the siteurl set to https://mysite.company-admin.test/ and the domain/path set to mysite.company-admin.test and /.

I can imagine in this same scenario that my front-end actually lives at https://company.com and that all home uses should generate the URL accordingly.

In reality, that would probably require extensive filtering. It's just where my imagination has gone.

That sounds more like the traditional view of domain mapping, which as you noted would require extensive filtering.

I guess the main goal here is simplifying the UI and we wanted to open a ticket to start up discussion as to what would happen without the checkbox. I guess we can't update home and site_url automatically because of the two situations outlined in the original ticket and the comments above.

Right now the checkbox only shows when home and site_url are identical. Which is actually not all that helpful because if they are the same, you wouldn't have the issue of the original ticket.

Perhaps we could just update both home and site_url automatically if they are the same and equal to the previous value of domainpath and don't update them if home and site_url are different. The downside of that is that it's confusing because now we have 4 separate values set in 2 locations and changing the domainpath values on site-info.php wouldn't really do anything.

Version 0, edited 9 years ago by earnjam (next)

#7 in reply to: ↑ 6 ; follow-up: @jeremyfelt
9 years ago

  • Milestone changed from Awaiting Review to 4.3
  • Type changed from defect (bug) to enhancement

Replying to earnjam:

Perhaps we could just update both home and site_url automatically if they are the same and equal to the previous value of domainpath and don't update them if home and site_url are different.

This seems pretty right on. We can remove the checkbox entirely and then treat the logic during update as:

  • If previous domain + path (from wp_blogs) === domain + path (from siteurl), update siteurl.
  • If previous domain + path (from wp_blogs) === domain + path (from home), update home.

If somebody already has a different siteurl or home, it won't track the site changes and we won't break anything. If somebody purposefully alters home or siteurl, changes will stop tracking and they're on their own.

At some point we'll also want to maintain the relationship after a scheme change by comparing domain + path and scheme separately. If the only difference when editing the site is a change from HTTP to HTTPS, that should be reflected. This, of course, relies on #14172 for tracking scheme at the wp_blogs level in the first place and shouldn't necessary be part of this ticket's patch if it can go in first.

The downside of that is that it's confusing because now we have 4 separate values set in 2 locations and changing the domainpath values on site-info.php wouldn't really do anything practical. It would just change the values in wp_blogs

The wp_blogs data is the most important part though, as it's what our bootstrap uses to find the site originally.

#8 in reply to: ↑ 7 @earnjam
9 years ago

Replying to jeremyfelt:

The wp_blogs data is the most important part though, as it's what our bootstrap uses to find the site originally.

Well yes...but if home and site_url were not updated with it then it would just redirect you to one of those URLs, right?

I'll update the patch to reflect that logic and remove the checkbox.

@earnjam
9 years ago

#9 @earnjam
9 years ago

Patch 32503.1.diff removes the checkbox and uses the following logic for updating home and siteurl:

  • If http://domainpath (from wp_blogs) === siteurl, update siteurl.
  • If http://domainpath (from wp_blogs) === home, update home.

This uses http for the protocol for the values from wp_blogs because it's closest to the current functionality. In the current version, the checkbox is unchecked if the full URL from home or siteurl is different from http://domainpath. So if either home or siteurl are changed to https, then it would be unchecked. Checking the box in that situation would force both home and siteurl to be updated to using http.

32503.1.diff only updates siteurl and/or home in the event they are unchanged from their original value of http. If either of those values are changed, then it assumes they are being managed independently. If/when we begin tracking scheme in #14172, then we can add support for that here as well.

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


9 years ago

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


9 years ago

#12 follow-up: @Ipstenu
9 years ago

This uses http for the protocol for the values from wp_blogs because it's closest to the current functionality.

and

So if either home or siteurl are changed to https, then it would be unchecked.

Unchecked or unchanged? Or both? That may be part of a bigger issue, though, https and Multisite is weird to begin with (I had to edit the DB to get https for one site on the network). I would want https://old.domain.com to turn into https://new.domain.com after all.

32503.1.diff​ works fine on my tests.

#13 in reply to: ↑ 12 @earnjam
9 years ago

Replying to Ipstenu:

This uses http for the protocol for the values from wp_blogs because it's closest to the current functionality.

and

So if either home or siteurl are changed to https, then it would be unchecked.

Unchecked or unchanged? Or both? That may be part of a bigger issue, though, https and Multisite is weird to begin with (I had to edit the DB to get https for one site on the network). I would want https://old.domain.com to turn into https://new.domain.com after all.

The box is unchecked when you load the page if either has an https value. If you were to check it and click save, it will reset both of the values to http.

@jeremyfelt
9 years ago

#14 @jeremyfelt
9 years ago

32503.2.diff handles scheme separately from domain and path. I think that should be safe to do overall.

  • Parse into scheme, domain, and path.
  • Compare domain and path only.
  • If domain and path match, update home and/or siteurl with the new domain and path and their respective existing schemes.

The repetitive use of parse_url is a little weird, but it avoids a bunch of isset( $parsed_url['path'] ), etc... Still looking at it though.

Once #22383 is in, we can listen for scheme changes as well when intentionally added.

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


9 years ago

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


9 years ago

#17 @jeremyfelt
9 years ago

  • Owner set to jeremyfelt
  • Resolution set to fixed
  • Status changed from new to closed

In 32760:

Remove the "Update siteurl and home as well" checkbox when editing a site

Rather than provide a checkbox to update the siteurl and home options, we can make an educated decision based on the current state. If the home and/or siteurl domain and path match the existing domain and path of the site, then we update with the new information.

Also, while scheme is not stored in wp_blogs along with a site, the scheme of the home and siteurl options can now be modified via the Site URL setting in site-info.php when the home and/or siteurl options match the existing domain.

Props @hugobaeta, @earnjam, @jeremyfelt.
Fixes #32503, see #22383.

#18 @jeremyfelt
9 years ago

In 32761:

Remove a now unused switch_to_blog() and restore_current_blog() in site-info.php

Now that we decide when to automatically update home and siteurl, we no longer need to switch to the site while displaying the form output.

See #32503.

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


9 years ago

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


9 years ago

#21 @chriscct7
9 years ago

#23270 was marked as a duplicate.

Note: See TracTickets for help on using tickets.