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#183 Convert the generating of temporary tables within the ra… #15876

Merged
merged 1 commit into from
Nov 21, 2019
Merged

dev/core#183 Convert the generating of temporary tables within the ra… #15876

merged 1 commit into from
Nov 21, 2019

Conversation

seamuslee001
Copy link
Contributor

…ndom segment custom search to using standard CRM_Utils_SQL_TemporaryTable method and ensure that the random segement custom search shows up in the list of avliable custom searches

Overview

This changes the way temporary tables are created within the random segment custom search and also ensures that it shows up in the list of possible custom searches.

Before

Legacy way of coding up temporary tables used

After

CRM_Utils_SQL_TempTable interface used

ping @eileenmcnaughton @totten

@civibot
Copy link

civibot bot commented Nov 17, 2019

(Standard links)

@civibot civibot bot added the master label Nov 17, 2019
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 if this isn't currently exposed I don't think we should expose it - the goal is to get these searches out of core not to encourage their use

eileenmcnaughton
eileenmcnaughton reviewed Nov 18, 2019
View reviewed changes
xml/templates/civicrm_data.tpl Outdated
@@ -578,6 +578,7 @@ VALUES
(@option_group_id_csearch , 'CRM_Contact_Form_Search_Custom_ContribSYBNT' ,13, 'CRM_Contact_Form_Search_Custom_ContribSYBNT', NULL, 0, NULL, 13, '{ts escape="sql"}Contributions made in Year X and not Year Y{/ts}', 0, 0, 1, NULL, NULL, NULL),
(@option_group_id_csearch , 'CRM_Contact_Form_Search_Custom_TagContributions' ,14, 'CRM_Contact_Form_Search_Custom_TagContributions', NULL, 0, NULL, 14, '{ts escape="sql"}Find Contribution Amounts by Tag{/ts}', 0, 0, 1, NULL, NULL, NULL),
(@option_group_id_csearch , 'CRM_Contact_Form_Search_Custom_FullText' ,15, 'CRM_Contact_Form_Search_Custom_FullText', NULL, 0, NULL, 15, '{ts escape="sql"}Full-text Search{/ts}', 0, 0, 1, NULL, NULL, NULL),
(@option_group_id_csearch , 'CRM_Contact_Form_Search_Custom_RandomSegment' ,16, 'CRM_Contact_Form_Search_Custom_RandomSegment', NULL, 0, NULL, 16, '{ts escape="sql"}Random Segment Search{/ts}', 0, 0, 1, NULL, NULL, NULL),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind changing the search form but let's not do this

@seamuslee001 seamuslee001 force-pushed the dev_core_183_random_segement branch from 22e50a1 to 67ae5d6 Compare November 18, 2019 04:33
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton ok have pulled those changes now

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 why is the composer change in there

@seamuslee001
Copy link
Contributor Author

because the composer.lock file is currently out of sync with composer.json after #15868 ping @colemanw tl/dr even tho its not strickly speaking a composer package the composer hash still needs updating in composer.lock

Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. Run update to update them.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 ok - it's probably not good to have it in the same PR

@seamuslee001
dev/core#183 Convert the generating of temporary tables within the ra…
1cde537 
…ndom segment custom search to using standard CRM_Utils_SQL_TemporaryTable method
@seamuslee001 seamuslee001 force-pushed the dev_core_183_random_segement branch from 67ae5d6 to 1cde537 Compare November 19, 2019 21:18
@seamuslee001
Copy link
Contributor Author

ok have pulled it out into #15886 @eileenmcnaughton

@eileenmcnaughton
Copy link
Contributor

As previously discussed this is probably not used at all & could be removed - happy to merge this change for grepping / clarity

@eileenmcnaughton eileenmcnaughton merged commit 212f1cd into civicrm:master Nov 21, 2019
@eileenmcnaughton eileenmcnaughton deleted the dev_core_183_random_segement branch November 21, 2019 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eileenmcnaughton eileenmcnaughton eileenmcnaughton left review comments

Assignees
No one assigned
Labels
master merge on pass
Projects
None yet
Milestone
No milestone
Development

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