Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#41806 closed enhancement (fixed)

Use get_network_by_path in get_admin_users_for_domain

Reported by: spacedmonkey's profile spacedmonkey Owned by: jeremyfelt's profile jeremyfelt
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch close
Focuses: multisite Cc:

Description

To save doing a raw SQL query to site table, use get_network_by_path instead in get_admin_users_for_domain

Attachments (2)

41806.diff (870 bytes) - added by spacedmonkey 7 years ago.
41806.2.diff (966 bytes) - added by spacedmonkey 7 years ago.

Download all attachments as: .zip

Change History (7)

@spacedmonkey
7 years ago

#1 @flixos90
7 years ago

Thanks for the patch @spacedmonkey! The patch itself looks fine, however the function is deprecated and I'm not sure how much effort should go into those regarding enhancements that go beyond bugfixes.

@jeremyfelt Any best practices for this kind of stuff? Should we just commit it as long as it's as simple as this or ignore since the function is deprecated?

#2 @jeremyfelt
7 years ago

  • Keywords close added

I'd prefer ignoring it. We shouldn't spend much time maintaining deprecated functions unless there's a noticeable benefit. Also, WP_Network::get_by_path() and get_network_by_path() are intended for bootstrap use. For something like this, I think a straight get_networks() would make more sense. I'm going to suggest we close this as a wontfix.

@spacedmonkey
7 years ago

#3 @spacedmonkey
7 years ago

@jeremyfelt The reason for the patch is simple, there are only 2 other places in core that run raw sql queries on the sites table. I already for a patch for ms_load_current_site_and_network (#41762) and it is pointless change network_domain_check as it only used installation process.

For the sake of completely and the simply nature of this patch, I would make the change.

#4 @jeremyfelt
7 years ago

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

In 41664:

Multisite: Use get_networks() instead of a direct query in get_admin_users_for_domain().

Props spacedmonkey.
Fixes #41806.

#5 @netweb
7 years ago

  • Milestone changed from Awaiting Review to 4.9
Note: See TracTickets for help on using tickets.