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: improve field tracking #41554

Open
wants to merge 13 commits into
base: master
from

Conversation

@rco-odoo
Copy link
Member

rco-odoo commented Dec 9, 2019

Improve tracking by generating the tracking message at flush.
Also implement the tracking for stored computed fields.

@rco-odoo rco-odoo requested a review from Xavier-Do Dec 9, 2019
@rco-odoo rco-odoo changed the title [IMP] mail: add tracking for computed fields [IMP] mail: improve field tracking Dec 9, 2019
@C3POdoo C3POdoo added the RD label Dec 9, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Dec 9, 2019
@rco-odoo rco-odoo requested a review from seb-odoo Dec 10, 2019
Copy link
Contributor

seb-odoo left a comment

Nothing really bad caught my attention, but the code was a bit tricky to understand I felt.

This also fixes task 2006854 so that's good.

It's still very inconsistent with translations however, but I guess this is outside the scope of this PR?
Basically when tracking the name, it only logs it when the source term is changed, and only from the record itself and not from the translation table.

I have not tested monetary that we know isn't logical with currencies, but also outside the scope I assume.

Maybe it would be nice to add a warning when trying to track a field type that is not supported (m2m, ...).

addons/mail/models/mail_thread.py Show resolved Hide resolved
while self.env.all.towrite:
model_name, id_vals = self.env.all.towrite.popitem()
process(self.env[model_name], id_vals)
for i in range(10):

This comment has been minimized.

Copy link
@seb-odoo

seb-odoo Dec 10, 2019

Contributor

This seems so arbitrary.

@rco-odoo rco-odoo force-pushed the odoo-dev:master-mail-tracking-computed-fields-rco branch 2 times, most recently from 0e356aa to 7654796 Dec 11, 2019
@robodoo robodoo added the CI 🤖 label Dec 11, 2019
@rco-odoo rco-odoo force-pushed the odoo-dev:master-mail-tracking-computed-fields-rco branch from 7654796 to 7886f51 Dec 11, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Dec 11, 2019
@rco-odoo rco-odoo force-pushed the odoo-dev:master-mail-tracking-computed-fields-rco branch from 7886f51 to c01a8a6 Dec 16, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Dec 16, 2019
rco-odoo added 8 commits Nov 26, 2019
Also delay the tracking until the call to flush(), so that several
updates on a record generate a single tracking message.
In order to count queries exactly, the warmup phase of a test method
(decorator `@warmup`) must do the exact same operations as the normal
phase.
@@ -539,20 +577,14 @@ def message_change_thread(self, new_thread):
# Automatic log / Tracking
# ------------------------------------------------------

@api.model
@tools.ormcache()

This comment has been minimized.

Copy link
@dbo-odoo

dbo-odoo Jan 13, 2020

Contributor

are you sure everything gets invalidated properly when modifying custom fields; e.g. writing on tracking in ir.model.fields?

@rco-odoo rco-odoo force-pushed the odoo-dev:master-mail-tracking-computed-fields-rco branch from c01a8a6 to 0ffe918 Jan 14, 2020
@robodoo robodoo removed the CI 🤖 label Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.