attachments are only un-linked from emails but not deleted in soledad

- removed backend delete of attachments
- JS unit tests
- Functional test

Issue #549

8 files changed, 64 insertions, 46 deletions

diff --git a/service/pixelated/adapter/mailstore/leap_attachment_store.py b/service/pixelated/adapter/mailstore/leap_attachment_store.py
index 2c004abf..982d9222 100644
--- a/service/pixelated/adapter/mailstore/leap_attachment_store.py
+++ b/service/pixelated/adapter/mailstore/leap_attachment_store.py
@@ -63,9 +63,3 @@ class LeapAttachmentStore(object):
     def _calc_attachment_id_(self, content, content_type, encoder=encoders.encode_base64):
         cdoc = self._attachment_to_cdoc(content, content_type, encoder)
         return cdoc.phash
-
-    @defer.inlineCallbacks
-    def delete_attachment(self, attachment_id):
-        doc = yield self.soledad.get_from_index('by-type-and-payloadhash', 'cnt', attachment_id)
-        doc = doc[0]
-        yield self.soledad.delete_doc(doc)
diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py
index d2506dcf..dd1a32d8 100644
--- a/service/pixelated/adapter/services/mail_service.py
+++ b/service/pixelated/adapter/services/mail_service.py
@@ -151,6 +151,3 @@ class MailService(object):
     @defer.inlineCallbacks
     def delete_permanent(self, mail_id):
         yield self.mail_store.delete_mail(mail_id)
-
-    def delete_attachment(self, attachment_id):
-        return self.attachment_store.delete_attachment(attachment_id)
diff --git a/service/pixelated/resources/attachments_resource.py b/service/pixelated/resources/attachments_resource.py
index da455f98..086f6e4e 100644
--- a/service/pixelated/resources/attachments_resource.py
+++ b/service/pixelated/resources/attachments_resource.py
@@ -72,20 +72,6 @@ class AttachmentResource(Resource):
         match = re.compile('([A-Za-z-]+\/[A-Za-z-]+)').search(content_type)
         return match.group(1)
 
-    def render_DELETE(self, request):
-        def success(_):
-            response_json = {"ident": self.attachment_id}
-            respond_json_deferred(response_json, request, status_code=200)
-
-        def failure(err):
-            request.code = 500
-            request.finish()
-
-        d = self.mail_service.delete_attachment(self.attachment_id)
-        d.addCallback(success)
-        d.addErrback(failure)
-        return server.NOT_DONE_YET
-
 
 class AttachmentsResource(BaseResource):
     BASE_URL = 'attachment'
diff --git a/service/test/functional/features/attachments.feature b/service/test/functional/features/attachments.feature
index 5cdbf7be..7724596d 100644
--- a/service/test/functional/features/attachments.feature
+++ b/service/test/functional/features/attachments.feature
@@ -37,6 +37,9 @@ Feature: Attachments
     When I dismiss the error message
     Then It should not show the error message anymore
     When I upload a valid file
+    And remove the file
+    Then I should not see it attached
+    When I upload a valid file
     And I send it
     When I select the tag 'sent'
     And I open the first mail in the mail list
diff --git a/service/test/functional/features/steps/attachments.py b/service/test/functional/features/steps/attachments.py
index b0c7c791..76e42177 100644
--- a/service/test/functional/features/steps/attachments.py
+++ b/service/test/functional/features/steps/attachments.py
@@ -96,3 +96,16 @@ def upload_attachment(context):
     fill_by_css_selector(context, '#fileupload', base_dir + fname)
     attachment_list_item = wait_until_element_is_visible_by_locator(context, (By.CSS_SELECTOR, '#attachment-list-item li a'))
     assert attachment_list_item.text == "%s (36.00 b)" % fname
+
+
+@when(u'remove the file')
+def click_remove_icon(context):
+    remove_icon = wait_until_element_is_visible_by_locator(context, (By.CSS_SELECTOR, '#attachment-list-item i.remove-icon'))
+    remove_icon.click()
+
+
+@then(u'I should not see it attached')
+def assert_attachment_removed(context):
+    attachments_list_ul = find_elements_by_css_selector(context, '#attachment-list-item')
+    attachments_list_li = context.browser.find_elements(By.CSS_SELECTOR, '#attachment-list-item li a')
+    assert len(attachments_list_li) == 0
diff --git a/service/test/unit/adapter/mailstore/test_leap_attachment_store.py b/service/test/unit/adapter/mailstore/test_leap_attachment_store.py
index f1bd8528..4e9b56b1 100644
--- a/service/test/unit/adapter/mailstore/test_leap_attachment_store.py
+++ b/service/test/unit/adapter/mailstore/test_leap_attachment_store.py
@@ -116,21 +116,6 @@ class TestLeapAttachmentStore(TestCase):
         except ValueError:
             pass
 
-    @defer.inlineCallbacks
-    def test_soledad_delete_doc_is_called_when_deleting_an_attachment(self):
-        attachment_id = '1B0A9AAD9E153D24265395203C53884506ABA276394B9FEC02B214BF9E77E48E'
-        doc = SoledadDocument(json=json.dumps({'content_type': 'foo/bar', 'raw': 'quoted-printable',
-                                               'phash': attachment_id,
-                                               'content_transfer_encoding': ''}))
-
-        when(self.soledad).get_from_index('by-type-and-payloadhash', 'cnt', attachment_id).thenReturn(defer.succeed([doc]))
-        when(self.soledad).delete_doc(doc).thenReturn(defer.succeed(None))
-
-        store = LeapAttachmentStore(self.soledad)
-        yield store.delete_attachment(attachment_id)
-
-        verify(self.soledad).delete_doc(doc)
-
     def _mock_get_mailbox(self, mailbox_name, create_new_uuid=False):
         mbox_uuid = self.mbox_uuid if not create_new_uuid else str(uuid4())
         when(self.soledad).list_indexes().thenReturn(defer.succeed(MAIL_INDEXES)).thenReturn(
diff --git a/web-ui/app/js/mail_view/ui/attachment_list.js b/web-ui/app/js/mail_view/ui/attachment_list.js
index ff7602ec..0483ae6a 100644
--- a/web-ui/app/js/mail_view/ui/attachment_list.js
+++ b/web-ui/app/js/mail_view/ui/attachment_list.js
@@ -195,14 +195,8 @@ define(
             };
 
             this.removeAttachments = function(event, data) {
-              var success = function() {
-                this.removeAttachmentFromList(data);
-                this.destroyAttachmentElement(data);
-              };
-
-              monitoredAjax(this, '/attachment/' + data.ident, {
-                type: 'DELETE'
-              }).done(success.bind(this));
+              this.removeAttachmentFromList(data);
+              this.destroyAttachmentElement(data);
             };
 
             this.after('initialize', function () {
diff --git a/web-ui/test/spec/mail_view/ui/attachment_list.spec.js b/web-ui/test/spec/mail_view/ui/attachment_list.spec.js
index 94559b63..eab605d6 100644
--- a/web-ui/test/spec/mail_view/ui/attachment_list.spec.js
+++ b/web-ui/test/spec/mail_view/ui/attachment_list.spec.js
@@ -120,6 +120,52 @@ describeMixin('mail_view/ui/attachment_list', function () {
                     expect(uploadAccepted).toBe(true);
                 });
             });
+
+            describe('Remove attachment', function () {
+                it('should call the remove attachment method when triggered the removeAttachement event', function () {
+                    var stubAttachment = {ident: 'whatever'};
+                    spyOn(this.component, 'removeAttachmentFromList');
+                    spyOn(this.component, 'destroyAttachmentElement');
+
+                    $(document).trigger(Pixelated.events.mail.removeAttachment, stubAttachment);
+
+                    expect(this.component.removeAttachmentFromList).toHaveBeenCalledWith(stubAttachment);
+                    expect(this.component.destroyAttachmentElement).toHaveBeenCalledWith(stubAttachment);
+                });
+
+                it('should remove the attachment item from the DOM', function () {
+                    var stubAttachment = {ident: 'whatever'};
+                    this.setupComponent('<div id="attachment-list">' +
+                        '<ul id="attachment-list-item"><li data-ident="whatever"></li></ul>' +
+                        '<ul id="attachment-upload-item"></ul>' +
+                        '</div>');
+
+                    expect(this.component.$node.find('li[data-ident=whatever]').length).toEqual(1);
+
+                    this.component.destroyAttachmentElement(stubAttachment);
+
+                    expect(this.component.$node.find('li[data-ident=whatever]').length).toEqual(0);
+                });
+
+
+                it('should remove attachment from attachment list', function () {
+                    var stubAttachment = {ident: 'whatever'};
+                    this.component.attr.attachments = [stubAttachment, {ident: 'another attachment'}];
+                    this.component.removeAttachmentFromList(stubAttachment);
+
+                    expect(this.component.attr.attachments).toEqual([{ident: 'another attachment'}]);
+                });
+
+
+                it('when remove attachment that is not on the attachment list should not do anything', function () {
+                    var stubAttachment = {ident: 'whatever'};
+                    this.component.attr.attachments = [stubAttachment];
+
+                    this.component.removeAttachmentFromList({ident: 'different attachment'});
+
+                    expect(this.component.attr.attachments).toEqual([stubAttachment]);
+                });
+            });
         });
     });
 });