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

Bug: s2Member-List Search #765

Closed
jaswrks opened this issue Oct 28, 2015 · 5 comments
Closed

Bug: s2Member-List Search #765

jaswrks opened this issue Oct 28, 2015 · 5 comments
Assignees

Comments

@jaswrks
Copy link
Contributor

jaswrks commented Oct 28, 2015

Overview

Reported and reproduced by @patdumond in this private ticket.

Steps to Reproduce

  • Create 5 users with s2Member Custom Field state and set the value to TX (Texas).

  • Use the following shortcode to search for members.

    [s2Member-List enable_list_search="yes" search_columns="s2member_custom_field_state" search="TX" limit="5" /]
    

Expected Result

5 results matching Custom Field state to the value of TX.

Observed Behavior

In WP_DEBUG mode, an SQL error occurs. Resulting in 0 results.

A Few Problems

  • Our regex pattern match looks for this:

    (^|\{)s\:[0-9]+\:"(state)";s\:[0-9]+\:"[^"]*TX[^"]*"
    

    This should include ; as a possible prefix also; i.e., (^|\{|;)

    (^|\{|;)s\:[0-9]+\:"(state)";s\:[0-9]+\:"[^"]*TX[^"]*"
    
  • We need to filter s2member_custom_field_ prefixes from this array, now that two searches take place in order to break things down properly. This was a recent change to address another bug, and has resulted in a new bug that now impacts searches looking for Custom Field values.

  • These two lines should only be run if the search query scans columns that are not s2Member Custom Fields. Otherwise the WP_User_Query{} call fails.

  • The fuzzy search that takes place (i.e., "[^"]*TX[^"]*") in this particular scenario goes against what we documented in this KBA when it comes to searches that are less than 3 characters. This is a separate bug, but it impacts searches against s2Member Custom Fields also. We should update this to provide better consistency and clarity whenever a site owner is operating with the instructions we have given. See: https://s2member.com/kb-article/s2member-list-shortcode-documentation/#toc-64ab22e4

    In short, it should be: (^|\{|;)s\:[0-9]+\:"(state)";s\:[0-9]+\:"TX", since TX is only two chars.

@jaswrks jaswrks self-assigned this Oct 28, 2015
@jaswrks jaswrks added this to the Next Release milestone Oct 28, 2015
@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 28, 2015

Next Actions

  • New feature branch in the websharks/s2member-pro repo.

  • After this line add the following:

    foreach(self::$search_columns_for_filter as $_key => $_column) {
      if(stripos($_column, 's2member_custom_field_') === 0)
        unset(self::$search_columns_for_filter[$_key]);
    }
    unset($_key, $column); // Housekeeping.
  • Replace these lines with the following:

    $user_ids = array(); // Intialize.
    if ($user_id_args['search'] && self::$search_columns_for_filter) {
        $user_ids_query = new WP_User_Query($user_id_args);
        $user_ids  = $user_ids_query->get_results();
    }
  • Replace this line with the following:

    $regex = '(^|\{|;)s\:[0-9]+\:"('.$matching_custom_fields_regex_frag.')";s\:[0-9]+\:"'.$search_regex_frag.'"'; // e.g., `a:1:{s:12:"country_code";s:3:"USA";}`.
  • Submit PR.

@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 28, 2015

Short-Term Patch

For the benefit of others, and NOT a part of the outline for this issue.

  • Download this updated file and upload via FTP. Allow this file to override your existing copy of /s2member-pro/includes/classes/member-list.inc.php

This is a quick hack that I used while running through the instructions above. It will fix you up for now.

@jaswrks jaswrks assigned renzms and unassigned jaswrks Oct 28, 2015
@jaswrks
Copy link
Contributor Author

jaswrks commented Oct 28, 2015

I wrote previously...

The fuzzy search that takes place (i.e., "[^"]TX[^"]") in this particular scenario goes against what we documented in this KBA when it comes to searches that are less than 3 characters.

That is incorrect. This is not a separate bug. It's the intended behavior. The KBA says >= 2 characters, and TX meets that criteria. I read it wrong before.

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

jaswrks commented Oct 29, 2015

Next Release Changelog:

  • (s2Member Pro) [s2Member-List /] Bug Fix: This release resolves a problem in the [s2Member-List /] shortcode whenever it is configured to search Custom Fields generated with s2Member. Props @patdumond @renzms. See this GitHub issue if you'd like additional details.

@jaswrks jaswrks closed this as completed Oct 29, 2015
@jaswrks
Copy link
Contributor Author

jaswrks commented Dec 10, 2015

Work from this issue was released in s2Member & s2Member Pro v151210.
See changelog: http://s2member.com/changelog/

Future comments on this issue will now be blocked. If you have any trouble please open a new issue and report it. A big thanks to all of our great supporters. Happy Holidays :-) ❄️

@wpsharks wpsharks locked and limited conversation to collaborators Dec 10, 2015
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

2 participants