[FIX] mail, portal, account: clean up portal attachment

Follow up of 61de1c263a7a40a36f2b58b8a107c350de6b6c7f

- Remove the dangerous **kw from the route in favor of specifying useful args.

- Use the existing method to link attachments to message instead of duplicating
the logic in an incomplete manner.

- Fix an issue where the actual email would not be sent if the message was empty
but an attachment was defined.

- Ensure the send attempt is done immediately instead of after the cr commit.
The goal is to show potential error messages correctly to the user with the
crash manager instead of a generic error page.

This also increases the send timeout and prevents the error in the first
place in most situations.

closes odoo/odoo#35263

Signed-off-by: Christophe Simonis <chs@odoo.com>

2 files changed, 0 insertions, 153 deletions

diff --git a/addons/portal/tests/__init__.py b/addons/portal/tests/__init__.py
index 752ffa6f96c..06a8d4cd529 100644
--- a/addons/portal/tests/__init__.py
+++ b/addons/portal/tests/__init__.py
@@ -2,4 +2,3 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import test_load_process
-from . import test_attachment
diff --git a/addons/portal/tests/test_attachment.py b/addons/portal/tests/test_attachment.py
deleted file mode 100644
index a3f8c3b11db..00000000000
--- a/addons/portal/tests/test_attachment.py
+++ /dev/null
@@ -1,152 +0,0 @@
-# -*- coding: utf-8 -*-
-# Part of Odoo. See LICENSE file for full copyright and licensing details.
-
-import json
-
-from odoo import http, tests
-from odoo.tools import mute_logger
-
-
-@tests.tagged('post_install', '-at_install')
-class TestUi(tests.HttpCase):
-
-    @mute_logger('odoo.addons.website.models.ir_http', 'odoo.http')
-    def test_01_portal_attachment(self):
-        base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url')
-        # For this test we need a parent document with an access_token field:
-        # attachment itself has an access_token so we can use it as parent too.
-        record = self.env['ir.attachment'].create({
-            'name': 'a record with an access_token field',
-        })
-
-        # Test public user can't create attachment without token of document
-        create_data = {
-            'name': "new attachment",
-            'res_model': record._name,
-            'res_id': record.id,
-            'csrf_token': http.WebRequest.csrf_token(self),
-        }
-        create_url = base_url + '/portal/attachment/add'
-        files = [('file', ('test.txt', b'test', 'plain/text'))]
-        res = self.url_open(url=create_url, data=create_data, files=files)
-        self.assertEqual(res.status_code, 400)
-        self.assertTrue("you do not have the rights" in res.text)
-
-        # Test public user can create attachment with token
-        create_data['access_token'] = record.generate_access_token()[0]
-        res = self.url_open(url=create_url, data=create_data, files=files)
-        self.assertEqual(res.status_code, 200)
-        create_res = json.loads(res.content.decode('utf-8'))
-        self.assertEqual(create_res['name'], "new attachment")
-
-        # Test created attachment is private
-        res_binary = self.url_open('/web/content/%d' % create_res['id'])
-        self.assertEqual(res_binary.status_code, 404)
-
-        # Test created access_token is working
-        res_binary = self.url_open('/web/content/%d?access_token=%s' % (create_res['id'], create_res['access_token']))
-        self.assertEqual(res_binary.status_code, 200)
-
-        # Test mimetype is neutered as non-admin
-        files = [('file', ('test.svg', b'<svg></svg>', 'image/svg+xml'))]
-        res = self.url_open(url=create_url, data=create_data, files=files)
-        self.assertEqual(res.status_code, 200)
-        create_res = json.loads(res.content.decode('utf-8'))
-        self.assertEqual(create_res['mimetype'], 'text/plain')
-
-        res_binary = self.url_open('/web/content/%d?access_token=%s' % (create_res['id'], create_res['access_token']))
-        self.assertEqual(res_binary.headers['Content-Type'], 'text/plain')
-        self.assertEqual(res_binary.content, b'<svg></svg>')
-
-        res_image = self.url_open('/web/image/%d?access_token=%s' % (create_res['id'], create_res['access_token']))
-        self.assertEqual(res_image.headers['Content-Type'], 'text/plain')
-        self.assertEqual(res_image.content, b'<svg></svg>')
-
-        # Test attachment can't be removed without valid token
-        remove_data = {
-            'attachment_id': create_res['id'],
-            'access_token': 'wrong',
-        }
-        remove_url = base_url + '/portal/attachment/remove'
-        res = self.opener.post(url=remove_url, json={'params': remove_data})
-        self.assertEqual(res.status_code, 200)
-        self.assertTrue(self.env['ir.attachment'].search([('id', '=', create_res['id'])]))
-        self.assertTrue("you do not have the rights" in res.text)
-
-        # Test attachment can be removed with token if "pending" state
-        remove_data['access_token'] = create_res['access_token']
-        res = self.opener.post(url=remove_url, json={'params': remove_data})
-        self.assertEqual(res.status_code, 200)
-        remove_res = json.loads(res.content.decode('utf-8'))['result']
-        self.assertFalse(self.env['ir.attachment'].search([('id', '=', create_res['id'])]))
-        self.assertTrue(remove_res is True)
-
-        # Test attachment can't be removed if not "pending" state
-        remove_data = {
-            'attachment_id': record.id,
-            'access_token': record.access_token,
-        }
-        res = self.opener.post(url=remove_url, json={'params': remove_data})
-        self.assertEqual(res.status_code, 200)
-        self.assertTrue(self.env['ir.attachment'].search([('id', '=', record.id)]))
-        self.assertTrue("not in a pending state" in res.text)
-
-        # Test attachment can't be removed if attached to a message
-        attachment = self.env['ir.attachment'].create({
-            'name': 'a record with an access_token field',
-            'res_model': 'mail.compose.message',
-            'res_id': 0,
-            'access_token': self.env['ir.attachment']._generate_access_token(),
-        })
-        message = self.env['mail.message'].create({
-            'attachment_ids': [(6, 0, attachment.ids)],
-        })
-        remove_data = {
-            'attachment_id': attachment.id,
-            'access_token': attachment.access_token,
-        }
-        res = self.opener.post(url=remove_url, json={'params': remove_data})
-        self.assertEqual(res.status_code, 200)
-        self.assertTrue(attachment.exists())
-        self.assertTrue("it is linked to a message" in res.text)
-        message.unlink()
-
-        # Need a `mail.thread` record for the following test
-        thread_record = self.env['mail.channel'].create({
-            'name': 'channel',
-            'public': 'public',
-        })
-        # Authenticate because public user can't create `mail.message` without
-        # token, and `mail.channel` does not have an access_token.
-        self.authenticate('portal', 'portal')
-
-        # Test attachment can't be associated if no token.
-        post_url = base_url + '/mail/chatter_post'
-        post_data = {
-            'res_model': thread_record._name,
-            'res_id': thread_record.id,
-            'message': "test message",
-            'attachment_ids': attachment.id,
-            'attachment_tokens': 'false',
-            'csrf_token': http.WebRequest.csrf_token(self),
-        }
-        self.assertFalse(thread_record.message_ids)
-        res = self.url_open(url=post_url, data=post_data)
-        thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids)
-        message = thread_record.message_ids[0]
-        self.assertFalse(message.attachment_ids)
-
-        # Test attachment can't be associated if not "pending" state
-        post_data['attachment_tokens'] = attachment.access_token
-        attachment.write({'res_model': 'model'})
-        res = self.url_open(url=post_url, data=post_data)
-        thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids)
-        message = thread_record.message_ids[0]
-        self.assertFalse(message.attachment_ids)
-
-        # Test attachment can be associated if all good
-        attachment.write({'res_model': 'mail.compose.message'})
-        res = self.url_open(url=post_url, data=post_data)
-        thread_record.invalidate_cache(fnames=['message_ids'], ids=thread_record.ids)
-        message = thread_record.message_ids[0]
-        self.assertEqual(len(message.attachment_ids), 1)