-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Replace expensive query with cheaper one #21426
Conversation
(Standard links)
|
well test failure is related @eileenmcnaughton |
@seamuslee001 it passed in isolation - my guess is there is a 'spare' Anthony Anderson in the db :-) Will check |
45a8ed2
to
11e8cfc
Compare
Rather than retrieve everything with a legacy function, just get what we want.... Note I commented on how silly I think it is but no change to outcome
11e8cfc
to
965ff8f
Compare
// this based on 'the first email in the db that matches'. | ||
// when we likely have the contact id. OTOH people probably barely | ||
// use preferredMailFormat these days - the good fight against html | ||
// emails was lost a decade ago... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would fully support ripping out 'preferred_mail_format'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmcclelland given you just put up a PR about importing this field I'm thinking you might still see value in it? I am a bit confused why we wouldn't always send 'BOTH' and let the mail client decide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think sending in non-html ever took off. I'd be in favour of obsoleting then later removing this.
My recent pull request was on |
Overview
Replace expensive query with cheaper one
Before
call so
apiQuery
- retrieves all fields across several tablesAfter
Apiv4 field - retrieves 1 field
Technical Details
Rather than retrieve everything with a legacy function, just get what we want....
Note I commented on how silly I think it is but no change to outcome
Comments