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#295 - Allow default 'from' email to be set in New email form #12603

Merged
merged 1 commit into from
Aug 9, 2018

Conversation

jitendrapurohit
Copy link
Contributor

@jitendrapurohit jitendrapurohit commented Jul 31, 2018

Overview

Allow default from email address to be set as default on the "New Email" form.

Before

Default From address is not loaded as default on the email form.

SE Post - https://civicrm.stackexchange.com/questions/25562/why-does-the-default-from-email-address-not-show-as-the-first-option-when-usin

After

If Allow Mail to be sent from logged in contact's email address is disabled in settings, default from address is loaded on the form.


Gitlab issue - https://lab.civicrm.org/dev/core/issues/295

@civibot
Copy link

civibot bot commented Jul 31, 2018

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@mattwire @seamuslee001 I feel like you have been involved in this

Copy link
Contributor

@mattwire mattwire left a comment

Choose a reason for hiding this comment

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

@jitendrapurohit I've reproduced the issue and agree we should fix it. But can you modify your code per my comments?

$form->_fromEmails = $fromEmailValues;
if (!Civi::settings()->get('allow_mail_from_logged_in_contact')) {
$defaults['from_email_address'] = key(CRM_Core_OptionGroup::values('from_email_address', TRUE, NULL, NULL, ' AND is_default = 1'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, we shouldn't be calling CRM_Core_OptionGroup::values directly here. I think the correct function to use would be CRM_Core_BAO_Domain::getNameAndEmail() from here: https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/BAO/Domain.php#L172

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mattwire Updated the PR with the above function replacement.

@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

@mattwire can you recheck

@mattwire
Copy link
Contributor

mattwire commented Aug 9, 2018

@eileenmcnaughton Ready to merge

@eileenmcnaughton
Copy link
Contributor

Thanks @mattwire @jitendrapurohit

@eileenmcnaughton eileenmcnaughton merged commit 9c13ddf into civicrm:master Aug 9, 2018
@jitendrapurohit jitendrapurohit deleted the core-295 branch August 27, 2018 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants