Refactor std.email action

Closes-bug: #1392652

Change-Id: I308b28aae758b2b99b5ae5f39f8f74743544f00a

mistral/actions/std_actions.py

@@ -193,19 +193,19 @@ class MistralHTTPAction(HTTPAction):
 
 
 class SendEmailAction(base.Action):
-    def __init__(self, params, settings):
+    def __init__(self, from_addr, to_addrs, smtp_server,
+                 smtp_password, subject=None, body=None):
         # TODO(dzimine): validate parameters
 
         # Task invocation parameters.
-        self.to = ', '.join(params['to'])
-        self.subject = params['subject']
-        self.body = params['body']
+        self.to = ', '.join(to_addrs)
+        self.subject = subject or "<No subject>"
+        self.body = body
 
         # Action provider settings.
-        self.smtp_server = settings['smtp_server']
-        self.sender = settings['from']
-        self.password = settings['password'] \
-            if 'password' in settings else None
+        self.smtp_server = smtp_server
+        self.sender = from_addr
+        self.password = smtp_password
 
     def run(self):
         LOG.info("Sending email message "

mistral/tests/unit/actions/test_std_email_action.py

@@ -49,79 +49,90 @@ class SendEmailActionTest(base.BaseTest):
 
     def setUp(self):
         super(SendEmailActionTest, self).setUp()
-        self.params = {
-            'to': ["dz@example.com, deg@example.com", "xyz@example.com"],
-            'subject': "Multi word subject с русскими буквами",
-            'body': "short multiline\nbody\nc русскими буквами",
-        }
-        self.settings = {
-            'smtp_server': 'mail.example.com:25',
-            'from': "bot@example.com",
-        }
-        self.to_addrs = ', '.join(self.params['to'])
+        self.to_addrs = ["dz@example.com, deg@example.com", "xyz@example.com"]
+        self.subject = "Multi word subject с русскими буквами"
+        self.body = "short multiline\nbody\nc русскими буквами"
+
+        self.smtp_server = 'mail.example.com:25'
+        self.from_addr = "bot@example.com"
+
+        self.to_addrs_str = ", ".join(self.to_addrs)
 
     @testtools.skipIf(not LOCAL_SMTPD, "Setup local smtpd to run it")
     def test_send_email_real(self):
-        action = std.SendEmailAction(self.params, self.settings)
+        action = std.SendEmailAction(
+            self.from_addr, self.to_addrs,
+            self.smtp_server, None, self.subject, self.body
+        )
         action.run()
 
     @testtools.skipIf(not REMOTE_SMTP, "Configure Remote SMTP to run it")
     def test_with_password_real(self):
-        self.params['to'] = ["dz@stackstorm.com"]
-        self.settings = {
-            'smtp_server': 'smtp.gmail.com:587',
-            'from': "username@gmail.com",
-            'password': 'secret'
-        }
+        self.to_addrs = ["dz@stackstorm.com"]
+        self.smtp_server = 'mail.example.com:25'
+        self.from_addr = "bot@example.com"
+        self.smtp_password = 'secret'
 
-        action = std.SendEmailAction(self.params, self.settings)
+        action = std.SendEmailAction(
+            self.from_addr, self.to_addrs,
+            self.smtp_server, self.smtp_password, self.subject, self.body
+        )
 
         action.run()
 
     @mock.patch('smtplib.SMTP')
     def test_send_email(self, smtp):
-        action = std.SendEmailAction(self.params, self.settings)
+        action = std.SendEmailAction(
+            self.from_addr, self.to_addrs,
+            self.smtp_server, None, self.subject, self.body
+        )
 
         action.run()
 
-        smtp.assert_called_once_with(self.settings['smtp_server'])
+        smtp.assert_called_once_with(self.smtp_server)
 
         sendmail = smtp.return_value.sendmail
 
         self.assertTrue(sendmail.called, "should call sendmail")
         self.assertEqual(
-            sendmail.call_args[1]['from_addr'], self.settings['from'])
+            sendmail.call_args[1]['from_addr'], self.from_addr)
         self.assertEqual(
-            sendmail.call_args[1]['to_addrs'], self.to_addrs)
+            sendmail.call_args[1]['to_addrs'], self.to_addrs_str)
 
         message = parser.Parser().parsestr(sendmail.call_args[1]['msg'])
 
-        self.assertEqual(self.settings['from'], message['from'])
-        self.assertEqual(self.to_addrs, message['to'])
-        self.assertEqual(self.params['subject'], message['subject'])
-        self.assertEqual(self.params['body'], message.get_payload())
+        self.assertEqual(self.from_addr, message['from'])
+        self.assertEqual(self.to_addrs_str, message['to'])
+        self.assertEqual(self.subject, message['subject'])
+        self.assertEqual(self.body, message.get_payload())
 
     @mock.patch('smtplib.SMTP')
     def test_with_password(self, smtp):
-        self.settings['password'] = "secret"
+        self.smtp_password = "secret"
 
-        action = std.SendEmailAction(self.params, self.settings)
+        action = std.SendEmailAction(
+            self.from_addr, self.to_addrs,
+            self.smtp_server, self.smtp_password, self.subject, self.body
+        )
 
         action.run()
 
         smtpmock = smtp.return_value
         calls = [mock.call.ehlo(), mock.call.starttls(), mock.call.ehlo(),
-                 mock.call.login(self.settings['from'],
-                                 self.settings['password'])]
+                 mock.call.login(self.from_addr,
+                                 self.smtp_password)]
 
         smtpmock.assert_has_calls(calls)
         self.assertTrue(smtpmock.sendmail.called, "should call sendmail")
 
     @mock.patch('mistral.actions.std_actions.LOG')
     def test_exception(self, log):
-        self.params['smtp_server'] = "wrong host"
+        self.smtp_server = "wrong host"
 
-        action = std.SendEmailAction(self.params, self.settings)
+        action = std.SendEmailAction(
+            self.from_addr, self.to_addrs,
+            self.smtp_server, None, self.subject, self.body
+        )
 
         try:
             action.run()

mistral/tests/unit/services/test_action_manager.py

@@ -29,7 +29,13 @@ class ActionManagerTest(base.DbTestCase):
         )
 
         self.assertEqual(http_action_input, std_http.input)
-        self.assertEqual("params, settings", std_email.input)
+
+        std_email_input = (
+            "from_addr, to_addrs, smtp_server, "
+            "smtp_password, subject=None, body=None"
+        )
+
+        self.assertEqual(std_email_input, std_email.input)
 
     def test_action_description(self):
         std_http = db_api.get_action("std.http")

mistral/tests/unit/utils/test_inpect_utils.py

@@ -32,7 +32,7 @@ class InspectUtilsTest(base.BaseTest):
         self.assertEqual(http_action_params, parameters_str)
 
     def test_get_parameters_str_all_mandatory(self):
-        action_class = std_actions.SendEmailAction
+        action_class = std_actions.SSHAction
         parameters_str = i_u.get_arg_list_as_str(action_class.__init__)
 
-        self.assertEqual("params, settings", parameters_str)
+        self.assertEqual("cmd, host, username, password", parameters_str)