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

referenece Organization Address and Contact Info, not FROM Email Addresses in help SMTP text (lab-879) #14055

Closed
wants to merge 2 commits into from

Conversation

michaelmcandrew
Copy link
Contributor

@michaelmcandrew michaelmcandrew commented Apr 15, 2019

@civibot
Copy link

civibot bot commented Apr 15, 2019

(Standard links)

@civibot civibot bot added the master label Apr 15, 2019
@michaelmcandrew
Copy link
Contributor Author

any idea why the conflicts cannot be resolved? seems weird to me.

@@ -102,7 +102,7 @@ public function postProcess() {

if (!$domainEmailAddress || $domainEmailAddress == '[email protected]') {
$fixUrl = CRM_Utils_System::url("civicrm/admin/domain", 'action=update&reset=1');
CRM_Core_Error::fatal(ts('The site administrator needs to enter a valid \'FROM Email Address\' in <a href="%1">Administer CiviCRM &raquo; Communications &raquo; FROM Email Addresses</a>. The email address used may need to be a valid mail account with your email service provider.', [1 => $fixUrl]));
CRM_Core_Error::fatal(ts('The site administrator needs to enter a valid email address in <a href="%1">Administer CiviCRM &raquo; Communications &raquo; Organization Address and Contact Info</a>. The email address used may need to be a valid mail account with your email service provider.', array(1 => $fixUrl)));
Copy link
Contributor

Choose a reason for hiding this comment

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

@michaelmcandrew This fatal should probably be replaced with CRM_Core_Error::statusBounce as it's pretty ugly triggering fatal when you get a setting wrong.

@colemanw
Copy link
Member

@michaelmcandrew could you please rebase out the merge-commit from this PR?

@yashodha
Copy link
Contributor

test this please

1 similar comment
@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

Replaced with #14329

@fkohrt
Copy link
Contributor

fkohrt commented Aug 29, 2019

@michaelmcandrew @eileenmcnaughton I believe it's the other way round: The text was correct, but linked to the wrong URL. Reason: The SMTP setup checks if a valid FROM address is set up before it allows to send a test email. If no valid FROM address is found, it triggers the message. The organizational contact email address has nothing to do with this and setting it does not help with sending the test email. The correct URL should (probably) be something like /civicrm/admin/options/from_email_address?reset=1&civicrmDestination=%2Fcivicrm%2Fadmin%2Fconfigtask%3Freset%3D1.

Honestly it's a problem with the configuration checklist as well as it does not make sense to configure the SMTP setup before the FROM address as the configuration checklists suggests.

@eileenmcnaughton
Copy link
Contributor

@fkohrt any chance you can do a PR to fix?

@fkohrt
Copy link
Contributor

fkohrt commented Aug 29, 2019

@eileenmcnaughton done with #15165 :)

But the order in the configuration checklist should be changed as well so that the From Address config precedes the SMTP setup.

seamuslee001 added a commit that referenced this pull request Sep 4, 2019
SMTP help text and URL are misleading to Organization Address and Contact Info instead of leading to From Email Addresses; see also #14055 and #14329
magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Oct 13, 2019
…tact Info instead of leading to From Email Addresses; see also civicrm#14055 and civicrm#14329
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.

6 participants