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.

Sign up for GitHub

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

Jump to bottom

dev/core#1244 Fix regression on searching by email in advanced search #15255

Merged
merged 1 commit into from
Sep 10, 2019
Merged

dev/core#1244 Fix regression on searching by email in advanced search #15255

merged 1 commit into from
Sep 10, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 9, 2019 •
edited by seamuslee001
Loading

Overview

Fixes regression whereby searching on email in advanced search fails

Before

Results not returned and warnings emitted
eileen pr before

After

Results returned and no warnings
Eileen pr after

Technical Details

Issue is it's coming in as an array & the code foolishly only handles arrays if $apiEntity is set. I renamed this to what it actually means

Comments

@civibot
Copy link

civibot bot commented Sep 9, 2019

(Standard links)

@civibot civibot bot added the 5.18 label Sep 9, 2019
@eileenmcnaughton eileenmcnaughton changed the title Fix regression on searching by contact in advanced search Fix regression on searching by email in advanced search Sep 9, 2019
@seamuslee001
Copy link
Contributor

@eileenmcnaughton can you fill out the PR template? Also is this released or unreleased?

@eileenmcnaughton
Copy link
Contributor Author

I'm assuming unreleased but I haven't checked / confirmed yet

@seamuslee001
Copy link
Contributor

@eileenmcnaughton it looks like this affects 5.17 but not 5.16 i think

@eileenmcnaughton
Copy link
Contributor Author

damn - still 5.17 about to go out?

@seamuslee001
Copy link
Contributor

@eileenmcnaughton 5.17.1 has been released

@eileenmcnaughton
Copy link
Contributor Author

hmm - we better do another one. At least the fact it's in 5.17 validates my decision to have just deployed 5.18 rather than 5.17 into production - we would have hit this either way

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

  • General standards
    • (r-explain) Pass
    • (r-user) Pass
    • (r-doc) Pass
    • (r-run) Pass was able to replicate the problem. Which can be done by just doing an advanced search and filling in the email field and then clicking search and you get warning notices and no results are found. After the notices disappear and search works.
  • Developer standards

@eileenmcnaughton
Copy link
Contributor Author

I updated the PR template but haven't grabbed a screenshot

@seamuslee001
Copy link
Contributor

@eileenmcnaughton added screenshots

@seamuslee001
Copy link
Contributor

@eileenmcnaughton would this be worth a drop by its self?

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 yes I think so

@seamuslee001 seamuslee001 changed the title Fix regression on searching by email in advanced search dev/core#1244 Fix regression on searching by email in advanced search Sep 10, 2019
@seamuslee001 seamuslee001 merged commit 5e16c4c into civicrm:5.18 Sep 10, 2019
@seamuslee001 seamuslee001 mentioned this pull request Sep 10, 2019
Merged
@seamuslee001 seamuslee001 mentioned this pull request Sep 10, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
5.18
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eileenmcnaughton @seamuslee001