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

dev/core#3495 - Fix fields with wildcards in Advanced Search #23697

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

bsilvern
Copy link
Contributor

@bsilvern bsilvern commented Jun 5, 2022

Overview

In the affected function, some fields are arrays, e.g. ['LIKE' => 'field-value'], rather than strings as was expected. Modified to now check for that and handle correctly.

Before

  1. Some search terms, e.g. Contribution Source, are changed to '%%' upon sorting or paging the results.
  2. After fixing the above, an extraneous trailing '%' on search terms was appending on each sort of paging of the results.

After

  1. All search terms are preserved.
  2. Extraneous wildcards are no longer appended.

Technical Details

Comments

Bob Silvern added 2 commits June 5, 2022 14:47
…rashed

Correctly handle those field values which are arrays, e.g.
['LIKE' => 'field-value'],  rather than strings.
Only add a trailing '%' if it does not already exist.
@civicrm-builder
Copy link

Can one of the admins verify this patch?

@civibot
Copy link

civibot bot commented Jun 5, 2022

(Standard links)

@civibot civibot bot added the master label Jun 5, 2022
@seamuslee001
Copy link
Contributor

Jenkins ok to test

@@ -6309,15 +6309,13 @@ public static function getQillValue($daoName, string $name, $value, $op, string
public static function getWildCardedValue($wildcard, $op, $value) {
if ($wildcard && $op === 'LIKE') {
if (CRM_Core_Config::singleton()->includeWildCardInName && (substr($value, 0, 1) != '%')) {
return "%$value%";
$value = "%$value";
Copy link
Member

@totten totten Jun 13, 2022

Choose a reason for hiding this comment

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

Before, it wrapped the string with %s on both sides. After, it only prepends %.

My memory of this codepath is a bit stale -- but wouldn't that be a substantive change in the search behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the following two lines (6314-6315) which add the trailing '%' if it does not already exist.

@eileenmcnaughton
Copy link
Contributor

I was able to replicate this and confirm that the patch fixes the bug and that paging still works on the Contribution Search (which did not have this bug).

Much as I hate touching anything at all in the legacy search code now this is clearly a bug rather than a new feature and stepping through the code I could see it was carefully implemented with handling for both forms.

I'm going with 'gratefully accept this patch'

@eileenmcnaughton eileenmcnaughton merged commit 4f9ab9f into civicrm:master Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants