-
-
Notifications
You must be signed in to change notification settings - Fork 633
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
[17.0][mig] mail_attach_existing_attachment_account #1547
base: 17.0
Are you sure you want to change the base?
[17.0][mig] mail_attach_existing_attachment_account #1547
Conversation
[UPD] README.rst [ADD] icon.png [UPD] Update mail_attach_existing_attachment_account.pot [UPD] README.rst
TT28899 [UPD] Update mail_attach_existing_attachment_account.pot [UPD] README.rst
…ti-company Doing a bulk search on accounts without company filter can lead to selecting an account of other company, and thus, making the test to fail. We restrict the account search to the current company.
We show the field with the same technique as in the base module, which improves the display, showing the label.
Currently translated at 50.0% (1 of 2 strings) Translation: social-16.0/social-16.0-mail_attach_existing_attachment_account Translate-URL: https://translation.odoo-community.org/projects/social-16-0/social-16-0-mail_attach_existing_attachment_account/it/
Currently translated at 100.0% (2 of 2 strings) Translation: social-16.0/social-16.0-mail_attach_existing_attachment_account Translate-URL: https://translation.odoo-community.org/projects/social-16-0/social-16-0-mail_attach_existing_attachment_account/it/
a073f31
to
d701bc5
Compare
c1132cd
to
1ff4372
Compare
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.
Overall, LGTM
# def _prepare_mail_values(self, res_ids): | ||
# res = super()._prepare_mail_values(res_ids) | ||
# if self.object_attachment_ids.ids and self.model and len(res_ids) == 1: | ||
# res[res_ids[0]].setdefault("attachment_ids", []).extend( | ||
# self.object_attachment_ids.ids | ||
# ) | ||
# return res |
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.
Remove this commented out code please :)
name="object_attachment_ids" | ||
widget="many2many_checkboxes" | ||
domain="[('res_model', '=', 'account.move'), ('res_id', 'in', move_ids)]" | ||
invisible="can_attach_attachment==False" |
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.
invisible="can_attach_attachment==False" | |
invisible="not can_attach_attachment" |
?
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 suppose we could improve test coverage on this module
# def _prepare_mail_values(self, res_ids): | ||
# res = super()._prepare_mail_values(res_ids) | ||
# if self.object_attachment_ids.ids and self.model and len(res_ids) == 1: | ||
# res[res_ids[0]].setdefault("attachment_ids", []).extend( | ||
# self.object_attachment_ids.ids | ||
# ) | ||
# return res |
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 suppose comment code could be removed
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.
TT54708
@Tecnativa
res["can_attach_attachment"] = True # pragma: no cover | ||
return res | ||
|
||
can_attach_attachment = fields.Boolean() |
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.
Remove the duplicate field; it is already declared at the top of the file.
No description provided.