-
-
Notifications
You must be signed in to change notification settings - Fork 587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MIG][9.0] mail_attach_existing_attachment: Module migrated #68
[MIG][9.0] mail_attach_existing_attachment: Module migrated #68
Conversation
@@ -29,7 +29,7 @@ | |||
'author': "ACSONE SA/NV,Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Tecnativa here
You can put a better icon for this module |
@pedrobaeza changes done |
@@ -44,7 +44,7 @@ Bug Tracker | |||
Bugs are tracked on `GitHub Issues <https://github.com/OCA/social/issues>`_. | |||
In case of trouble, please check there if your issue has already been reported. | |||
If you spotted it first, help us smashing it by providing a detailed and welcomed feedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can simplify this part following latest README template
@pedrobaeza @elicoidal Changes done |
def get_mail_values(self, res_ids): | ||
res = super(MailComposeMessage, self).get_mail_values(res_ids) | ||
if self.object_attachment_ids.ids and self.model and len(res_ids) == 1: | ||
if not res[res_ids[0]].get('attachment_ids'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make res_id = res_ids[0]
for clarifying the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that you can simplify whole expression with:
res[res_ids[0]].setdefault('attachment_ids', []).extend(self.object_attachment_ids.ids)
There's only one line without coverage. As is a corner case, you can put |
Tested functionally with SO an INV 👍 |
@@ -35,7 +35,7 @@ def default_get(self, fields_list): | |||
if res.get('res_id') and res.get('model') and \ | |||
res.get('composition_mode', '') != 'mass_mail' and\ | |||
not res.get('can_attach_attachment'): | |||
# pragma: nocoverlo | |||
# pragma: nocover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the same line:
res['can_attach_attachment'] = True # pragma: nocover
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ups..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch now by my part. It's # pragma: no cover
5e5f164
to
cbbdc24
Compare
👍 code + tested on partner in Runbot |
@sergio-teruel review comments from @pedrobaeza and we can merge |
👍 |
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
* [MIG][9.0] mail_attach_existing_attachment: Module migrated
@Tecnativa
@rafaelbn please review..