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#2721 [REF] Further divide savedSearchParam loading into the sql functions #20954

Merged
merged 2 commits into from
Jul 27, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

dev/core#2721 [REF] Further divide savedSearchParam loading into the sql functions

Before

handling for getting the savedSearch params from the saved search all munged up

After

split out to the type of search they relate to

Technical Details

Builds on #20953

Comments

@civibot
Copy link

civibot bot commented Jul 26, 2021

(Standard links)

@civibot civibot bot added the master label Jul 26, 2021
@colemanw
Copy link
Member

Test failures look related

We have 3 types of saved searches
- search kit
- legacy core searches
- legacy custom searches

The only information these 3 need to load is the savedSearch details and
the group ID (to add in the add & exclude). The wrangling of the params should
happen in the getSql functions - which we can think about being in a listener once
they have standard inputs & outputs. However, to get to that point
we want to standardise those inputs & outputs. This removes
only point of randomness - ie savedSearch has a consistent value & the wrangling
of what is in it is pushed closer to the relevant functions
@eileenmcnaughton
Copy link
Contributor Author

yep I messed up a variable - but noting that there is test cover in these functions

CRM_Contact_Form_Search_Custom_GroupTest.testCount with data set #15
CRM_Contact_Form_Search_Custom_GroupTest.testCount with data set #16
CRM_Contact_Form_Search_Custom_GroupTest.testCount with data set #17
CRM_Contact_Form_Search_Custom_GroupTest.testAll with data set #15
CRM_Contact_Form_Search_Custom_GroupTest.testAll with data set #16
CRM_Contact_Form_Search_Custom_GroupTest.testAll with data set #17
CRM_Contact_Form_Search_Custom_GroupTest.testContactIDs with data set #15
CRM_Contact_Form_Search_Custom_GroupTest.testContactIDs with data set #16
CRM_Contact_Form_Search_Custom_GroupTest.testContactIDs with data set #17
api.v4.Entity.SavedSearchTest.testContactSmartGroup
api.v4.Entity.SavedSearchTest.testEmailSmartGroup
api.v4.Entity.SavedSearchTest.testSmartGroupWithHaving
api.v4.Entity.SavedSearchTest.testMultipleSmartGroups

}
if (isset($ssParams['customSearchID'])) {
$sql = self::getCustomSearchSQL($savedSearchID, $ssParams, $addSelect, $excludeClause);
if (!empty($savedSearch['form_values']['customSearchID'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton is this right why is it buried in form_values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - why is it! But, it is. that form_values field saved to the db is what holds it

@seamuslee001
Copy link
Contributor

ok am comfortable with the unit tests covering us here and reviewed the extraction and seems to make sense merging

@seamuslee001 seamuslee001 merged commit 3fa3f9c into civicrm:master Jul 27, 2021
@seamuslee001 seamuslee001 deleted the sql2 branch July 27, 2021 02:08
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.

3 participants