Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-14095

make batch emails in test all come from mailing.list.batch

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-2051Make StyleableEmailContentServiceImpl the default email content service implementation
      KULRICE-6964Update Rice default From email address
      KULRICE-13600Come up with a way to effectively unit test immediate email content, daily, and weekly email reminders
      KULRICE-6965Update Rice Documentation references to default From email address
      KULRICE-1159user / role cleanup batch job
      KULRICE-5525emails not sent on edoclite acknowledge emails
      KULRICE-7784Add application code to the subject of Incident Reports and other email from Kauli (i.e. email from batch jobs, etc)
      KULRICE-8422Fix upgrade script issue: Update Rice default From email address
      KULRICE-4937Track things coming in from the community in a better way.
      KULRICE-4059Have a config param that turns emails on or off
    • Epic Link:
    • Application Requirement:
      KFS
    • Sprint:
      Middleware 2.5.2 Sprint 4
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes
    • Story Points:
      2

      Description

      As per a discussion with Claus and Jonathan this morning, we'd like to also add the requirements of KFSCNTRB-1796 to this - which is to say, in non-production environments, set the from address on the e-mail to the Non-Production notification list (see the last comment of the jira).

      The naming should be "from email address" not a mailing list

        Issue Links

          Activity

          Hide
          Jonathan Keller added a comment -
          mailService Definition
          <bean id="mailService" class="org.kuali.rice.krad.service.impl.MailServiceImpl">
            <property name="mailer" ref="mailer"/>
            <property name="batchMailingList" value="${mailing.list.batch}"/>
            <property name="nonProductionNotificationMailingList" value="${nonproduction.notification.mailing.list}" />
            <property name="realNotificationsEnabled" value="${real.notifications.enabled}" />
          </bean>
          
          Show
          Jonathan Keller added a comment - mailService Definition <bean id= "mailService" class= "org.kuali.rice.krad.service.impl.MailServiceImpl" > <property name= "mailer" ref= "mailer" /> <property name= "batchMailingList" value= "${mailing.list.batch}" /> <property name= "nonProductionNotificationMailingList" value= "${nonproduction.notification.mailing.list}" /> <property name= "realNotificationsEnabled" value= "${real.notifications.enabled}" /> </bean>
          Hide
          Jonathan Keller added a comment -

          If we are just going to take this issue at it's simplest, then all we are doing is to take the lines which look like this:

          mm.setFromAddress(message.getFromAddress());

          and set them to the batchMailingList value when isRealNotificationsEnabled() is false

          (And add the original from address to the logged information in the getNonProductionMessage() method.

          Show
          Jonathan Keller added a comment - If we are just going to take this issue at it's simplest, then all we are doing is to take the lines which look like this: mm.setFromAddress(message.getFromAddress()); and set them to the batchMailingList value when isRealNotificationsEnabled() is false (And add the original from address to the logged information in the getNonProductionMessage() method.
          Hide
          James Smith added a comment -

          That sounds close to what Philip was asking for on KFSCNTRB-1796, so I believe that would work for us.

          Show
          James Smith added a comment - That sounds close to what Philip was asking for on KFSCNTRB-1796 , so I believe that would work for us.
          Hide
          Jonathan Keller added a comment -

          For the record, I really disagree with having this "nonproduction" setup. You should not have non production data in your production environment. If you need to alter the behavior of the mail service, then you should use an alternate version of the mail service by overriding the mailService bean. (Which is included in non-production environments per the config properties as real.notifications.enabled would be.)

          (In UCD's case, we wanted to prevent any emails from being sent - we have a logger which puts them into their own file.)

          public class DevelopmentMailServiceImpl extends MailServiceImpl {
              private static final org.apache.log4j.Logger LOG = org.apache.log4j.Logger.getLogger(DevelopmentMailServiceImpl.class);
          
              @Override
              public void sendMessage(MailMessage message) throws InvalidAddressException, MessagingException {
                  LOG.info( getMessageForLogs(message));
              }
          
              public static String getMessageForLogs( MailMessage message ) {
                  StringBuilder sb = new StringBuilder();
                  sb.append( '\n' );
                  sb.append( "*********************** EMAIL SEND *****************************\n");
                  sb.append( "FROM : " ).append( message.getFromAddress() ).append( '\n');
                  sb.append( "TO   : " ).append( message.getToAddresses() ).append( '\n');
                  sb.append( "CC   : " ).append( message.getCcAddresses() ).append( '\n');
                  sb.append( "BCC  : " ).append( message.getBccAddresses() ).append( '\n');
                  sb.append( "SUBJECT : " ).append( message.getSubject() ).append( '\n');
                  sb.append( "MESSAGE : \n" ).append( message.getMessage() ).append( '\n');
                  sb.append( "*********************** END EMAIL  *****************************\n");
                  return sb.toString();
              }
          }
          
          Show
          Jonathan Keller added a comment - For the record, I really disagree with having this "nonproduction" setup. You should not have non production data in your production environment. If you need to alter the behavior of the mail service, then you should use an alternate version of the mail service by overriding the mailService bean. (Which is included in non-production environments per the config properties as real.notifications.enabled would be.) (In UCD's case, we wanted to prevent any emails from being sent - we have a logger which puts them into their own file.) public class DevelopmentMailServiceImpl extends MailServiceImpl { private static final org.apache.log4j.Logger LOG = org.apache.log4j.Logger.getLogger(DevelopmentMailServiceImpl.class); @Override public void sendMessage(MailMessage message) throws InvalidAddressException, MessagingException { LOG.info( getMessageForLogs(message)); } public static String getMessageForLogs( MailMessage message ) { StringBuilder sb = new StringBuilder(); sb.append( '\n' ); sb.append( "*********************** EMAIL SEND *****************************\n" ); sb.append( "FROM : " ).append( message.getFromAddress() ).append( '\n'); sb.append( "TO : " ).append( message.getToAddresses() ).append( '\n'); sb.append( "CC : " ).append( message.getCcAddresses() ).append( '\n'); sb.append( "BCC : " ).append( message.getBccAddresses() ).append( '\n'); sb.append( "SUBJECT : " ).append( message.getSubject() ).append( '\n'); sb.append( "MESSAGE : \n" ).append( message.getMessage() ).append( '\n'); sb.append( "*********************** END EMAIL *****************************\n" ); return sb.toString(); } }
          Hide
          James Smith added a comment -

          I'll bring up in our MAM tomorrow, though I suspect that is a longer term change.

          Show
          James Smith added a comment - I'll bring up in our MAM tomorrow, though I suspect that is a longer term change.
          Hide
          James Smith added a comment -

          Hi Jonathan. Kevin, Bryan, Ailish and I discussed the separate mail beans in the MAM. In the discussion we remembered a couple of issues with using flags versus different implementations:

          • we've run into issues where creating multiple beans for the mail services (for instance, to handle attachments) led to a lot of inheritance headaches. (This is not to say that the different bean idea is bad, just that it includes its own set of trade-offs).
          • there were some very strong feelings on the team in relation to production/non-production code parity

          Ultimately at this point, though, using a different bean for non-production mail service stuff would have more impact for implementers than would just going ahead with flags. So if we could go ahead with the "setFromAddress" solution for the time being, we'd be really grateful. Thanks Jonathan!

          Show
          James Smith added a comment - Hi Jonathan. Kevin, Bryan, Ailish and I discussed the separate mail beans in the MAM. In the discussion we remembered a couple of issues with using flags versus different implementations: we've run into issues where creating multiple beans for the mail services (for instance, to handle attachments) led to a lot of inheritance headaches. (This is not to say that the different bean idea is bad, just that it includes its own set of trade-offs). there were some very strong feelings on the team in relation to production/non-production code parity Ultimately at this point, though, using a different bean for non-production mail service stuff would have more impact for implementers than would just going ahead with flags. So if we could go ahead with the "setFromAddress" solution for the time being, we'd be really grateful. Thanks Jonathan!
          Hide
          Sona Sona (Inactive) added a comment -

          Hi Jonathan,
          Out of curiosity - what is the difference between mailing.list.batch and nonProductionNotificationMailingList?

          Show
          Sona Sona (Inactive) added a comment - Hi Jonathan, Out of curiosity - what is the difference between mailing.list.batch and nonProductionNotificationMailingList?
          Hide
          Jonathan Keller added a comment -

          The latter is used in place of any provided (to a service call) TO or CC addresses when the feature is turned on (property or parameter - don't remember) and the environment code does not match the production environment code. (usually prd)

          Show
          Jonathan Keller added a comment - The latter is used in place of any provided (to a service call) TO or CC addresses when the feature is turned on (property or parameter - don't remember) and the environment code does not match the production environment code. (usually prd)
          Hide
          Claus Niesen added a comment -

          This change was rolled back. See KULRICE-14252 for instructions to reapply.

          Show
          Claus Niesen added a comment - This change was rolled back. See KULRICE-14252 for instructions to reapply.

            People

            • Assignee:
              Sona Sona (Inactive)
              Reporter:
              James Smith
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - Not Specified
                Not Specified
                Logged:
                Time Spent - 2 minutes
                2m

                  Agile

                    Structure Helper Panel