Merge pull request #166 from pixelated-project/fix_reply_to_all_recipients

Fix reply to all recipients

12 files changed, 249 insertions, 95 deletions

diff --git a/service/pixelated/adapter/mail.py b/service/pixelated/adapter/mail.py
index 949daa9e..e2a48062 100644
--- a/service/pixelated/adapter/mail.py
+++ b/service/pixelated/adapter/mail.py
@@ -214,7 +214,11 @@ class PixelatedMail(Mail):
 
     @property
     def headers(self):
-        _headers = {}
+        _headers = {
+            'To': [],
+            'Cc': [],
+            'Bcc': []
+        }
         hdoc_headers = self.hdoc.content['headers']
 
         for header in ['To', 'Cc', 'Bcc']:
@@ -226,11 +230,17 @@ class PixelatedMail(Mail):
 
         for header in ['From', 'Subject']:
             _headers[header] = hdoc_headers.get(header)
+
         _headers['Date'] = self._get_date()
+
         if self.parts and len(self.parts['alternatives']) > 1:
             _headers['content_type'] = 'multipart/alternative; boundary="%s"' % self.boundary
         elif self.hdoc.content['headers'].get('Content-Type'):
             _headers['content_type'] = hdoc_headers.get('Content-Type')
+
+        if hdoc_headers.get('Reply-To'):
+            _headers['Reply-To'] = hdoc_headers.get('Reply-To')
+
         return _headers
 
     def _get_date(self):
@@ -319,13 +329,23 @@ class PixelatedMail(Mail):
         return self.hdoc.content["headers"].get("OpenPGP", None) is not None
 
     def as_dict(self):
-        return {
-            'header': {k.lower(): v for k, v in self.headers.items()},
-            'ident': self.ident,
-            'tags': list(self.tags),
-            'status': list(self.status),
-            'security_casing': self.security_casing,
-            'body': self.body,
-            'mailbox': self.mailbox_name.lower(),
-            'attachments': self.parts['attachments'] if self.parts else []
-        }
+        dict_mail = {'header': {k.lower(): v for k, v in self.headers.items()},
+                     'ident': self.ident,
+                     'tags': list(self.tags),
+                     'status': list(self.status),
+                     'security_casing': self.security_casing,
+                     'body': self.body,
+                     'mailbox': self.mailbox_name.lower(),
+                     'attachments': self.parts['attachments'] if self.parts else []}
+        dict_mail['replying'] = {'single': None, 'all': {'to-field': [], 'cc-field': []}}
+
+        sender_mail = self.headers.get('Reply-To', self.headers['From'])
+
+        recipients = [recipient for recipient in self.headers['To'] if recipient != InputMail.FROM_EMAIL_ADDRESS]
+        recipients.append(sender_mail)
+        ccs = [cc for cc in self.headers['Cc'] if cc != InputMail.FROM_EMAIL_ADDRESS]
+
+        dict_mail['replying']['single'] = sender_mail
+        dict_mail['replying']['all']['to-field'] = recipients
+        dict_mail['replying']['all']['cc-field'] = ccs
+        return dict_mail
diff --git a/service/pixelated/adapter/mail_service.py b/service/pixelated/adapter/mail_service.py
index 6c093b6d..722b9a29 100644
--- a/service/pixelated/adapter/mail_service.py
+++ b/service/pixelated/adapter/mail_service.py
@@ -83,3 +83,7 @@ class MailService:
 
     def drafts(self):
         raise NotImplementedError()
+
+    def reply_all_template(self, mail_id):
+        mail = self.mail(mail_id)
+        return mail.to_reply_template()
diff --git a/service/pixelated/config/app_factory.py b/service/pixelated/config/app_factory.py
index ede19e60..2021556d 100644
--- a/service/pixelated/config/app_factory.py
+++ b/service/pixelated/config/app_factory.py
@@ -62,13 +62,40 @@ def update_info_sync_and_index_partial(sync_info_controller, search_engine, mail
     return wrapper
 
 
+def _setup_routes(app, home_controller, mails_controller, tags_controller, features_controller, sync_info_controller,
+                  attachments_controller):
+    # mails
+    app.route('/mails', methods=['GET'])(mails_controller.mails)
+    app.route('/mail/<mail_id>/read', methods=['POST'])(mails_controller.mark_mail_as_read)
+    app.route('/mail/<mail_id>/unread', methods=['POST'])(mails_controller.mark_mail_as_unread)
+    app.route('/mails/unread', methods=['POST'])(mails_controller.mark_many_mail_unread)
+    app.route('/mails/read', methods=['POST'])(mails_controller.mark_many_mail_read)
+    app.route('/mail/<mail_id>', methods=['GET'])(mails_controller.mail)
+    app.route('/mail/<mail_id>/reply_all_template', methods=['GET'])(mails_controller.reply_all_template)
+    app.route('/mail/<mail_id>', methods=['DELETE'])(mails_controller.delete_mail)
+    app.route('/mails', methods=['DELETE'])(mails_controller.delete_mails)
+    app.route('/mails', methods=['POST'])(mails_controller.send_mail)
+    app.route('/mail/<mail_id>/tags', methods=['POST'])(mails_controller.mail_tags)
+    app.route('/mails', methods=['PUT'])(mails_controller.update_draft)
+    # tags
+    app.route('/tags', methods=['GET'])(tags_controller.tags)
+    # features
+    app.route('/features', methods=['GET'])(features_controller.features)
+    # sync info
+    app.route('/sync_info', methods=['GET'])(sync_info_controller.sync_info)
+    # attachments
+    app.route('/attachment/<attachment_id>', methods=['GET'])(attachments_controller.attachment)
+    # static
+    app.route('/', methods=['GET'], branch=True)(home_controller.home)
+
+
 def init_leap_session(app):
     try:
         leap_session = LeapSession.open(app.config['LEAP_USERNAME'],
                                         app.config['LEAP_PASSWORD'],
                                         app.config['LEAP_SERVER_NAME'])
-    except ConnectionError:
-        print("Can't connect to the requested provider")
+    except ConnectionError, error:
+        print("Can't connect to the requested provider", error)
         sys.exit(1)
     except LeapAuthException, e:
         print("Couldn't authenticate with the credentials provided %s" % e.message)
diff --git a/service/pixelated/controllers/mails_controller.py b/service/pixelated/controllers/mails_controller.py
index 3a2e0d3b..d84018dc 100644
--- a/service/pixelated/controllers/mails_controller.py
+++ b/service/pixelated/controllers/mails_controller.py
@@ -123,6 +123,10 @@ class MailsController:
         self._search_engine.index_mail(self._mail_service.mail(ident))
         return respond_json({'ident': ident}, request)
 
+    def reply_all_template(self, request, mail_id):
+        mail = self._mail_service.reply_all_template(mail_id)
+        return respond_json(mail, request)
+
     def _format_exception(self, exception):
         exception_info = map(str, list(exception.args))
         return '\n'.join(exception_info)
diff --git a/service/test/integration/drafts_test.py b/service/test/integration/drafts_test.py
index 2ba14dfd..7fd0a46b 100644
--- a/service/test/integration/drafts_test.py
+++ b/service/test/integration/drafts_test.py
@@ -14,6 +14,8 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
+import unittest
+
 from test.support.integration import *
 
 
diff --git a/service/test/integration/reply_test.py b/service/test/integration/reply_test.py
new file mode 100644
index 00000000..177cbb6b
--- /dev/null
+++ b/service/test/integration/reply_test.py
@@ -0,0 +1,42 @@
+#
+# Copyright (c) 2014 ThoughtWorks, Inc.
+#
+# Pixelated is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Pixelated is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
+
+import unittest
+from test.support.integration_helper import MailBuilder, SoledadTestBase
+from pixelated.adapter.mail import InputMail
+
+
+class ReplyTest(SoledadTestBase):
+
+    def setUp(self):
+        SoledadTestBase.setUp(self)
+
+    def tearDown(self):
+        SoledadTestBase.tearDown(self)
+
+    def test_get_provides_template_for_reply_all(self):
+        InputMail.FROM_EMAIL_ADDRESS = 'user@pixelated.org'
+        mail = MailBuilder().with_subject('some subject').with_to_addresses(['user@pixelated.org', 'another@pixelated.org']).with_ident(1).build_input_mail()
+        self.add_mail_to_inbox(mail)
+
+        mails = self.get_mails_by_tag('inbox')
+        self.assertNotIn('read', mails[0].status)
+
+        response = self.reply_all_template(mail.ident)
+
+        self.assertEquals(200, response['code'])
+        self.assertEquals(['another@pixelated.org'], response['body']['header']['to'][0])
+        self.assertEquals('Re: some subject', response['body']['header']['subject'])
diff --git a/service/test/unit/adapter/mail_service_test.py b/service/test/unit/adapter/mail_service_test.py
index c69f8f74..e5085724 100644
--- a/service/test/unit/adapter/mail_service_test.py
+++ b/service/test/unit/adapter/mail_service_test.py
@@ -49,3 +49,11 @@ class TestMailService(unittest.TestCase):
         self.mail_service.delete_mail(1)
 
         verify(self.mailboxes).move_to_trash(1)
+
+    def test_reply_all_template(self):
+        mail = mock()
+        when(self.mail_service).mail(1).thenReturn(mail)
+
+        self.mail_service.reply_all_template(1)
+
+        verify(mail).to_reply_template()
diff --git a/service/test/unit/adapter/mail_test.py b/service/test/unit/adapter/mail_test.py
index e0879b44..332fad8a 100644
--- a/service/test/unit/adapter/mail_test.py
+++ b/service/test/unit/adapter/mail_test.py
@@ -69,9 +69,13 @@ class TestPixelatedMail(unittest.TestCase):
         self.assertEquals(mail.fdoc.content['flags'], [])
 
     def test_as_dict(self):
-        fdoc, hdoc, bdoc = test_helper.leap_mail(flags=['\\Recent'])
-        hdoc.content['headers']['Subject'] = 'The subject'
-        hdoc.content['headers']['From'] = 'me@pixelated.org'
+        headers = {'Subject': 'The subject',
+                   'From': 'someone@pixelated.org',
+                   'To': 'me@pixelated.org'}
+        fdoc, hdoc, bdoc = test_helper.leap_mail(flags=['\\Recent'],
+                                                 extra_headers=headers)
+
+        InputMail.FROM_EMAIL_ADDRESS = 'me@pixelated.org'
 
         mail = PixelatedMail.from_soledad(fdoc, hdoc, bdoc, soledad_querier=self.querier)
 
@@ -80,16 +84,45 @@ class TestPixelatedMail(unittest.TestCase):
         self.assertEquals(_dict, {'body': 'body',
                                   'header': {
                                       'date': dateparser.parse(hdoc.content['date']).isoformat(),
-                                      'from': 'me@pixelated.org',
-                                      'subject': 'The subject'
+                                      'from': 'someone@pixelated.org',
+                                      'subject': 'The subject',
+                                      'to': ['me@pixelated.org'],
+                                      'cc': [],
+                                      'bcc': []
                                   },
                                   'ident': 'chash',
                                   'mailbox': 'inbox',
                                   'security_casing': {'imprints': [], 'locks': []},
                                   'status': ['recent'],
                                   'tags': [],
-                                  'attachments': []
-                                  })
+                                  'attachments': [],
+                                  'replying': {
+                                      'single': 'someone@pixelated.org',
+                                      'all': {
+                                          'to-field': ['someone@pixelated.org'],
+                                          'cc-field': []
+                                      }
+                                  }})
+
+    def test_use_reply_to_address_for_replying(self):
+        headers = {'Subject': 'The subject',
+                   'From': 'someone@pixelated.org',
+                   'Reply-To': 'reply-to-this-address@pixelated.org',
+                   'To': 'me@pixelated.org, \nalice@pixelated.org'}
+        fdoc, hdoc, bdoc = test_helper.leap_mail(flags=['\\Recent'],
+                                                 extra_headers=headers)
+
+        InputMail.FROM_EMAIL_ADDRESS = 'me@pixelated.org'
+
+        mail = PixelatedMail.from_soledad(fdoc, hdoc, bdoc, soledad_querier=self.querier)
+
+        _dict = mail.as_dict()
+
+        self.assertEquals(_dict['replying'], {'single': 'reply-to-this-address@pixelated.org',
+                                              'all': {
+                                                  'to-field': ['alice@pixelated.org', 'reply-to-this-address@pixelated.org'],
+                                                  'cc-field': []
+                                              }})
 
     def test_alternatives_body(self):
         parts = {'alternatives': [], 'attachments': []}
@@ -113,10 +146,12 @@ class TestPixelatedMail(unittest.TestCase):
         self.assertRegexpMatches(mail.body, '([\s\S]*100%){2}')
 
     def test_clean_line_breaks_on_address_headers(self):
-        fdoc, hdoc, bdoc = test_helper.leap_mail(flags=['\\Recent'])
-        hdoc.content['headers']['To'] = 'One <one@mail.com>,\nTwo <two@mail.com>, Normal <normal@mail.com>,\nalone@mail.com'
-        hdoc.content['headers']['Bcc'] = hdoc.content['headers']['To']
-        hdoc.content['headers']['Cc'] = hdoc.content['headers']['To']
+        many_recipients = 'One <one@mail.com>,\nTwo <two@mail.com>, Normal <normal@mail.com>,\nalone@mail.com'
+        headers = {'Cc': many_recipients,
+                   'Bcc': many_recipients,
+                   'To': many_recipients}
+        fdoc, hdoc, bdoc = test_helper.leap_mail(flags=['\\Recent'],
+                                                 extra_headers=headers)
 
         mail = PixelatedMail.from_soledad(fdoc, hdoc, bdoc, soledad_querier=self.querier)
 
diff --git a/service/test/unit/controllers/mails_controller_test.py b/service/test/unit/controllers/mails_controller_test.py
index 6d566c83..3e28e6b0 100644
--- a/service/test/unit/controllers/mails_controller_test.py
+++ b/service/test/unit/controllers/mails_controller_test.py
@@ -113,9 +113,15 @@ class TestMailsController(unittest.TestCase):
 
         verify(self.mail_service).delete_permanent(1)
 
+    def test_reply_all_returns_template(self):
+        when(self.mail_service).reply_all_template(1).thenReturn(self.input_mail.json)
+
+        self.mails_controller.reply_all_template(self.dummy_request, 1)
+
+        verify(self.mail_service).reply_all_template(1)
+
     def _successfuly_send_mail(self, ident, mail):
         sent_mail = mock()
-        sent_mail.mailbox_name = 'TRASH'
         sent_mail.as_dict = lambda: self.input_mail.json
 
         return sent_mail
diff --git a/web-ui/app/js/services/model/mail.js b/web-ui/app/js/services/model/mail.js
index 3fa3ceca..870c6a80 100644
--- a/web-ui/app/js/services/model/mail.js
+++ b/web-ui/app/js/services/model/mail.js
@@ -30,15 +30,6 @@ define(['helpers/contenttype'],
       return  this.mailbox === 'DRAFTS';
     }
 
-    function normalize(recipients) {
-      return _.chain([recipients])
-        .flatten()
-        .filter(function (r) {
-          return !_.isUndefined(r) && !_.isEmpty(r);
-        })
-        .value();
-    }
-
     function isInTrash() {
       return _.contains(this.tags, 'trash');
     }
@@ -47,32 +38,17 @@ define(['helpers/contenttype'],
       this.draft_reply_for = ident;
     }
 
-    function recipients(){
+    function replyToAddress() {
       return {
-        to: normalize(this.header.to),
-        cc: normalize(this.header.cc)
+        to: [this.replying.single],
+        cc: []
       };
     }
 
-    function replyToAddress() {
-      var recipients;
-
-      if (this.isSentMail()) {
-        recipients = this.recipients();
-      } else {
-        recipients = {
-          to: normalize(this.header.reply_to || this.header.from),
-          cc: []
-        };
-      }
-
-      return recipients;
-    }
-
     function replyToAllAddress() {
       return {
-        to: normalize([this.header.reply_to, this.header.from, this.header.to]),
-        cc: normalize(this.header.cc)
+        to: this.replying.all['to-field'],
+        cc: this.replying.all['cc-field']
       };
     }
 
@@ -142,7 +118,6 @@ define(['helpers/contenttype'],
       this.setDraftReplyFor = setDraftReplyFor;
       this.replyToAddress = replyToAddress;
       this.replyToAllAddress = replyToAllAddress;
-      this.recipients = recipients;
       this.getMailMediaType = getMailMediaType;
       this.isMailMultipartAlternative = isMailMultipartAlternative;
       this.getMailMultiParts = getMailMultiParts;
diff --git a/web-ui/test/spec/services/model/mail.spec.js b/web-ui/test/spec/services/model/mail.spec.js
index 24a8d244..2cbd21e9 100644
--- a/web-ui/test/spec/services/model/mail.spec.js
+++ b/web-ui/test/spec/services/model/mail.spec.js
@@ -2,31 +2,17 @@
 
 require(['services/model/mail'], function (Mail) {
   'use strict';
-  var testData;
 
   describe('services/model/mail', function () {
-    describe('reply addresses', function () {
-      it('returns the "to" and "cc" addresses if the mail was sent', function () {
-        var mail = Mail.create({
-          header: { to: ['a@b.c', 'e@f.g'], cc: ['x@x.x'] },
-          tags: [],
-          mailbox: 'SENT'
-        });
-
-        var addresses = mail.replyToAddress();
-
-        expect(addresses).toEqual({ to: ['a@b.c', 'e@f.g'], cc: ['x@x.x']});
-      });
-    });
-
     describe('parsing', function () {
       describe('a single email', function () {
-        var sentMail, draftMail, recievedMail, recievedMailWithCC;
+        var sentMail, draftMail, recievedMail, recievedMailWithCC, rawMailWithMultipleTo;
         beforeEach(function () {
           sentMail = Mail.create(Pixelated.testData().rawMail.sent);
           draftMail = Mail.create(Pixelated.testData().rawMail.draft);
           recievedMail = Mail.create(Pixelated.testData().rawMail.recieved);
           recievedMailWithCC = Mail.create(Pixelated.testData().rawMail.recievedWithCC);
+          rawMailWithMultipleTo = Mail.create(Pixelated.testData().rawMail.rawMailWithMultipleTo);
         });
 
         it('correctly identifies a sent mail', function () {
@@ -41,25 +27,6 @@ require(['services/model/mail'], function (Mail) {
           expect(recievedMail.isSentMail()).toBe(false);
           expect(recievedMail.isDraftMail()).toBe(false);
         });
-
-        it('reply to of a sent mail should be original recipient', function () {
-          expect(sentMail.replyToAddress()).toEqual({to: ['mariane_dach@davis.info'], cc: ['duda@la.lu']});
-        });
-
-        it('reply to of a mail should be the reply_to field if existent', function () {
-          expect(recievedMail.replyToAddress()).toEqual({to: ['afton_braun@botsford.biz'], cc: [] });
-        });
-
-        it('reply to of a mail should be the from field if no reply_to present', function () {
-          expect(recievedMailWithCC.replyToAddress()).toEqual({to: ['cleve_jaskolski@schimmelhirthe.net'], cc: []});
-        });
-
-        it('reply to all should include all email addresses in the header', function () {
-          expect(recievedMailWithCC.replyToAllAddress()).toEqual({
-            to: ['cleve_jaskolski@schimmelhirthe.net', 'stanford@sipes.com'],
-            cc: ['mariane_dach@davis.info']
-          });
-        });
       });
 
       describe('multipart email', function () {
diff --git a/web-ui/test/test_data.js b/web-ui/test/test_data.js
index 194cd02d..018c3143 100644
--- a/web-ui/test/test_data.js
+++ b/web-ui/test/test_data.js
@@ -16,6 +16,13 @@ define(function() {
     security_casing: {
       locks: [],
       imprints: []
+    },
+    replying: {
+      single: 'laurel@hamil.info',
+      all: {
+        'to-field': ['laurel@hamil.info'],
+        'cc-field': []
+      }
     }
   };
 
@@ -25,7 +32,15 @@ define(function() {
     'tags':['photography','sky'],
     'status':['read'],
     'body':'Illum eos nihil commodi voluptas. Velit consequatur odio quibusdam. Beatae aliquam hic quos.',
-    'mailbox': 'SENT'
+    'mailbox': 'SENT',
+    replying: {
+      single: 'laurel@hamil.info',
+      all: {
+        'to-field': ['mariane_dach@davis.info'],
+        'cc-field': ['duda@la.lu']
+      }
+    }
+
   };
 
   var rawDraftMail = {
@@ -34,7 +49,15 @@ define(function() {
     'tags':['photography','sky'],
     'status':['read'],
     'body':'Illum eos nihil commodi voluptas. Velit consequatur odio quibusdam. Beatae aliquam hic quos.',
-    'mailbox': 'DRAFTS'
+    'mailbox': 'DRAFTS',
+    replying: {
+      single: 'afton_braun@botsford.biz',
+      all: {
+        'to-field': ['afton_braun@botsford.biz'],
+        'cc-field': []
+      }
+    }
+
   };
 
   var rawRecievedMail = {
@@ -42,7 +65,15 @@ define(function() {
     'ident':242,
     'tags':['garden','instalovers','popularpic'],
     'status':['read'],
-    'body':'Sed est neque tempore. Alias officiis pariatur ullam porro corporis. Tempore eum quia placeat. Sapiente fuga cum.'
+    'body':'Sed est neque tempore. Alias officiis pariatur ullam porro corporis. Tempore eum quia placeat. Sapiente fuga cum.',
+    replying: {
+      single: 'afton_braun@botsford.biz',
+      all: {
+        'to-field': ['cleve_jaskolski@schimmelhirthe.net', 'afton_braun@botsford.biz'],
+        'cc-field': []
+      }
+    }
+
   };
 
   var rawRecievedWithCCMail = {
@@ -50,7 +81,31 @@ define(function() {
     'ident':242,
     'tags':['garden','instalovers','popularpic'],
     'status':['read'],
-    'body':'Sed est neque tempore. Alias officiis pariatur ullam porro corporis. Tempore eum quia placeat. Sapiente fuga cum.'
+    'body':'Sed est neque tempore. Alias officiis pariatur ullam porro corporis. Tempore eum quia placeat. Sapiente fuga cum.',
+    replying: {
+      single: 'cleve_jaskolski@schimmelhirthe.net',
+      all: {
+        'to-field': ['cleve_jaskolski@schimmelhirthe.net'],
+        'cc-field': ['mariane_dach@davis.info']
+      }
+    }
+
+  };
+
+  var rawMailWithMultipleTo = {
+    'header':{'to':['stanford@sipes.com', 'someoneelse@some-other-domain.tld'],'from':'cleve_jaskolski@schimmelhirthe.net','cc':'mariane_dach@davis.info','subject':'Cumque pariatur vel consequuntur deleniti ex.','date':'2014-06-17T05:40:29-03:00'},
+    'ident':242,
+    'tags':['garden','instalovers','popularpic'],
+    'status':['read'],
+    'body':'Sed est neque tempore. Alias officiis pariatur ullam porro corporis. Tempore eum quia placeat. Sapiente fuga cum.',
+    replying: {
+      single: 'cleve_jaskolski@schimmelhirthe.net',
+      all: {
+        'to-field': ['cleve_jaskolski@schimmelhirthe.net', 'someoneelse@some-other-domain.tld'],
+        'cc-field': ['mariane_dach@davis.info']
+      }
+    }
+
   };
 
   var rawMultipartMail = {
@@ -73,7 +128,15 @@ define(function() {
           'Content-Transfer-Encoding: quoted-printable\n' +
           '\n' +
           '<p><b>Hello everyone!</b></p>\n' +
-          '--asdfghjkl--\n'
+          '--asdfghjkl--\n',
+    replying: {
+      single: 'laurel@hamil.info',
+      all: {
+        'to-field': ['laurel@hamil.info'],
+        'cc-field': []
+      }
+    }
+
   };
 
   var simpleTextPlainMail = {
@@ -158,6 +221,7 @@ define(function() {
       draft: rawDraftMail,
       recieved: rawRecievedMail,
       recievedWithCC: rawRecievedWithCCMail,
+      rawMailWithMultipleTo: rawMailWithMultipleTo,
       multipart: rawMultipartMail
     },
     parsedMail: {