Skip to content
New issue

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

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

Already on GitHub? # to your account

[BUGS-6657] Handle explicit WP_ALLOW_MULTISITE false correctly #23

Merged
merged 4 commits into from
Aug 30, 2023

Conversation

pwtyler
Copy link
Member

@pwtyler pwtyler commented Aug 15, 2023

Sites with define('WP_ALLOW_MULTISITE', false); are erroneously getting prompted to fix their upstream, as we we only check WP_ALLOW_MULTISITE was set, not its value.

Further consolidated the logic given we were checking both WP_ALLOW_MULTISITE and MULTISITE back-to-back

@pwtyler pwtyler marked this pull request as ready for review August 15, 2023 05:56
@pwtyler pwtyler requested a review from a team as a code owner August 15, 2023 05:56
@pwtyler
Copy link
Member Author

pwtyler commented Aug 30, 2023

Pushed patch to a test site with define('WP_ALLOW_MULTISITE', false); and asserted the banner disappears.

Before

Before

After

after

Copy link
Contributor

@jazzsequence jazzsequence left a comment

Choose a reason for hiding this comment

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

This looks correct to me. My only question (and the answer is probably "no") is whether the splitting of the two things was intentional and whether there's ever a case where WP_ALLOW_MULTISITE is defined but not true and MULTISITE is not defined or is empty and we'd want to load both files.

I think this is a wild enough idea that it's unlikely it was ever the intended purpose so, without looking into each of those files, I'm gonna go ahead and just say yeah, do it.

@pwtyler pwtyler merged commit 223ba5c into main Aug 30, 2023
@jazzsequence jazzsequence deleted the BUGS-6657 branch April 26, 2024 00:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants