Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multisite fix #25

Merged
merged 3 commits into from Feb 24, 2022
Merged

Multisite fix #25

merged 3 commits into from Feb 24, 2022

Conversation

dotsam
Copy link
Contributor

@dotsam dotsam commented Feb 3, 2022

Fixes #24

I did end up taking the "treat the URL as a suffix" approach, rather than trying to guess and split the domain into parts. I believe this should handle all cases, but in cases where there isn't a match, it may produce an invalid URL.

As discussed, I've also added a filter for cases when the normal logic doesn't apply (primarily I'm thinking of production sites that have a unique domain, in which case the user will need to maintain their own mapping).

I've also removed the composer autoloader since there are no longer any external deps, and also tweaked the CSS selector for the admin bar icon to be more in-line with how WP Core targets things.

 - Switch to treating the provided environment host as a suffix
 - Move things out of the stages loop that don't need to be there
 - Add a filter on the URL for any custom logic requirements
@dotsam
Copy link
Contributor Author

dotsam commented Feb 16, 2022

@swalkinshaw Thoughts on this PR? I'm happily using it in production on a multisite subdomain installation.

'class' => 'environment-' . sanitize_html_class(strtolower($current_stage)),
],
]);
$url = apply_filters('bedrock/stage_switcher_url', rtrim($url, '/') . $_SERVER['REQUEST_URI'], $url, $stage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the filter name just be the plugin name exactly? So either roots/wp-stage-switcher or wp-stage-switcher. I don't think bedrock should be part of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was using the format of the existing bedrock/stage_switcher_visibility filter, but I'm happy to change it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, forgot about that. Okay this works 👍

@swalkinshaw swalkinshaw merged commit dd50bff into roots:master Feb 24, 2022
@swalkinshaw
Copy link
Member

Thanks @dotsam. We'll do a release soon

@dotsam
Copy link
Contributor Author

dotsam commented May 19, 2022

@swalkinshaw Hi Scott, would it be possible to cut a new release now? I've had dev-master running on multisite and non-multisite installs for a while now without issue.

@retlehs
Copy link
Sponsor Member

retlehs commented May 31, 2022

@dotsam done, thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Multisite URL Rewriting Not Functional
3 participants