[IMP] onchange_helper: Improve performance when called on an existing record

Before this change, the inmemory record was populated from values copied from the exisitng record. It was no predictable to know in advance which values were required to correctly play onchange methods. Therefore all the values defined on the model were copied from the exising record to populate the inmemory record. The side effect of this approach was that in a lot of cases, a lot of useless values was copied leading to performance issue with computed fields. With this change, we use the current record to call the onchange methods in an onchange context to avoid direct write to the database each time a new value is assigned by an onchange. At the end of the process, we restore the current record to its original state and return a dictionary with only the fields modified by the onchange.
6 years ago · 3b6dd12bcd
32 changed files with 1069 additions and 57 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -25,8 +25,10 @@ env:
  matrix:
  - LINT_CHECK="1"
  - TRANSIFEX="1"
-  - TESTS="1" ODOO_REPO="odoo/odoo" EXCLUDE="database_cleanup,auth_session_timeout,auth_brute_force,base_import_odoo"
-  - TESTS="1" ODOO_REPO="OCA/OCB" EXCLUDE="database_cleanup,auth_session_timeout,auth_brute_force,base_import_odoo"
+  - TESTS="1" ODOO_REPO="odoo/odoo" EXCLUDE="database_cleanup,auth_session_timeout,auth_brute_force,base_import_odoo,onchange_helper,test_onchange_helper"
+  - TESTS="1" ODOO_REPO="OCA/OCB" EXCLUDE="database_cleanup,auth_session_timeout,auth_brute_force,base_import_odoo,onchange_helper,test_onchange_helper"
+  - TESTS="1" ODOO_REPO="odoo/odoo" INCLUDE="onchange_helper,test_onchange_helper"
+  - TESTS="1" ODOO_REPO="OCA/OCB" INCLUDE="onchange_helper,test_onchange_helper"
  - TESTS="1" ODOO_REPO="odoo/odoo" INCLUDE="database_cleanup"
  - TESTS="1" ODOO_REPO="OCA/OCB" INCLUDE="database_cleanup"
  - TESTS="1" ODOO_REPO="odoo/odoo" INCLUDE="auth_session_timeout"
--- a/onchange_helper/README.rst
+++ b/onchange_helper/README.rst
@ -56,6 +56,7 @@ Contributors

 * Guewen Baconnier <guewen.baconnier@camptocamp.com>
 * Florian da Costa <florian.dacosta@akretion.com>
+* Laurent Mignon <laurent.mignon@acsone.eu>

 Maintainer
 ----------
--- a/onchange_helper/manifest.py
+++ b/onchange_helper/manifest.py
@ -3,10 +3,11 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

 {'name': 'Onchange Helper',
- 'version': '10.0.1.0.0',
+ 'version': '10.0.2.0.0',
 'summary': 'Technical module that ease execution of onchange in Python code',
- 'author': 'Akretion,Camptocamp,Odoo Community Association (OCA)',
- 'website': 'http://www.akretion.com',
+ 'author': 'Akretion,Camptocamp, ACSONE SA/NV, '
+           'Odoo Community Association (OCA)',
+ 'website': 'https://github.com/OCA/server-tools',
 'license': 'AGPL-3',
 'category': 'Generic Modules',
 'depends': ['base'],
--- a/onchange_helper/models/models.py
+++ b/onchange_helper/models/models.py
@ -1,51 +1,219 @@
 # -*- coding: utf-8 -*-
 # © 2016-2017 Akretion (http://www.akretion.com)
 # © 2016-2017 Camptocamp (http://www.camptocamp.com/)
+# © 2019 ACSONE SA/NV
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

-from odoo import api, models
+from odoo import api, models, fields


 class Base(models.AbstractModel):
-    _inherit = 'base'
+    _inherit = "base"

    @api.model
-    def _get_new_values(self, record, on_change_result):
-        vals = on_change_result.get('value', {})
-        new_values = {}
-        for fieldname, value in vals.iteritems():
-            if fieldname not in record:
-                column = self._fields[fieldname]
-                if value and column.type == 'many2one':
-                    value = value[0]  # many2one are tuple (id, name)
-                new_values[fieldname] = value
-        return new_values
-
-    def play_onchanges(self, values, onchange_fields):
-        onchange_specs = self._onchange_spec()
-        # we need all fields in the dict even the empty ones
-        # otherwise 'onchange()' will not apply changes to them
-        all_values = values.copy()
-
-        # If self is a record (play onchange on existing record)
-        # we take the value of the field
-        # If self is an empty record we will have an empty value
+    def _compute_onchange_dirty(
+        self, original_record, modified_record, fieldname_onchange=None
+    ):
+        """
+        Return the list of dirty fields. (designed to be called by
+        play_onchanges)
+        The list of dirty fields is computed from the list marked as dirty
+        on the record. Form this list, we remove the fields for which the value
+        into the original record is the same as the one into the current record
+        :param original_record:
+        :return: changed values
+        """
+        dirties = []
+        if fieldname_onchange:
+            for field_name, field in modified_record._fields.items():
+                # special case. We consider that a related field is modified
+                # if a modified field is in the first position of the path
+                # to traverse to get the value.
+                if field.related and field.related[0].startswith(
+                    fieldname_onchange
+                ):
+                    dirties.append(field_name)
+        for field_name in modified_record._get_dirty():
+            original_value = original_record[field_name]
+            new_value = modified_record[field_name]
+            if original_value == new_value:
+                continue
+            dirties.append(field_name)
+        for field_name, field in modified_record._fields.items():
+            new_value = modified_record[field_name]
+            if field.type not in ("one2many", "many2many"):
+                continue
+            # if the field is a one2many or many2many, check that any
+            # item is a new Id
+            if models.NewId in [type(i.id) for i in new_value]:
+                dirties.append(field_name)
+                continue
+            # if the field is a one2many or many2many, check if any item
+            # is dirty
+            for r in new_value:
+                if r._get_dirty():
+                    ori = [
+                        r1
+                        for r1 in original_record[field_name]
+                        if r1.id == r.id
+                    ][0]
+                    # if fieldname_onchange is None avoid recurssion...
+                    if fieldname_onchange and self._compute_onchange_dirty(
+                        ori, r
+                    ):
+                        dirties.append(field_name)
+                        break
+        return dirties
+
+    def _convert_to_onchange(self, record, field, value):
+        if field.type == "many2one":
+            # for many2one, we keep the id and don't call the
+            # convert_on_change to avoid the call to name_get by the
+            # convert_to_onchange
+            if value.id:
+                return value.id
+            return False
+        elif field.type in ("one2many", "many2many"):
+            result = [(5,)]
+            for record in value:
+                vals = {}
+                for name in record._cache:
+                    if name in models.LOG_ACCESS_COLUMNS:
+                        continue
+                    v = record[name]
+                    f = record._fields[name]
+                    if f.type == "many2one" and isinstance(v.id, models.NewId):
+                        continue
+                    vals[name] = self._convert_to_onchange(record, f, v)
+                if not record.id:
+                    result.append((0, 0, vals))
+                elif vals:
+                    result.append((1, record.id, vals))
+                else:
+                    result.append((4, record.id))
+            return result
+        else:
+            return field.convert_to_onchange(value, record, [field.name])
+
+    def play_onchanges(self, values, onchange_fields=None):
+        """
+        Play the onchange methods defined on the current record and return the
+        changed values.
+        The record is not modified by the onchange.
+
+        The intend of this method is to provide a way to get on the server side
+        the values returned by the onchange methods when called by the UI.
+        This method is useful in B2B contexts where records are created or
+        modified from server side.
+
+        The returned values are those changed by the execution of onchange
+        methods registered for the onchange_fields according to the provided
+        values. As consequence, the result will not contain the modifications
+        that could occurs by the execution of compute methods registered for
+        the same onchange_fields.
+
+        It's on purpose that we avoid to trigger the compute methods for the
+        onchange_fields. These compute methods will be triggered when calling
+        the create or write method. In this way we avoid to compute useless
+        information.
+
+
+        :param values: dict of input value that
+        :param onchange_fields: fields for which onchange methods will be
+        played. If not provided, the list of field is based on the values keys.
+        Order in onchange_fields is very important as onchanges methods will
+        be played in that order.
+        :return: changed values
+
+        This method reimplement the onchange method to be able to work on the
+        current recordset if provided.
+        """
+        env = self.env
        if self:
            self.ensure_one()
-            record_values = self._convert_to_write(self.read()[0])
-        else:
-            record_values = {}
-        for field in self._fields:
-            if field not in all_values:
-                all_values[field] = record_values.get(field, False)
-
-        new_values = {}
-        for field in onchange_fields:
-            onchange_values = self.onchange(all_values, field, onchange_specs)
-            new_values.update(self._get_new_values(values, onchange_values))
-            all_values.update(new_values)
-
-        return {
-            f: v for f, v in all_values.iteritems()
-            if not (self._fields[f].compute and not self._fields[f].inverse)
-            and (f in values or f in new_values)}
+
+        if not isinstance(onchange_fields, list):
+            onchange_fields = [onchange_fields]
+
+        if not onchange_fields:
+            onchange_fields = values.keys()
+
+        # filter out keys in field_onchange that do not refer to actual fields
+        names = [n for n in onchange_fields if n in self._fields]
+
+        # create a new record with values, and attach ``self`` to it
+        with env.do_in_onchange():
+            # keep a copy of the original record.
+            # attach ``self`` with a different context (for cache consistency)
+            origin = self.with_context(__onchange=True)
+            origin_dirty = set(self._get_dirty())
+            fields.copy_cache(self, origin.env)
+            if self:
+                record = self
+                record.update(values)
+            else:
+                # initialize with default values, they may be used in onchange
+                new_values = self.default_get(self._fields.keys())
+                new_values.update(values)
+                record = self.new(new_values)
+            values = {name: record[name] for name in record._cache}
+            record._origin = origin
+
+        # determine which field(s) should be triggered an onchange
+        todo = list(names) or list(values)
+        done = set()
+
+        # dummy assignment: trigger invalidations on the record
+        with env.do_in_onchange():
+            for name in todo:
+                if name == "id":
+                    continue
+                value = record[name]
+                field = self._fields[name]
+                if field.type == "many2one" and field.delegate and not value:
+                    # do not nullify all fields of parent record for new
+                    # records
+                    continue
+                record[name] = value
+
+        dirty = set()
+
+        # process names in order (or the keys of values if no name given)
+        while todo:
+            name = todo.pop(0)
+            if name in done:
+                continue
+            done.add(name)
+
+            with env.do_in_onchange():
+                # apply field-specific onchange methods
+                record._onchange_eval(name, "1", {})
+
+                # determine which fields have been modified
+                dirties = self._compute_onchange_dirty(origin, record, name)
+                dirty |= set(dirties)
+                todo.extend(dirties)
+
+        # prepare the result to return a dictionary with the new values for
+        # the dirty fields
+        result = {}
+        for name in dirty:
+            field = self._fields[name]
+            value = record[name]
+            if field.type == "many2one" and isinstance(value.id, models.NewId):
+                continue
+            result[name] = self._convert_to_onchange(record, field, value)
+
+        # reset dirty values into the current record
+        if self:
+            to_reset = dirty | set(values.keys())
+            with env.do_in_onchange():
+                for name in to_reset:
+                    original = origin[name]
+                    new = self[name]
+                    if original == new:
+                        continue
+                    self[name] = origin[name]
+                    env.dirty[record].discard(name)
+            env.dirty[record].update(origin_dirty)
+        return result
--- a/onchange_helper/tests/test_onchange.py
+++ b/onchange_helper/tests/test_onchange.py
@ -1,26 +1,64 @@
 # -*- coding: utf-8 -*-
 # Copyright 2018 Akretion (http://www.akretion.com).
+# Copyright 2019 ACSONE SA/NV
 # @author Sébastien BEAU <sebastien.beau@akretion.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

-import openerp.tests.common as common
+import mock
+import odoo.tests.common as common


 class TestOnchange(common.TransactionCase):

    def test_playing_onchange_on_model(self):
-        result = self.env['res.partner'].play_onchanges({
-            'company_type': 'company',
-            }, ['company_type'])
-        self.assertEqual(result['is_company'], True)
+        res_partner = self.env["res.partner"]
+        with mock.patch.object(
+            res_partner.__class__, "write"
+        ) as patched_write:
+            result = self.env["res.partner"].play_onchanges(
+                {"company_type": "company"}, ["company_type"]
+            )
+            patched_write.assert_not_called()
+        self.assertEqual(result["is_company"], True)

    def test_playing_onchange_on_record(self):
-        company = self.env.ref('base.main_company')
-        result = company.play_onchanges({
-            'email': 'contact@akretion.com'},
-            ['email'])
+        company = self.env.ref("base.main_company")
+        with mock.patch.object(company.__class__, "write") as patched_write:
+            result = company.play_onchanges(
+                {"email": "contact@akretion.com"}, ["email"]
+            )
+            patched_write.assert_not_called()
+        modified_fields = set(result.keys())
+        self.assertSetEqual(
+            modified_fields, {"rml_footer", "rml_footer_readonly"}
+        )
        self.assertEqual(
-            result['rml_footer'],
-            u'Phone: +1 555 123 8069 | Email: contact@akretion.com | '
-            u'Website: http://www.example.com')
-        self.assertEqual(company.email, u'info@yourcompany.example.com')
+            result["rml_footer"],
+            u"Phone: +1 555 123 8069 | Email: contact@akretion.com | "
+            u"Website: http://www.example.com",
+        )
+        self.assertEqual(result["rml_footer_readonly"], result["rml_footer"])
+
+        # check that the original record is not modified
+        self.assertFalse(company._get_dirty())
+        self.assertEqual(company.email, u"info@yourcompany.example.com")
+
+    def test_onchange_record_with_dirty_field(self):
+        company = self.env.ref("base.main_company")
+        company._set_dirty("name")
+        self.assertListEqual(company._get_dirty(), ["name"])
+        company.play_onchanges({"email": "contact@akretion.com"}, ["email"])
+        self.assertListEqual(company._get_dirty(), ["name"])
+
+    def test_onchange_wrong_key(self):
+        res_partner = self.env["res.partner"]
+        with mock.patch.object(
+                res_partner.__class__, "write"
+        ) as patched_write:
+            # we specify a wrong field name... This field should be
+            # ignored
+            result = self.env["res.partner"].play_onchanges(
+                {"company_type": "company"}, ["company_type", "wrong_key"]
+            )
+            patched_write.assert_not_called()
+        self.assertEqual(result["is_company"], True)
--- a/setup/test_onchange_helper/odoo/init.py
+++ b/setup/test_onchange_helper/odoo/init.py
@ -0,0 +1 @@
+__import__('pkg_resources').declare_namespace(__name__)
--- a/setup/test_onchange_helper/odoo/addons/init.py
+++ b/setup/test_onchange_helper/odoo/addons/init.py
@ -0,0 +1 @@
+__import__('pkg_resources').declare_namespace(__name__)
--- a/setup/test_onchange_helper/odoo/addons/test_onchange_helper
+++ b/setup/test_onchange_helper/odoo/addons/test_onchange_helper
@ -0,0 +1 @@
+../../../../test_onchange_helper
--- a/setup/test_onchange_helper/setup.py
+++ b/setup/test_onchange_helper/setup.py
@ -0,0 +1,6 @@
+import setuptools
+
+setuptools.setup(
+    setup_requires=['setuptools-odoo'],
+    odoo_addon=True,
+)
--- a/test_onchange_helper/README.rst
+++ b/test_onchange_helper/README.rst
@ -0,0 +1,9 @@
+.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
+   :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
+   :alt: License: AGPL-3
+
+====================
+Onchange Helper TEST
+====================
+
+*This module is only intended to test the onchange_helper addon.*
--- a/test_onchange_helper/init.py
+++ b/test_onchange_helper/init.py
@ -0,0 +1 @@
+from . import models
--- a/test_onchange_helper/manifest.py
+++ b/test_onchange_helper/manifest.py
@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+{
+    "name": "Onchange Helper TEST",
+    "summary": """
+        Test addon for the onchange_helper addon""",
+    "version": "10.0.1.0.0",
+    "license": "AGPL-3",
+    "author": "ACSONE SA/NV,Odoo Community Association (OCA)",
+    "website": "https://acsone.eu/",
+    "depends": ["onchange_helper"],
+    "data": [
+        'security/test_onchange_helper_multi_line.xml',
+        'security/test_onchange_helper_multi.xml',
+        "security/test_onchange_helper_emailmessage.xml",
+        "security/test_onchange_helper_message.xml",
+        "security/test_onchange_helper_discussion.xml",
+        "security/test_onchange_helper_category.xml",
+    ],
+    "demo": [
+        "demo/test_onchange_helper_discussion.xml",
+        "demo/test_onchange_helper_message.xml",
+    ]
+}
--- a/test_onchange_helper/demo/test_onchange_helper_discussion.xml
+++ b/test_onchange_helper/demo/test_onchange_helper_discussion.xml
@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo noupdate="1">
+
+     <record id="discussion_demo_0" model="test_onchange_helper.discussion">
+            <field name="name">Stuff</field>
+            <field name="participants" eval="[(4, ref('base.user_root')), (4, ref('base.user_demo'))]"/>
+     </record>
+
+</odoo>
--- a/test_onchange_helper/demo/test_onchange_helper_message.xml
+++ b/test_onchange_helper/demo/test_onchange_helper_message.xml
@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo noupdate="1">
+
+     <record id="message_demo_0_0" model="test_onchange_helper.message">
+        <field name="discussion" ref="discussion_demo_0"/>
+        <field name="body">Hey dude!</field>
+    </record>
+
+    <record id="message_demo_0_1" model="test_onchange_helper.message">
+        <field name="discussion" ref="discussion_demo_0"/>
+        <field name="author" ref="base.user_demo"/>
+        <field name="body">What's up?</field>
+    </record>
+
+    <record id="message_demo_0_2" model="test_onchange_helper.message">
+        <field name="discussion" ref="discussion_demo_0"/>
+        <field name="body">This is a much longer message</field>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/models/init.py
+++ b/test_onchange_helper/models/init.py
@ -0,0 +1,6 @@
+from . import test_onchange_helper_category
+from . import test_onchange_helper_discussion
+from . import test_onchange_helper_message
+from . import test_onchange_helper_emailmessage
+from . import test_onchange_helper_multi
+from . import test_onchange_helper_multi_line
--- a/test_onchange_helper/models/test_onchange_helper_category.py
+++ b/test_onchange_helper/models/test_onchange_helper_category.py
@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import api, fields, models
+
+
+class TestOnchangeHelperCategory(models.Model):
+
+    _name = "test_onchange_helper.category"
+    _description = "Test Onchange Helper Category"
+
+    name = fields.Char(required=True)
+    parent = fields.Many2one(_name)
+    root_categ = fields.Many2one(_name)
+    display_name = fields.Char()
+    computed_display_name = fields.Char(
+        compute="_compute_computed_display_name"
+    )
+
+    @api.onchange("name", "parent")
+    def _onchange_name_or_parent(self):
+        if self.parent:
+            self.display_name = self.parent.display_name + " / " + self.name
+        else:
+            self.display_name = self.name
+
+    @api.onchange("parent")
+    def _onchange_parent(self):
+        current = self
+        while current.parent:
+            current = current.parent
+        if current == self:
+            self.root_categ = False
+        else:
+            self.root_categ = current
+
+    @api.depends("name", "parent.display_name")
+    def _compute_computed_display_name(self):
+        for cat in self:
+            if cat.parent:
+                self.display_name = cat.parent.display_name + " / " + cat.name
+            else:
+                cat.display_name = cat.name
--- a/test_onchange_helper/models/test_onchange_helper_discussion.py
+++ b/test_onchange_helper/models/test_onchange_helper_discussion.py
@ -0,0 +1,72 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import api, fields, models
+
+
+class TestOnchangeHelperDiscussion(models.Model):
+
+    _name = "test_onchange_helper.discussion"
+    _description = "Test Onchange Helper Discussion"
+
+    name = fields.Char(
+        string="Title",
+        required=True,
+        help="General description of what this discussion is about.",
+    )
+    moderator = fields.Many2one("res.users")
+    categories = fields.Many2many(
+        "test_onchange_helper.category",
+        "test_onchange_helper_discussion_category",
+        "discussion",
+        "category",
+    )
+    participants = fields.Many2many("res.users")
+    messages = fields.One2many("test_onchange_helper.message", "discussion")
+    message_concat = fields.Text(string="Message concatenate")
+    important_messages = fields.One2many(
+        "test_onchange_helper.message",
+        "discussion",
+        domain=[("important", "=", True)],
+    )
+    very_important_messages = fields.One2many(
+        "test_onchange_helper.message",
+        "discussion",
+        domain=lambda self: self._domain_very_important(),
+    )
+    emails = fields.One2many("test_onchange_helper.emailmessage", "discussion")
+    important_emails = fields.One2many(
+        "test_onchange_helper.emailmessage",
+        "discussion",
+        domain=[("important", "=", True)],
+    )
+
+    def _domain_very_important(self):
+        """Ensure computed O2M domains work as expected."""
+        return [("important", "=", True)]
+
+    @api.onchange("name")
+    def _onchange_name(self):
+        # test onchange modifying one2many field values
+        # update body of existings messages and emails
+        for message in self.messages:
+            message.body = "not last dummy message"
+        for message in self.important_messages:
+            message.body = "not last dummy message"
+        # add new dummy message
+        message_vals = self.messages._add_missing_default_values(
+            {"body": "dummy message", "important": True}
+        )
+        self.messages |= self.messages.new(message_vals)
+        self.important_messages |= self.messages.new(message_vals)
+
+    @api.onchange("moderator")
+    def _onchange_moderator(self):
+        self.participants |= self.moderator
+
+    @api.onchange("messages")
+    def _onchange_messages(self):
+        self.message_concat = "\n".join(
+            ["%s:%s" % (m.name, m.body) for m in self.messages]
+        )
--- a/test_onchange_helper/models/test_onchange_helper_emailmessage.py
+++ b/test_onchange_helper/models/test_onchange_helper_emailmessage.py
@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import fields, models
+
+
+class TestOnchangeHelperEmailmessage(models.Model):
+
+    _name = "test_onchange_helper.emailmessage"
+    _description = "Test Onchange Helper Emailmessage"
+    _inherits = {"test_onchange_helper.message": "message"}
+
+    message = fields.Many2one(
+        "test_onchange_helper.message",
+        "Message",
+        required=True,
+        ondelete="cascade",
+    )
+    email_to = fields.Char("To")
--- a/test_onchange_helper/models/test_onchange_helper_message.py
+++ b/test_onchange_helper/models/test_onchange_helper_message.py
@ -0,0 +1,57 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import api, fields, models
+
+
+class TestOnchangeHelperMessage(models.Model):
+
+    _name = "test_onchange_helper.message"
+    _description = "Test Onchange Helper Message"
+
+    discussion = fields.Many2one(
+        "test_onchange_helper.discussion", ondelete="cascade"
+    )
+    body = fields.Text()
+    author = fields.Many2one("res.users", default=lambda self: self.env.user)
+    name = fields.Char(string="Title", compute="_compute_name", store=True)
+    display_name = fields.Char(
+        string="Abstract", compute="_compute_display_name"
+    )
+    discussion_name = fields.Char(
+        related="discussion.name", string="Discussion Name"
+    )
+    author_partner = fields.Many2one(
+        "res.partner",
+        compute="_compute_author_partner",
+        search="_search_author_partner",
+    )
+    important = fields.Boolean()
+
+    @api.one
+    @api.depends("author.name", "discussion.name")
+    def _compute_name(self):
+        self.name = "[%s] %s" % (
+            self.discussion.name or "",
+            self.author.name or "",
+        )
+
+    @api.one
+    @api.depends("author.name", "discussion.name", "body")
+    def _compute_display_name(self):
+        stuff = "[%s] %s: %s" % (
+            self.author.name,
+            self.discussion.name or "",
+            self.body or "",
+        )
+        self.display_name = stuff[:80]
+
+    @api.one
+    @api.depends("author", "author.partner_id")
+    def _compute_author_partner(self):
+        self.author_partner = self.author.partner_id
+
+    @api.model
+    def _search_author_partner(self, operator, value):
+        return [("author.partner_id", operator, value)]
--- a/test_onchange_helper/models/test_onchange_helper_multi.py
+++ b/test_onchange_helper/models/test_onchange_helper_multi.py
@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import api, fields, models
+
+
+class TestOnchangeHelperMulti(models.Model):
+
+    _name = "test_onchange_helper.multi"
+    _description = "Test Onchange Helper Multi"
+
+    name = fields.Char(related="partner.name", readonly=True)
+    partner = fields.Many2one("res.partner")
+    lines = fields.One2many("test_onchange_helper.multi.line", "multi")
+
+    @api.onchange("name")
+    def _onchange_name(self):
+        for line in self.lines:
+            line.name = self.name
+
+    @api.onchange("partner")
+    def _onchange_partner(self):
+        for line in self.lines:
+            line.partner = self.partner
--- a/test_onchange_helper/models/test_onchange_helper_multi_line.py
+++ b/test_onchange_helper/models/test_onchange_helper_multi_line.py
@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+from odoo import fields, models
+
+
+class TestOnchangeHelperMultiLine(models.Model):
+
+    _name = "test_onchange_helper.multi.line"
+    _description = "Test Onchange Helper Multi Line"
+
+    multi = fields.Many2one("test_onchange_helper.multi", ondelete="cascade")
+    name = fields.Char()
+    partner = fields.Many2one("res.partner")
--- a/test_onchange_helper/readme/CONTRIBUTORS.rst
+++ b/test_onchange_helper/readme/CONTRIBUTORS.rst
@ -0,0 +1 @@
+* Laurent Mignon <laurent.mignon@acsone.eu> (https://acsone.eu)
--- a/test_onchange_helper/readme/DESCRIPTION.rst
+++ b/test_onchange_helper/readme/DESCRIPTION.rst
@ -0,0 +1 @@
+*This module is only intended to test the onchange_helper addon.*
--- a/test_onchange_helper/security/test_onchange_helper_category.xml
+++ b/test_onchange_helper/security/test_onchange_helper_category.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_category_access_name">
+        <field name="name">test_onchange_helper.category access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_category"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="1"/>
+        <field name="perm_write" eval="1"/>
+        <field name="perm_unlink" eval="1"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/security/test_onchange_helper_discussion.xml
+++ b/test_onchange_helper/security/test_onchange_helper_discussion.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_discussion_access_name">
+        <field name="name">test_onchange_helperdiscussion access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_discussion"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="1"/>
+        <field name="perm_write" eval="1"/>
+        <field name="perm_unlink" eval="1"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/security/test_onchange_helper_emailmessage.xml
+++ b/test_onchange_helper/security/test_onchange_helper_emailmessage.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_emailmessage_access_name">
+        <field name="name">test_onchange_helperemailmessage access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_emailmessage"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="1"/>
+        <field name="perm_write" eval="1"/>
+        <field name="perm_unlink" eval="1"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/security/test_onchange_helper_message.xml
+++ b/test_onchange_helper/security/test_onchange_helper_message.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_message_access_name">
+        <field name="name">test_onchange_helper.message access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_message"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="0"/>
+        <field name="perm_write" eval="0"/>
+        <field name="perm_unlink" eval="0"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/security/test_onchange_helper_multi.xml
+++ b/test_onchange_helper/security/test_onchange_helper_multi.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_multi_access_name">
+        <field name="name">test.onchange.helper.multi access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_multi"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="0"/>
+        <field name="perm_write" eval="0"/>
+        <field name="perm_unlink" eval="0"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/security/test_onchange_helper_multi_line.xml
+++ b/test_onchange_helper/security/test_onchange_helper_multi_line.xml
@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 ACSONE SA/NV
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
+
+<odoo>
+
+    <record model="ir.model.access" id="test_onchange_helper_multi_line_access_name">
+        <field name="name">test.onchange.helper.multi.line access name</field>
+        <field name="model_id" ref="model_test_onchange_helper_multi_line"/>
+        <field name="group_id" ref="base.group_user"/>
+        <field name="perm_read" eval="1"/>
+        <field name="perm_create" eval="0"/>
+        <field name="perm_write" eval="0"/>
+        <field name="perm_unlink" eval="0"/>
+    </record>
+
+</odoo>
--- a/test_onchange_helper/static/description/icon.png
+++ b/test_onchange_helper/static/description/icon.png
--- a/test_onchange_helper/tests/init.py
+++ b/test_onchange_helper/tests/init.py
@ -0,0 +1 @@
+from . import test_onchange_helper
--- a/test_onchange_helper/tests/test_onchange_helper.py
+++ b/test_onchange_helper/tests/test_onchange_helper.py
@ -0,0 +1,378 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+
+import mock
+from contextlib import contextmanager
+import odoo.tests.common as common
+
+
+class TestOnchangeHelper(common.TransactionCase):
+    def setUp(self):
+        super(TestOnchangeHelper, self).setUp()
+        self.Category = self.env["test_onchange_helper.category"]
+        self.Message = self.env["test_onchange_helper.message"]
+        self.Discussion = self.env["test_onchange_helper.discussion"]
+
+    @contextmanager
+    def assertNoOrmWrite(self, model):
+        with mock.patch.object(
+            model.__class__, "create"
+        ) as mocked_create, mock.patch.object(
+            model.__class__, "write"
+        ) as mocked_write:
+            yield
+            mocked_create.assert_not_called()
+            mocked_write.assert_not_called()
+
+    def test_play_onhanges_no_recompute(self):
+        # play_onchanges must not trigger recomputes except if an onchange
+        # method access a computed field.
+        # changing 'discussion' should recompute 'name'
+        values = {"name": "Cat Name"}
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(self.Category):
+            result = self.Category.play_onchanges(values, ["name"])
+        self.assertNotIn("computed_display_name", result)
+
+    def test_play_onchanges_many2one_new_record(self):
+        root = self.Category.create({"name": "root"})
+
+        values = {"name": "test", "parent": root.id, "root_categ": False}
+
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(self.Category):
+            result = self.Category.play_onchanges(values, "parent")
+        self.assertIn("root_categ", result)
+        self.assertEqual(result["root_categ"], root.id)
+
+        values.update(result)
+        values["parent"] = False
+
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(self.Category):
+            result = self.Category.play_onchanges(values, "parent")
+        # since the root_categ is already False into values the field is not
+        # changed by the onchange
+        self.assertNotIn("root_categ", result)
+
+    def test_play_onchanges_many2one_existing_record(self):
+        root = self.Category.create({"name": "root"})
+
+        values = {"name": "test", "parent": root.id, "root_categ": False}
+
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(self.Category):
+            result = self.Category.play_onchanges(values, "parent")
+        self.assertIn("root_categ", result)
+        self.assertEqual(result["root_categ"], root.id)
+
+        # create child catefory
+        values.update(result)
+        child = self.Category.create(values)
+        self.assertEqual(root.id, child.root_categ.id)
+
+        # since the parent is set to False and the root_categ
+        values = {"parent": False}
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(child):
+            result = child.play_onchanges(values, "parent")
+
+        self.assertIn("root_categ", result)
+        self.assertEqual(result["root_categ"], False)
+
+    def test_play_onchange_one2many_new_record(self):
+        """ test the effect of play_onchanges() on one2many fields on new
+        record"""
+        BODY = "What a beautiful day!"
+        USER = self.env.user
+
+        # create an independent message
+        message = self.Message.create({"body": BODY})
+
+        # modify discussion name
+        values = {
+            "name": "Foo",
+            "categories": [],
+            "moderator": False,
+            "participants": [],
+            "messages": [
+                (4, message.id),
+                (
+                    0,
+                    0,
+                    {
+                        "name": "[%s] %s" % ("", USER.name),
+                        "body": BODY,
+                        "author": USER.id,
+                        "important": False,
+                    },
+                ),
+            ],
+        }
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(self.Discussion):
+            result = self.Discussion.play_onchanges(values, "name")
+        self.assertIn("messages", result)
+        self.assertItemsEqual(
+            result["messages"],
+            [
+                (5,),
+                (
+                    1,
+                    message.id,
+                    {
+                        "name": "[%s] %s" % ("Foo", USER.name),
+                        "body": "not last dummy message",
+                        "author": message.author.id,
+                        "important": message.important,
+                    },
+                ),
+                (
+                    0,
+                    0,
+                    {
+                        "name": "[%s] %s" % ("Foo", USER.name),
+                        "body": "not last dummy message",
+                        "author": USER.id,
+                        "important": False,
+                    },
+                ),
+                (
+                    0,
+                    0,
+                    {
+                        "name": "[%s] %s" % ("Foo", USER.name),
+                        "body": "dummy message",
+                        "author": USER.id,
+                        "important": True,
+                    },
+                ),
+            ],
+        )
+
+        self.assertIn("important_messages", result)
+        self.assertItemsEqual(
+            result["important_messages"],
+            [
+                (5,),
+                (
+                    0,
+                    0,
+                    {
+                        "author": USER.id,
+                        "body": "dummy message",
+                        "important": True,
+                    },
+                ),
+            ],
+        )
+
+    def test_play_onchange_one2many_existing_record(self):
+        """ test the effect of play_onchanges() on one2many fields on existing
+        record"""
+        BODY = "What a beautiful day!"
+        USER = self.env.user
+
+        # create an independent message
+        message = self.Message.create({"body": BODY})
+
+        # modify discussion name
+        values = {
+            "name": "Foo",
+            "categories": [],
+            "moderator": False,
+            "participants": [],
+            "messages": [
+                (4, message.id),
+                (
+                    0,
+                    0,
+                    {
+                        "name": "[%s] %s" % ("", USER.name),
+                        "body": BODY,
+                        "author": USER.id,
+                        "important": False,
+                    },
+                ),
+            ],
+        }
+        discussion = self.Discussion.create(values)
+
+        values = {"name": "New foo"}
+        with self.assertNoOrmWrite(discussion):
+            result = discussion.play_onchanges(values, "name")
+        self.assertIn("messages", result)
+        self.assertItemsEqual(
+            result["messages"],
+            [
+                (5,),
+                (
+                    1,
+                    discussion.messages[0].id,
+                    {
+                        "name": "[%s] %s" % ("New foo", USER.name),
+                        "body": "not last dummy message",
+                        "author": message.author.id,
+                        "important": message.important,
+                        "discussion": discussion.id,
+                    },
+                ),
+                (
+                    1,
+                    discussion.messages[1].id,
+                    {
+                        "name": "[%s] %s" % ("New foo", USER.name),
+                        "body": "not last dummy message",
+                        "author": USER.id,
+                        "important": False,
+                        "discussion": discussion.id,
+                    },
+                ),
+                (
+                    0,
+                    0,
+                    {
+                        "name": "[%s] %s" % ("New foo", USER.name),
+                        "body": "dummy message",
+                        "author": USER.id,
+                        "important": True,
+                        "discussion": discussion.id,
+                    },
+                ),
+            ],
+        )
+
+        self.assertIn("important_messages", result)
+        self.assertItemsEqual(
+            result["important_messages"],
+            [
+                (5,),
+                (
+                    0,
+                    0,
+                    {
+                        "author": USER.id,
+                        "body": "dummy message",
+                        "important": True,
+                        "discussion": discussion.id,
+                    },
+                ),
+            ],
+        )
+
+    def test_onchange_specific(self):
+        """test that only the id is added if a new item is added to an
+        existing relation"""
+        discussion = self.env.ref("test_onchange_helper.discussion_demo_0")
+        demo = self.env.ref("base.user_demo")
+
+        # first remove demo user from participants
+        discussion.participants -= demo
+        self.assertNotIn(demo, discussion.participants)
+
+        # check that demo_user is added to participants when set as moderator
+        values = {
+            "name": discussion.name,
+            "moderator": demo.id,
+            "categories": [(4, cat.id) for cat in discussion.categories],
+            "messages": [(4, msg.id) for msg in discussion.messages],
+            "participants": [(4, usr.id) for usr in discussion.participants],
+        }
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(discussion):
+            result = discussion.play_onchanges(values, "moderator")
+
+        self.assertIn("participants", result)
+        self.assertItemsEqual(
+            result["participants"],
+            [(5,)] + [(4, user.id) for user in discussion.participants + demo],
+        )
+
+    def test_onchange_one2many_value(self):
+        """ test that the values provided for a one2many field inside are used
+        by the play_onchanges  """
+        discussion = self.env.ref("test_onchange_helper.discussion_demo_0")
+        demo = self.env.ref("base.user_demo")
+
+        self.assertEqual(len(discussion.messages), 3)
+        messages = [(4, msg.id) for msg in discussion.messages]
+        messages[0] = (1, messages[0][1], {"body": "test onchange"})
+        values = {
+            "name": discussion.name,
+            "moderator": demo.id,
+            "categories": [(4, cat.id) for cat in discussion.categories],
+            "messages": messages,
+            "participants": [(4, usr.id) for usr in discussion.participants],
+            "message_concat": False,
+        }
+        with self.assertNoOrmWrite(discussion):
+            result = discussion.play_onchanges(values, "messages")
+        self.assertIn("message_concat", result)
+        self.assertEqual(
+            result["message_concat"],
+            "\n".join(
+                ["%s:%s" % (m.name, m.body) for m in discussion.messages]
+            ),
+        )
+
+    def test_onchange_one2many_line(self):
+        """ test that changes on a field used as first position into the
+        related path of a related field will trigger the onchange also on the
+        related field """
+        partner = self.env.ref("base.res_partner_1")
+        multi = self.env["test_onchange_helper.multi"].create(
+            {"partner": partner.id}
+        )
+        line = multi.lines.create({"multi": multi.id})
+
+        values = multi._convert_to_write(
+            {key: multi[key] for key in ("partner", "lines")}
+        )
+        self.assertEqual(
+            values, {"partner": partner.id, "lines": [(6, 0, [line.id])]}
+        )
+
+        # modify 'partner'
+        #   -> set 'partner' on all lines
+        #   -> recompute 'name' (related on partner)
+        #       -> set 'name' on all lines
+        partner = self.env.ref("base.res_partner_2")
+        values = {
+            "partner": partner.id,
+            "lines": [
+                (6, 0, [line.id]),
+                (0, 0, {"name": False, "partner": False}),
+            ],
+        }
+
+        self.env.invalidate_all()
+        with self.assertNoOrmWrite(multi):
+            result = multi.play_onchanges(values, "partner")
+        self.assertEqual(
+            result,
+            {
+                "name": partner.name,
+                "lines": [
+                    (5,),
+                    (
+                        1,
+                        line.id,
+                        {
+                            "name": partner.name,
+                            "partner": partner.id,
+                            "multi": multi.id,
+                        },
+                    ),
+                    (
+                        0,
+                        0,
+                        {
+                            "name": partner.name,
+                            "partner": partner.id,
+                            "multi": multi.id,
+                        },
+                    ),
+                ],
+            },
+        )