CRM-20719 : Show warning on system status page if reply_id for mailing is not set to any default #10496
Conversation
jitendrapurohit
commented
Jun 12, 2017
•
- CRM-20719: Show warning on system status page if reply_id for mailing is not set to any default.
|
I'm taking a look at this now |
|
Hi @jitendrapurohit - In testing I was able to replicate the issue described in JIRA: I went to 'Admin -> CiviMail -> Headers, Footers, and Automated Messages' and disabled 'default' for the type 'Reply'. Then I started a new Mailing and filled in all required fields. The 'next' button was disabled with no user feedback. I also noticed the same behavior with 'OptOut', 'Resubscribe', 'Subscribe', and 'Unsubscribe' and I noticed the same behavior when any of these records were disabled entirely as well, not just when they weren't set as 'default'. I agree that a user should get some sort of feedback as to why they can't continue with their mailing form, but when I checked out the patch I wasn't able to see any change in behavior. Can you take a look at this again to see if it's still working for you and think about generalizing the feedback provided for the cases above? |
|
Hi @elisseck. Thank you for testing. The warning message is displayed on the system status page - CiviMail assigns the value for this field at the initial loading of the mailing screen so I think its more helpful for the user to know this requirement in advance. Below is the screenshot which still works fine for me on system status page.
|
CRM/Utils/Check/Component/Env.php
Outdated
| ts('Reply Auto Responder is not set to any default value in <a href=%1>Headers, Footers, and Automated Messages</a>. This will disable the submit operation on any mailing created from CiviMail.', array(1 => CRM_Utils_System::url('civicrm/admin/component', 'reset=1'))), | ||
| ts('No Default value for Auto Responder.'), | ||
| \Psr\Log\LogLevel::WARNING, | ||
| 'fa-server' |
There was a problem hiding this comment.
@agh1 thinks that the reply icon "fa-reply" would be more fitting for this functionality instead of 'fa-server'
There was a problem hiding this comment.
Updated the PR to use fa-reply icon in the display. Thanks 👍
|
I was able to test this on a local build. The "No Default value for Auto Responder" message popped up on the status page when I made the Auto-responder not default or disabled it all together. See images below:
@agh1 suggested that the 'fa-reply' icon might be a better fit then the 'fa-server' icon, otherwise looks good on our end. Great addition. |
9ad60df to
8819d49
Compare
|
Thank you for changing the icon! I think this is ready to be merged. |
CRM/Utils/Check/Component/Env.php
Outdated
| if (!CRM_Mailing_PseudoConstant::defaultComponent('Reply', '')) { | ||
| $messages[] = new CRM_Utils_Check_Message( | ||
| __FUNCTION__, | ||
| ts('Reply Auto Responder is not set to any default value in <a href=%1>Headers, Footers, and Automated Messages</a>. This will disable the submit operation on any mailing created from CiviMail.', array(1 => CRM_Utils_System::url('civicrm/admin/component', 'reset=1'))), |
There was a problem hiding this comment.
I think this html will be malformed because of lack of quotes around the value of href. You could stick the quotes in and that would fix it, but while we're at it, my personal preference is to supply all the attributes of the tag in the argument, so that the tag simply reads <a %1> and the 2nd param to ts would be [1 => 'href="' . CRM_Utils_System::url('civicrm/admin/component', 'reset=1') . '"']. The reason I prefer this syntax is that the translatable string remains the same even if we add other attributes to the html such as 'target="_blank"'. No need to re-translate it into 50 languages just due to html changes.
There was a problem hiding this comment.
Updated the PR to use the anchor tag as mentioned above. Thanks @colemanw 👍
8819d49 to
765e99a
Compare