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

[s2Member-List /] Search by Custom Profile Field #155

Closed
jaswrks opened this issue May 20, 2014 · 28 comments
Closed

[s2Member-List /] Search by Custom Profile Field #155

jaswrks opened this issue May 20, 2014 · 28 comments

Comments

@jaswrks
Copy link
Contributor

jaswrks commented May 20, 2014

@clavaque writes...

Will [s2Member-List /] support search by profile field?

No, only by columns in the wp_users table at this time, since that's what is supported by WP_User_Query. However, we could alter this to improve things in a future release.

@jaswrks
Copy link
Contributor Author

jaswrks commented Jun 9, 2014

@raamdev If you feel like taking a look at this, please let me know. I'd love it if you could help me to make this possible. Here is the routine that we'll need to improve to get this working.

What we need to do is find a way to get this method to search additional user fields; including custom field values stored by s2Member. I see this being quite challenging, but if you have some thoughts on the topic I'd love to hear them.

See also: http://www.primothemes.com/forums/viewtopic.php?t=15658#p48878

@raamdev
Copy link
Contributor

raamdev commented Jun 11, 2014

Yes, I'll take a look at this and see if I can't get some workable code pushed up to a branch.

By the looks of it, this will require running custom MySQL queries, is that correct? And if so, it probably makes sense to implement caching with WordPress transients to improve performance, right?

Assigning this to myself. I'll plan to take a look at this later this week or early next week.

@raamdev raamdev self-assigned this Jun 11, 2014
@raamdev
Copy link
Contributor

raamdev commented Jul 14, 2014

Bump @jaswsinc (see questions above).

@jaswrks
Copy link
Contributor Author

jaswrks commented Jul 15, 2014

By the looks of it, this will require running custom MySQL queries, is that correct? And if so, it probably makes sense to implement caching with WordPress transients to improve performance, right?

Yes, that sounds like a plan to me. Honestly though, I have not thought this through completely. That is, I don't have anything specific in mind for this yet. Thus, whatever you can come up with that is a workable solution would be OK with me, since I don't really know what exactly needs to happen here just yet.

If you start work on this I'd encourage you to just start pushing some ideas/concepts that you have in mind and maybe we can hash things out together. Whatever you think is fine :-) The outline you gave sounds like a great place to start.

@clavaque
Copy link
Contributor

clavaque commented Oct 2, 2014

Another one requesting this: https://websharks.zendesk.com/agent/#/tickets/3896

@raamdev
Copy link
Contributor

raamdev commented Oct 2, 2014

I'll take a closer look at this today and see if I can come up with a workable solution.

@raamdev
Copy link
Contributor

raamdev commented Oct 3, 2014

I'm not sure there's going to be any optimized way of handling this. If we assume that we'll need this to work on a site with thousands (perhaps tens of thousands) of users, then we're likely going to run into some serious performance concerns.


The only way I can see this working is to run a routine prior to calling WP_User_Query($args) that runs an SQL query to search all users for an s2Member Custom Field with a value of the search term (@jaswsinc does my regex syntax look right here?):

SELECT `user_id` as `ID` FROM `wp_usermeta` WHERE `meta_key` = 'wp_s2member_custom_fields' AND `meta_value` REGEXP '.*;s:[0-9]+:".*SEARCHTERM.*".*'

That would give us all users who have an s2Member Custom Field with SEARCHTERM anywhere in any Custom Field value.

Then, with that list of User IDs, we could use the WP_User_Query include parameter to include that list of User IDs in the query that gets run, so that those users would be included (in addition to any other users that were found by the query parameters):

// .....
if($args['number'] < 1) $args['number'] = 1; // Make sure this is always >= 1.
$args['offset'] = ($page - 1) * $args['number']; // Calculate dynamically.

// --------------------------------------------------------

/* Search database for any users with s2Member Custom Fields that contain SEARCHTERM in value */
global $wpdb;
$users = $wpdb->get_results ("SELECT `user_id` as `ID` FROM `" . $wpdb->usermeta . "` WHERE `meta_key` = '" . $wpdb->prefix . "s2member_custom_fields' AND `meta_value` REGEXP '.*;s:[0-9]+:\".*SEARCHTERM.*\".*'");

/* Build an array of all User IDs whose s2Member Custom Fields contain SEARCHTERM in their value */
$include_user_ids = array();
if (is_array ($users) && count ($users) > 0)
    foreach ($users as $user)
        $include_user_ids[] = $user->ID;

/* Add User IDs to a new include argument for WP_User_Query */
if(!empty($include_user_ids))
    $args[] = array('include' => $include_user_ids);

// --------------------------------------------------------

$query = new WP_User_Query($args);
return array('query' => $query, 'pagination' => self::paginate($page, (integer)$query->get_total(), $args['number']));

This is untested code. I just wrote this off the top of my head as an idea for how this might work.

Thoughts @jaswsinc? Do you want me to try implementing this, or are there bigger issues (perhaps related to performance) that you foresee?

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 3, 2014

The only way I can see this working is to run a routine prior to calling WP_User_Query($args) that runs an SQL query to search all users for an s2Member Custom Field with a value of the search term (@jaswsinc does my regex syntax look right here?):

I agree. I think that's the best we can do. It sounds like a great plan to me!

For the regex, you'll want something like this...

(^|;)s\\:[0-9]+\\:\"[KEY]\";s\\:[0-9]+\\:\"[VALUE]\"

In PHP, that would be done like this...

$search_col = 'country'; // Search for the `country` field.
$search_val = 'US'; // Search for this value in the `country` field.

$regex = '(^|;)s\:[0-9]+\:"'.preg_quote($search_col).'";s\:[0-9]+\:"'.preg_quote($search_val).'"';
$sql_regex = "REGEXP '".esc_sql($regex)."'";

// e.g. REGEXP '(^|;)s\\:[0-9]+\\:\"country\";s\\:[0-9]+\\:\"US\"'

It might be possible to optimize this a little bit further by limiting the number of rows that need to be tested via regex, to those which match a simpler (slightly faster) LIKE match first.

WHERE `col` LIKE '%[VALUE]%' AND `col` REGEXP ...

@raamdev
Copy link
Contributor

raamdev commented Oct 4, 2014

$search_col = 'country'; // Search for thecountryfield.

If I read the code in member-list.inc.php correctly, the search automatically applies to all the fields (name, email, etc.), is that correct? That's why I was assuming with my regex that we didn't care what s2Member Custom Field we searched, but rather just the values inside all s2Member Custom Fields.

Am I misunderstanding something about how the shortcode works?

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 4, 2014

WordPress itself takes a search_columns parameter also, and this is supported in member-list.inc.php via the search_columns attribute in the [s2Member-List /] shortcode.

By default, we search all columns. However, it's possible to provide a comma-delimited list of columns to search in the wp_users table if you wish to be more specific. What I'd like to add is the ability to search through custom fields generated with s2Member too.

So for instance, something like this could search all fields, including s2 custom fields (all of them)...

[s2Member-List search="[KEYWORDS]" enable_list_search="yes" /]

Something like this would search only specific fields...

  • user_login, in the wp_users table (already supported)
  • country in the s2member custom fields for each user.
[s2Member-List search="[KEYWORDS]" search_columns="user_login, s2member_custom_field_country" enable_list_search="yes" /]

@jaswrks jaswrks added this to the Next Release milestone Oct 8, 2014
@raamdev
Copy link
Contributor

raamdev commented Oct 9, 2014

@jaswsinc

By default, we search all columns. However, it's possible to provide a comma-delimited list of columns to search in the wp_users table if you wish to be more specific.

I'm looking over the code for this in member-list.inc.php and I see that default search_columns are set. So when you say "by default, we search all columns", do you mean that by default we search those default columns ('ID', 'user_login', 'user_email', 'user_url', 'user_nicename')?

I'm trying to figure out what the conditional would be to check if we're searching "all columns" (i.e., the search_columns attribute was not supplied to the shortcode).

Since we're setting default values for that argument, I can't just search if the search_columns argument is empty. But if I check if the search_columns argument contains only the defaults ('ID', 'user_login', 'user_email', 'user_url', 'user_nicename'), how would I know if the user supplied those exact columns with the intention of excluding the s2Member Custom Fields from their search?

Am I missing something here?

jaswrks pushed a commit to wpsharks/s2member-pro that referenced this issue Oct 9, 2014
jaswrks pushed a commit to wpsharks/s2member-pro that referenced this issue Oct 9, 2014
@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 9, 2014

If you're working from c_ws_plugin__s2member_pro_sc_member_list_in::shortcode(), then an empty value for $attr['search_columns'] or the subsequent $args['search_columns'] in that same routine, would indicate that we are searching all columns. If it's not empty, then a user supplied the search_columns="" attribute and we should only search the columns they have listed specifically.


If you are working from c_ws_plugin__s2member_pro_member_list::query(), then you can check if(empty($original_args['search_columns'])) as seen here. I just added the $original_args variable there, so you might want to do a git pull.

If $original_args['search_columns'] is empty, the same applies. We use the defaults; i.e. we search all columns by default. I fixed a bug related to this just now also. So a git pull would help you.

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 9, 2014

Here is the line where I fixed a bug related to default search columns.

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 9, 2014

So when you say "by default, we search all columns", do you mean that by default we search those default columns ('ID', 'user_login', 'user_email', 'user_url', 'user_nicename')?

Yes, that's right. Those are all of the search-friendly columns supported by \WP_User_Query. However, if we are searching all columns, that list would need to include s2Member custom fields too, as part of this issue. Of course, how we go about doing that exactly, I'm not sure.

As you mentioned before, I think it's going to take additional/separate queries, since we can't rely upon \WP_User_Query to handle the custom s2 fields also.

@raamdev
Copy link
Contributor

raamdev commented Oct 10, 2014

I just added the $original_args variable there, so you might want to do a git pull.

Perfect! Thank you.

@raamdev
Copy link
Contributor

raamdev commented Oct 10, 2014

we could use the WP_User_Query include parameter to include that list of User IDs in the query that gets run, so that those users would be included (in addition to any other users that were found by the query parameters):

Unfortunately, it appears that the include argument to WP_User_Query is not designed to mean "also include these users in the search result", but rather "include these users in the query", which isn't what we want.

I'm now looking into running two queries and merging them. This may actually require three queries (two searches merged and then one final query that we can paginate) because simply merging two queries would break pagination.

@raamdev
Copy link
Contributor

raamdev commented Oct 10, 2014

@jaswsinc I've submitted a Pull Request for review (see wpsharks/s2member-pro#55). Here's how I got this working:

  • Run the original search query with all arguments but return only User IDs ('fields' => 'ID')
  • Run an additional query to search s2Member Custom Fields, if necessary (see new self::search_s2_custom_fields() method); also returns User IDs
  • Merge two groups of User IDs
  • Run a final query to pull User objects with all data ('fields' => 'all_with_meta') and return.

I've tested this and it appears to be working as expected, but I'd appreciate some help doing some additional testing.

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 11, 2014

Oh, that sounds great! I'll review the PR shortly :-) Thanks!!

jaswrks pushed a commit that referenced this issue Oct 14, 2014
jaswrks pushed a commit to wpsharks/s2member-pro that referenced this issue Oct 14, 2014
jaswrks pushed a commit to wpsharks/s2member-pro that referenced this issue Oct 14, 2014
@jaswrks
Copy link
Contributor Author

jaswrks commented Jan 2, 2015

Work from this issue went out with the release of s2Member v150102.
http://www.s2member.com/changelog/#s2-changes-v150102

@jaswrks
Copy link
Contributor Author

jaswrks commented Jan 9, 2015

Reopening this issue due to a bug that was brought to my attention by @brucewsinc. It looks like our work to add search fields added a bug by mistake.

Pagination is no longer working as expected. We need to include the order, orderby, and number args in the second and final query that takes place once all of the user IDs are collected.

Referencing: https://github.com/websharks/s2member-pro/blob/000000-dev/s2member-pro/includes/classes/member-list.inc.php#L113

@jaswrks jaswrks reopened this Jan 9, 2015
@jaswrks
Copy link
Contributor Author

jaswrks commented Jan 12, 2015

@raamdev Would you mind having a look at this for me?

~ Here is a related issue we could try to knock out at the same time:
#394

@raamdev raamdev modified the milestones: Next Release, v150102 Release Jan 12, 2015
@raamdev
Copy link
Contributor

raamdev commented Jan 12, 2015

Would you mind having a look at this for me?

Got it. I'll work on this.

Here is a related issue we could try to knock out at the same time: #394

I'll also tackle that one.

@jaswrks
Copy link
Contributor Author

jaswrks commented Jan 23, 2015

@raamdev I took care of this already, along with #394. It just seemed easier since I was already in waste deep. If you have done work on this yourself that you'd like to merge in, please feel free to do that. Otherwise, no worries :-)

@jaswrks
Copy link
Contributor Author

jaswrks commented Jan 23, 2015

Next Release Changelog:

  • (s2Member Pro) [s2Member-List /] Bug Fix: This release resolves an issue with pagination in the [s2Member-List /] shortcode after recent improvements to the search functionality. See this GitHub issue if you'd like additional details.
  • (s2Member Pro) [s2Member-List /] Enhancement: This release improves search functionality in the [s2Member-List /] shortcode, making it so that all searches default to *[query]*; i.e. are automatically wrapped by wildcards. If a user enters a wildcard explicitly (or a double quote), this default behavior is overridden and the search query is taken as given in such a scenario. This makes the search functionality easier for end-users to work with, since it no longer requires an exact match. Default behavior is now a fuzzy match. See also: this GitHub issue if you'd like further details.

@raamdev
Copy link
Contributor

raamdev commented Jan 23, 2015

I took care of this already, along with #394.

Copy. Thanks for taking over. :) I hadn't started--was caught up with... well, with everything else!

@jaswrks
Copy link
Contributor Author

jaswrks commented Feb 4, 2015

This issue was resolved in the release of s2Member v150203. Please see: http://www.s2member.com/kb/v150203/ If there are any follow-ups needed, please open a new issue and we will investigate. Thanks!

@wpsharks wpsharks locked and limited conversation to collaborators Feb 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants