[REF] Move more code to the trait [WIP] #16936
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
email2
branch
8 times, most recently
from
7f392e5 to
4e08a5f
Compare
eileenmcnaughton
changed the title
|
@colemanw I wanted to check your thoughts on this - 4e08a5f - basically it's an attempt to change the ad-hoc ajax to be an entity reference (& over up the refine search goodness). That patch only touches the BCC field (not to or cc) However, I hit a couple of limitations
Thoughts on the best approach? Note that the query gets slow if you do sort_name LIKE 'c%' OR email LIKE 'c%' & you really need a union |
The function CRM_Contact_Page_AJAX::getContactEmail is one of our earlier ajax attempts & this approach has been largely replaced with entity Reference fields. In order to switch over we need to bring Email.getlist api to parity which means 1) searching on sortname first, if less than 10 results on emails include emails 2) appropriate respect for includeWildCardInName (this should already be in the generic getlist) 3) filter out on_hold, is_deceased, do_not_email 4) acl support (should already be part of the api). The trickiest of these to support is the first - because we need to avoid using a non-performant OR My current solution is the idea of a fallback field to search if the search results are less than the limit. in most cases this won't require a second query but when it does it should be fairly quick.
The function CRM_Contact_Page_AJAX::getContactEmail is one of our earlier ajax attempts & this approach has been largely replaced with entity Reference fields. In order to switch over we need to bring Email.getlist api to parity which means 1) searching on sortname first, if less than 10 results on emails include emails 2) appropriate respect for includeWildCardInName (this should already be in the generic getlist) 3) filter out on_hold, is_deceased, do_not_email 4) acl support (should already be part of the api). The trickiest of these to support is the first - because we need to avoid using a non-performant OR My current solution is the idea of a fallback field to search if the search results are less than the limit. in most cases this won't require a second query but when it does it should be fairly quick.
This is still overriden on the contact email.
I have duplicated this code rather than moving it in case any non-core code calls it. We can remove in a bit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Follow on cleanup commits from #16935
This affects code to send emails called from
Before
Emails task forms work. Code very nasty
After
Emails task forms work. Code moving towards readability
Technical Details
I'll keep adding to this & likely spin stuff off if #16935 is merged (unless someone wants to review / merge this)
Comments