Merge branch 'refactor-compose-command'

2 files changed, 141 insertions, 119 deletions

diff --git a/alot/commands/globals.py b/alot/commands/globals.py
index cd2e9512..c1e8d3cf 100644
--- a/alot/commands/globals.py
+++ b/alot/commands/globals.py
@@ -779,58 +779,34 @@ class ComposeCommand(Command):
         self.encrypt = encrypt
         self.tags = tags
 
-    async def apply(self, ui):
-        if self.envelope is None:
-            if self.rest:
-                if self.rest.startswith('mailto'):
-                    self.envelope = mailto_to_envelope(self.rest)
-                else:
-                    self.envelope = Envelope()
-                    self.envelope.add('To', self.rest)
-            else:
-                self.envelope = Envelope()
-        if self.template is not None:
-            # get location of tempsdir, containing msg templates
-            tempdir = settings.get('template_dir')
-
-            path = os.path.expanduser(self.template)
-            if not os.path.dirname(path):  # use tempsdir
-                if not os.path.isdir(tempdir):
-                    ui.notify('no templates directory: %s' % tempdir,
-                              priority='error')
-                    return
-                path = os.path.join(tempdir, path)
+    class ApplyError(Exception):
+        pass
 
-            if not os.path.isfile(path):
-                ui.notify('could not find template: %s' % path,
-                          priority='error')
-                return
-            try:
-                with open(path, 'rb') as f:
-                    template = helper.try_decode(f.read())
-                self.envelope.parse_template(template)
-            except Exception as e:
-                ui.notify(str(e), priority='error')
-                return
+    def _get_template(self, ui):
+        # get location of tempsdir, containing msg templates
+        tempdir = settings.get('template_dir')
 
-        # set forced headers
-        for key, value in self.headers.items():
-            self.envelope.add(key, value)
+        path = os.path.expanduser(self.template)
+        if not os.path.dirname(path):  # use tempsdir
+            if not os.path.isdir(tempdir):
+                ui.notify('no templates directory: %s' % tempdir,
+                          priority='error')
+                raise self.ApplyError()
+            path = os.path.join(tempdir, path)
 
-        # set forced headers for separate parameters
-        if self.sender:
-            self.envelope.add('From', self.sender)
-        if self.subject:
-            self.envelope.add('Subject', self.subject)
-        if self.to:
-            self.envelope.add('To', ','.join(self.to))
-        if self.cc:
-            self.envelope.add('Cc', ','.join(self.cc))
-        if self.bcc:
-            self.envelope.add('Bcc', ','.join(self.bcc))
-        if self.tags:
-            self.envelope.tags = [t for t in self.tags.split(',') if t]
+        if not os.path.isfile(path):
+            ui.notify('could not find template: %s' % path,
+                      priority='error')
+            raise self.ApplyError()
+        try:
+            with open(path, 'rb') as f:
+                template = helper.try_decode(f.read())
+            self.envelope.parse_template(template)
+        except Exception as e:
+            ui.notify(str(e), priority='error')
+            raise self.ApplyError()
 
+    async def _get_sender_details(self, ui):
         # find out the right account, if possible yet
         account = self.envelope.account
         if account is None:
@@ -872,7 +848,8 @@ class ComposeCommand(Command):
         if self.envelope.account is None:
             self.envelope.account = account
 
-        # add signature
+    async def _set_signature(self, ui):
+        account = self.envelope.account
         if not self.omit_signature and account.signature:
             logging.debug('has signature')
             sig = os.path.expanduser(account.signature)
@@ -894,10 +871,48 @@ class ComposeCommand(Command):
                           priority='error')
                 if (await ui.choice('send without signature?', 'yes',
                                     'no')) == 'no':
-                    return
+                    raise self.ApplyError
 
-        # Figure out whether we should GPG sign messages by default
-        # and look up key if so
+    async def apply(self, ui):
+        try:
+            await self.__apply(ui)
+        except self.ApplyError:
+            return
+
+    def _get_account(self, ui):
+        # find out the right account
+        sender = self.envelope.get('From')
+        _, addr = email.utils.parseaddr(sender)
+        try:
+            account = settings.get_account_by_address(addr)
+        except NoMatchingAccount:
+            msg = 'Cannot compose mail - no account found for `%s`' % addr
+            logging.error(msg)
+            ui.notify(msg, priority='error')
+            raise CommandCanceled()
+
+        if account is None:
+            accounts = settings.get_accounts()
+            if not accounts:
+                ui.notify('no accounts set.', priority='error')
+                raise self.ApplyError
+            account = accounts[0]
+
+        return account
+
+    def _set_envelope(self):
+        if self.envelope is None:
+            if self.rest:
+                if self.rest.startswith('mailto'):
+                    self.envelope = mailto_to_envelope(self.rest)
+                else:
+                    self.envelope = Envelope()
+                    self.envelope.add('To', self.rest)
+            else:
+                self.envelope = Envelope()
+
+    def _set_gpg_sign(self, ui):
+        account = self.envelope.account
         if account.sign_by_default:
             if account.gpg_key:
                 self.envelope.sign = account.sign_by_default
@@ -908,7 +923,8 @@ class ComposeCommand(Command):
                 logging.warning(msg)
                 ui.notify(msg, priority='error')
 
-        # get missing To header
+    async def _set_to(self, ui):
+        account = self.envelope.account
         if 'To' not in self.envelope.headers:
             allbooks = not settings.get('complete_matching_abook_only')
             logging.debug(allbooks)
@@ -925,6 +941,43 @@ class ComposeCommand(Command):
             ui.recipienthistory.append(to)
             self.envelope.add('To', to)
 
+    async def _set_gpg_encrypt(self, ui):
+        account = self.envelope.account
+        if self.encrypt or account.encrypt_by_default == u"all":
+            logging.debug("Trying to encrypt message because encrypt=%s and "
+                          "encrypt_by_default=%s", self.encrypt,
+                          account.encrypt_by_default)
+            await update_keys(ui, self.envelope, block_error=self.encrypt)
+        elif account.encrypt_by_default == u"trusted":
+            logging.debug("Trying to encrypt message because "
+                          "account.encrypt_by_default=%s",
+                          account.encrypt_by_default)
+            await update_keys(ui, self.envelope, block_error=self.encrypt,
+                              signed_only=True)
+        else:
+            logging.debug("No encryption by default, encrypt_by_default=%s",
+                          account.encrypt_by_default)
+
+    def _set_base_attributes(self):
+        # set forced headers
+        for key, value in self.headers.items():
+            self.envelope.add(key, value)
+
+        # set forced headers for separate parameters
+        if self.sender:
+            self.envelope.add('From', self.sender)
+        if self.subject:
+            self.envelope.add('Subject', self.subject)
+        if self.to:
+            self.envelope.add('To', ','.join(self.to))
+        if self.cc:
+            self.envelope.add('Cc', ','.join(self.cc))
+        if self.bcc:
+            self.envelope.add('Bcc', ','.join(self.bcc))
+        if self.tags:
+            self.envelope.tags = [t for t in self.tags.split(',') if t]
+
+    async def _set_subject(self, ui):
         if settings.get('ask_subject') and \
                 'Subject' not in self.envelope.headers:
             subject = await ui.prompt('Subject')
@@ -934,6 +987,7 @@ class ComposeCommand(Command):
 
             self.envelope.add('Subject', subject)
 
+    async def _set_compose_tags(self, ui):
         if settings.get('compose_ask_tags'):
             comp = TagsCompleter(ui.dbman)
             tags = ','.join(self.tags) if self.tags else ''
@@ -944,27 +998,37 @@ class ComposeCommand(Command):
 
             self.envelope.tags = tags
 
+    def _set_attachments(self):
         if self.attach:
             for gpath in self.attach:
                 for a in glob.glob(gpath):
                     self.envelope.attach(a)
                     logging.debug('attaching: %s', a)
 
+    async def __apply(self, ui):
+        self._set_envelope()
+        if self.template is not None:
+            self._get_template(ui)
+        # Set headers and tags
+        self._set_base_attributes()
+        # set account and missing From header
+        await self._get_sender_details(ui)
+
+        # add signature
+        await self._set_signature(ui)
+        # Figure out whether we should GPG sign messages by default
+        # and look up key if so
+        self._set_gpg_sign(ui)
+        # get missing To header
+        await self._set_to(ui)
+        # Set subject
+        await self._set_subject(ui)
+        # Add additional tags
+        await self._set_compose_tags(ui)
+        # Set attachments
+        self._set_attachments()
         # set encryption if needed
-        if self.encrypt or account.encrypt_by_default == u"all":
-            logging.debug("Trying to encrypt message because encrypt=%s and "
-                          "encrypt_by_default=%s", self.encrypt,
-                          account.encrypt_by_default)
-            await update_keys(ui, self.envelope, block_error=self.encrypt)
-        elif account.encrypt_by_default == u"trusted":
-            logging.debug("Trying to encrypt message because "
-                          "account.encrypt_by_default=%s",
-                          account.encrypt_by_default)
-            await update_keys(ui, self.envelope, block_error=self.encrypt,
-                              signed_only=True)
-        else:
-            logging.debug("No encryption by default, encrypt_by_default=%s",
-                          account.encrypt_by_default)
+        await self._set_gpg_encrypt(ui)
 
         cmd = commands.envelope.EditCommand(envelope=self.envelope,
                                             spawn=self.force_spawn,
@@ -972,6 +1036,7 @@ class ComposeCommand(Command):
         await ui.apply_command(cmd)
 
 
+
 @registerCommand(
     MODE, 'move', help='move focus in current buffer',
     arguments=[
diff --git a/tests/commands/test_global.py b/tests/commands/test_global.py
index 9b026e6a..9ef3d957 100644
--- a/tests/commands/test_global.py
+++ b/tests/commands/test_global.py
@@ -55,56 +55,37 @@ class TestComposeCommand(unittest.TestCase):
         account.signature = None
         return account
 
-    @utilities.async_test
-    async def test_apply_sign_by_default_okay(self):
+    def test_set_gpg_sign_by_default_okay(self):
         envelope = self._make_envelope_mock()
         envelope.account = self._make_account_mock()
         cmd = g_commands.ComposeCommand(envelope=envelope)
 
-        with mock.patch('alot.commands.globals.settings.get_addressbooks',
-                        mock.Mock(side_effect=Stop)):
-            try:
-                await cmd.apply(mock.Mock())
-            except Stop:
-                pass
+        cmd._set_gpg_sign(mock.Mock())
 
         self.assertTrue(envelope.sign)
         self.assertIs(envelope.sign_key, mock.sentinel.gpg_key)
 
-    @utilities.async_test
-    async def test_apply_sign_by_default_false_doesnt_set_key(self):
+    def test_set_gpg_sign_by_default_false_doesnt_set_key(self):
         envelope = self._make_envelope_mock()
         envelope.account = self._make_account_mock(sign_by_default=False)
         cmd = g_commands.ComposeCommand(envelope=envelope)
 
-        with mock.patch('alot.commands.globals.settings.get_addressbooks',
-                        mock.Mock(side_effect=Stop)):
-            try:
-                await cmd.apply(mock.Mock())
-            except Stop:
-                pass
+        cmd._set_gpg_sign(mock.Mock())
 
         self.assertFalse(envelope.sign)
         self.assertIs(envelope.sign_key, None)
 
-    @utilities.async_test
-    async def test_apply_sign_by_default_but_no_key(self):
+    def test_set_gpg_sign_by_default_but_no_key(self):
         envelope = self._make_envelope_mock()
         envelope.account = self._make_account_mock(gpg_key=None)
         cmd = g_commands.ComposeCommand(envelope=envelope)
 
-        with mock.patch('alot.commands.globals.settings.get_addressbooks',
-                        mock.Mock(side_effect=Stop)):
-            try:
-                await cmd.apply(mock.Mock())
-            except Stop:
-                pass
+        cmd._set_gpg_sign(mock.Mock())
 
         self.assertFalse(envelope.sign)
         self.assertIs(envelope.sign_key, None)
 
-    @utilities.async_test
-    async def test_decode_template_on_loading(self):
+    def test_get_template_decode(self):
         subject = u'This is a täßϑ subject.'
         to = u'recipient@mail.com'
         _from = u'foo.bar@mail.fr'
@@ -116,38 +97,14 @@ class TestComposeCommand(unittest.TestCase):
         self.addCleanup(os.unlink, f.name)
 
         cmd = g_commands.ComposeCommand(template=f.name)
-
-        # Crutch to exit the giant `apply` method early.
-        with mock.patch(
-                'alot.commands.globals.settings.get_accounts',
-                mock.Mock(side_effect=Stop)):
-            try:
-                await cmd.apply(mock.Mock())
-            except Stop:
-                pass
+        cmd._set_envelope()
+        cmd._get_template(mock.Mock())
 
         self.assertEqual({'To': [to],
                           'From': [_from],
                           'Subject': [subject]}, cmd.envelope.headers)
         self.assertEqual(body, cmd.envelope.body)
 
-    @utilities.async_test
-    async def test_single_account_no_from(self):
-        # issue #1277
-        envelope = self._make_envelope_mock()
-        del envelope.headers['From']
-        envelope.account = self._make_account_mock()
-        envelope.account.realname = "foo"
-        envelope.account.address = 1  # maybe this should be a real Address?
-        cmd = g_commands.ComposeCommand(envelope=envelope)
-
-        with mock.patch('alot.commands.globals.settings.get_addressbooks',
-                        mock.Mock(side_effect=Stop)):
-            try:
-                await cmd.apply(mock.Mock())
-            except Stop:
-                pass
-
 
 class TestExternalCommand(unittest.TestCase):