Skip to content
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

[IMP] mail, test_mail, *: add new email.cc.mixin on main models #31275

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@Xavier-Do
Copy link
Contributor

Xavier-Do commented Feb 20, 2019

*: crm, project, maintenance, hr_recruitment

When a record is created from an email, cc can be lost. This commit
proposes a new mixin to keep cc on the record and allows to create
partner for each of them when sending a message from the chatter.

The mixin is added on the mail recors that can be created from mail.alias:

document
helpdesk.ticket
mrp.eco
quality.alert
hr.applicant
crm.lead
project.task
mail.channel
maintenance.equipment

A new show more/ show less mechanism is also added to the partner suggestions in chatter.

Task: 1925001
Linked PR: odoo/enterprise#3689

@Xavier-Do Xavier-Do requested review from alexkuhn and tde-banana-odoo Feb 20, 2019

@robodoo robodoo added the seen 🙂 label Feb 20, 2019

@C3POdoo C3POdoo added the RD label Feb 20, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-cc-xdo branch Feb 20, 2019

cc_values = {
'email_cc': ", ".join(self._email_dict(msg_dict.get('cc')).values()),
}
cc_values.update(custom_values)

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

Just personal preference, but it's easier to understand code with custom_values.update(cc_values) instead

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

Maybe you do that because you do not want to change in place the passed dict custom_values? Would name variable cc_values differently then, something like new_custom_values?

old_cc = self._email_dict(self.email_cc)
new_cc.update(old_cc)
cc_values['email_cc'] = ", ".join(new_cc.values())
cc_values.update(update_vals)

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

same thing as with message_new (easier to read that update_vals is updated)

if custom_values is None:
custom_values = {}
cc_values = {
'email_cc': ", ".join(self._email_dict(msg_dict.get('cc')).values()),

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

Are you sure you need _email_dict here? Looks like you can simply pass msg_dict.get('cc'), no?

This comment has been minimized.

@Xavier-Do

Xavier-Do Feb 20, 2019

Author Contributor

it is to avoid duplicate

cc_values = {}
new_cc = self._email_dict(msg_dict.get('cc'))
if new_cc:
old_cc = self._email_dict(self.email_cc)

This comment was marked as resolved.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

maybe I would put a comment that you make the union of self.email_cc and msg_dict.get('cc')

This comment was marked as resolved.

@Xavier-Do

Xavier-Do Feb 20, 2019

Author Contributor

it is explained in doc string

addons/mail/static/src/js/composers/chatter_composer.js Outdated
*/
_onClickPartnerSuggestionsReadMoreLess: function (ev) {
ev.preventDefault();
this.$('.o_suggested_toggle').toggleClass('o_suggested_hidden');

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

couldn't you use o_hidden?

addons/mail/static/src/js/composers/chatter_composer.js Outdated
ev.preventDefault();
this.$('.o_suggested_toggle').toggleClass('o_suggested_hidden');
},

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

no newline between methods of same category

addons/mail/static/src/scss/composer.scss Outdated
@@ -104,6 +104,9 @@
.o_chatter_composer_info, .o_composer_suggested_partners {
flex: 0 0 100%;
margin-bottom: $o-mail-chatter-gap*0.5;
.o_suggested_hidden {
display:none;

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

whitespace between key and value, but I think you could drop this addition if you use o_hidden instead

addons/mail/static/src/xml/chatter.xml Outdated
@@ -60,7 +60,7 @@
<!-- List of followers -->
<div class="o_composer_suggested_partners">
<t t-foreach='widget.suggestedPartners' t-as='recipient'>
<div t-attf-title="Add as recipient and follower (reason: #{recipient.reason})">
<div t-attf-title="Add as recipient and follower (reason: #{recipient.reason})" t-att-class="recipient_index &gt; 2 ? 'o_suggested_toggle o_suggested_hidden' : ''">

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

I don't think you need to escape > to &gt;

This comment has been minimized.

@alexkuhn

alexkuhn Feb 20, 2019

Contributor

undefined parameter recipient_index in the template

This comment has been minimized.

@Xavier-Do

Xavier-Do Feb 20, 2019

Author Contributor

&gt is needed or crash, recipient_index is generated by foreach i think, it works fine

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-cc-xdo branch 3 times, most recently Feb 20, 2019

@robodoo robodoo added the CI 🤖 label Feb 20, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-cc-xdo branch Feb 21, 2019

@robodoo robodoo removed the CI 🤖 label Feb 21, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-cc-xdo branch 2 times, most recently Feb 21, 2019

@robodoo robodoo added the CI 🤖 label Feb 21, 2019

Xavier-Do added some commits Feb 20, 2019

[IMP] mail: add show more button on partner suggestion.
When sending a message from chatter, a list of suggested partners appears
on the top of the text area. With the previous commit,
cc will be added to this list. This commit adds a show more/show less
button in case there is more than 3 suggested recipients to display.

Task: 1925001
[IMP] mail, test_mail, *: add new email.cc.mixin on main models
*: crm, project, maintenance, hr_recruitment

When a record is created from an email, cc can be lost. This commit
proposes a new mixin to keep cc on the record and allows to create
partner for each of them when sending a message from the chatter.

The mixin is added on the mail recors that can be created from mail.alias:

document
helpdesk.ticket
mrp.eco
quality.alert
hr.applicant
crm.lead
project.task
mail.channel
maintenance.equipment

Task: 1925001

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-cc-xdo branch to c8814b5 Mar 7, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.