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, *: don't track fields at create #31945

Closed

Conversation

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

Xavier-Do commented Mar 19, 2019

*: project, crm, maintenance, helpdesk,

It is useless to track fields during create since they
have no initial value and future tracking message will
show changes on tracked field.

We can log a default creation message instead
(as it is now if there is no mail_create_nolog context key)

This change will implies

  • less queries when creating record
  • cleaner creation messages
  • less occurence of mail_create_nolog ctx key

Removing tracking at create could break the creation subtypes
mechanism (example: following task creation subtype on project)
Instead of using _track_subtype to give a subtype at create,
a new _creation_subtype method can be override. If a creation
subtype is set on a specific modlel, creation messages will be
create by message_post instead of _message_log.

We also need to adapt the message_track_post_template in order to
keep this feature whithout tracking.

Task: #1916916

@Xavier-Do Xavier-Do requested a review from tde-banana-odoo Mar 19, 2019

@robodoo robodoo added the seen 🙂 label Mar 19, 2019

@C3POdoo C3POdoo added the RD label Mar 19, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch 2 times, most recently from f958b79 to 320f324 Mar 19, 2019

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

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch 4 times, most recently from 43dba26 to 5e7176f Mar 22, 2019

@robodoo robodoo added the CI 🤖 label Mar 22, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch from 5e7176f to f3dfd0f Mar 25, 2019

@robodoo robodoo removed the CI 🤖 label Mar 25, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch 6 times, most recently from 41256d2 to 7b7dd0b Mar 25, 2019

@tde-banana-odoo
Copy link
Contributor

tde-banana-odoo left a comment

Tech review: LGTM, some small comments. Good work !

for thread in track_threads:
create_values = create_values_list[thread.id]
tracked_fields = self._get_tracked_fields(list(create_values))
changes = [field for field in tracked_fields if create_values.get(field)] # based on tracked field to stay consistent with write

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 28, 2019

Contributor

Shouldn't we keep void values and just do if field in create_values ? Indeed we may perform check on 0 / False values when deciding a subtype to trigger.

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 28, 2019

Contributor

Just maybe add a comment to say it is like old behavior and is actually wanted at create

@@ -124,6 +124,8 @@ def test_message_process_alias_basic(self):
self.assertEqual(res[0], self.env.uid)

# Test: one message that is the incoming email
for message in record.message_ids:
print(message.body)

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 28, 2019

Contributor

HumanLinter: forgotten print

@@ -476,7 +474,7 @@ def test_complex_message_subscribe(self):
self.assertEqual(rec.message_channel_ids, self.channel)

@mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@users('__system__', 'emp')
@users('__system__')

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 28, 2019

Contributor

HumanLinter: put employee back again

partner_ids = [(4, parent_message.author_id.id)]
else:
subtype = 'mail.mt_comment'
if not subtype_id:

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 28, 2019

Contributor

Don't forget to add in pad functional change that now incoming email has subtype (impact for example lead users).

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch 2 times, most recently from 0e4ae26 to 37aa0f2 Mar 28, 2019

@Xavier-Do Xavier-Do changed the title Master remove tracking at create xdo [IMP] mail, *: don't track fields at create Mar 28, 2019

@Xavier-Do Xavier-Do requested a review from alexkuhn Mar 28, 2019

@robodoo robodoo added the CI 🤖 label Mar 28, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch from 37aa0f2 to c942f08 Mar 29, 2019

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

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch from c942f08 to 20ec2ce Mar 29, 2019

@robodoo robodoo removed the CI 🤖 label Mar 29, 2019

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch from 20ec2ce to d565570 Mar 29, 2019

@robodoo robodoo added the CI 🤖 label Mar 29, 2019

@tde-banana-odoo
Copy link
Contributor

tde-banana-odoo left a comment

Final review: just one final functional thingy, otherwise LGTM !

@@ -295,7 +295,7 @@ def fields_view_get(self, view_id=None, view_type='form', toolbar=False, submenu

@api.model
def create(self, vals):
move = super(AccountMove, self.with_context(mail_create_nolog=True, check_move_validity=False, partner_id=vals.get('partner_id'))).create(vals)
move = super(AccountMove, self.with_context(check_move_validity=False, partner_id=vals.get('partner_id'))).create(vals)

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 29, 2019

Contributor

Just re-add the create nolog

[IMP] mail, *: don't track fields at create
*: project, crm, maintenance, helpdesk,

It is useless to track fields during create since they
have no initial value and future tracking message will
show changes on tracked field.

We can log a default creation message instead
(as it is now if there is no mail_create_nolog context key)

This change will implies
- less queries when creating record
- cleaner creation messages
- less occurence of mail_create_nolog ctx key

Removing tracking at create could break the creation subtypes
mechanism (example: following task creation subtype on project)
Instead of using _track_subtype to give a subtype at create,
a new _creation_subtype method can be override. If a creation
subtype is set on a specific modlel, creation messages will be
create by message_post instead of _message_log.

We also need to adapt the message_track_post_template in order to
keep this feature whithout tracking.

Task: #1916916

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-remove-tracking-at-create-xdo branch from d565570 to 6137cc5 Mar 29, 2019

@robodoo robodoo removed the CI 🤖 label Mar 29, 2019

@tde-banana-odoo

This comment has been minimized.

Copy link
Contributor

tde-banana-odoo commented Mar 29, 2019

robodoo pushed a commit that referenced this pull request Mar 29, 2019

[IMP] mail, *: don't track fields at create
*: project, crm, maintenance, helpdesk,

It is useless to track fields during create since they
have no initial value and future tracking message will
show changes on tracked field.

We can log a default creation message instead
(as it is now if there is no mail_create_nolog context key)

This change will implies
- less queries when creating record
- cleaner creation messages
- less occurence of mail_create_nolog ctx key

Removing tracking at create could break the creation subtypes
mechanism (example: following task creation subtype on project)
Instead of using _track_subtype to give a subtype at create,
a new _creation_subtype method can be override. If a creation
subtype is set on a specific modlel, creation messages will be
create by message_post instead of _message_log.

We also need to adapt the message_track_post_template in order to
keep this feature whithout tracking.

Task: #1916916

closes #31945

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 29, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 29, 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.