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

Introduce filter, 'wp_multisite_sso_pre_get_network_sites' #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

r-a-y
Copy link
Contributor

@r-a-y r-a-y commented Mar 15, 2018

Thanks for the useful plugin!

In the get_network_sites() method, by default, it fetches all sites and active mapped domains.

For our site's needs, we don't need to fetch all sites since most of our sites are subdomains, so logging in from the main site handles 90% of our needs here. Related: #26.

Secondly, we have many mapped domains, so the default behavior to login to each mapped domain isn't useful to us. Also, not all mapped domains are active or in WPMU Domain Mapping terms, primary.

In this PR, I've introduced a new filter, 'wp_multisite_sso_pre_get_network_sites', to allow developers to do their own site lookup routine to bypass the default network site lookup.

If you're interested to see how we're using this filter, let me know and I can pass on what we're using. We basically are using this filter to address #6.

This filter allows developers to bypass the default logic to fetch all
network sites and active mapped domains for SSO.
@kevinlangleyjr
Copy link
Member

@r-a-y thanks for this! I can see the benefit of having this filter before the logic so you can perform your own logic, but I think we should add a filter before we return the value as well (line 95). This would allow someone to either use the pre filter you added here to perform their own logic or allow our logic to run in the plugin but still give another opportunity to filter it after the logic is done but before returning. I'll add that in and merge your PR either today or tomorrow.

Thanks again for all the contributions to the plugin!

@r-a-y
Copy link
Contributor Author

r-a-y commented Apr 10, 2018

but I think we should add a filter before we return the value as well (line 95)

Sounds good, go ahead and add an extra filter there. I didn't do it because I wanted to avoid the initial lookup.

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.

2 participants