From cebf649760e1511523b5a7f48e4e22a8aec7b6b5 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Fri, 23 Oct 2015 16:57:47 +0200 Subject: [PATCH] Rules are applied also for manual edition --- partner_changeset/README.rst | 19 ++++++--- .../models/changeset_field_rule.py | 39 ++++++++----------- .../models/res_partner_changeset.py | 20 +++++----- .../tests/test_changeset_field_type.py | 22 +++++------ .../tests/test_changeset_flow.py | 38 ++++-------------- .../tests/test_changeset_origin.py | 24 ++++++------ 6 files changed, 70 insertions(+), 92 deletions(-) diff --git a/partner_changeset/README.rst b/partner_changeset/README.rst index 8a69bea41..1d45bef56 100644 --- a/partner_changeset/README.rst +++ b/partner_changeset/README.rst @@ -39,24 +39,31 @@ The supported fields are: * Boolean * Many2one +Rules can be global (no source model) or configured by source model. +Rules by source model have the priority. If a field is not configured +for the source model, it will use the global rule (if existing). + +If a field has no rule, it is written to the partner without changeset. + Usage ===== General case ------------ -When users modify the partners, new 'validated' changeset are created so -there is nothing to do. Addons wanting to create changeset which need a -validation should pass the key ``_changeset_rules`` in the context when -they write on the partner. +The first step is to create the changeset rules, once that done, -The keys a caller should pass in the context are: -* ``__changeset_rules``: activate the rules for the changesets +Addons wanting to create changeset with their own rules should pass the +following keys in the context when they write on the partner: * ``__changeset_rules_source_model``: name of the model which asks for the change * ``__changeset_rules_source_id``: id of the record which asks for the change +Also, they should extend the selection in +``ChangesetFieldRule._domain_source_models`` to add their model (the +same that is passed in ``__changeset_rules_source_model``). + The source is used for the application of the rules, it is also stored on the changeset for information. diff --git a/partner_changeset/models/changeset_field_rule.py b/partner_changeset/models/changeset_field_rule.py index 2346bd36b..1c8729419 100644 --- a/partner_changeset/models/changeset_field_rule.py +++ b/partner_changeset/models/changeset_field_rule.py @@ -19,8 +19,6 @@ # # -from itertools import chain - from openerp import models, fields, api from openerp.tools.cache import ormcache @@ -77,7 +75,7 @@ class ChangesetFieldRule(models.Model): Rules without model are global and apply for all models. """ - return self.env['ir.model'].browse() + return self.env.ref('base.model_res_users') @api.model def _default_model_id(self): @@ -95,33 +93,30 @@ class ChangesetFieldRule(models.Model): def get_rules(self, model_name, source_model_name): """ Return the rules for a model - If the key ``__changeset_rules_source_model`` is provided in the - context with the name of a model, rules for this specific model - will be searched for, if no rule is found, a generic rule - (without source_model_id) will be searched. + When a model is specified, it will return the rules for this + model. Fields that have no rule for this model will use the + global rules (those without source). The source model is the model which ask for a change, it will be - for instance ``res.users``, ``lefac.backend`` or ``magellan.backend``. + for instance ``res.users``, ``lefac.backend`` or + ``magellan.backend``. The second argument (``source_model_name``) is optional but cannot be an optional keyword argument otherwise it would not be in the key for the cache. The callers have to pass ``None`` if they want only global rules. """ - if source_model_name: - model_rules = self.search( - [('model_id', '=', model_name), - ('source_model_id.model', '=', source_model_name)], - ) - else: - model_rules = self.browse() - - rules = self.search([('model_id', '=', model_name), - ('source_model_id', '=', False)]) - # model's rules have precedence over global ones so we take the - # global rules first, then we update them with the source - # model's rules - return {rule.field_id.name: rule for rule in chain(rules, model_rules)} + model_rules = self.search( + [('model_id', '=', model_name), + '|', ('source_model_id.model', '=', source_model_name), + ('source_model_id', '=', False)], + # using 'DESC' means that 'NULLS FIRST' is the default + order='source_model_id DESC', + ) + # model's rules have precedence over global ones so we iterate + # over global rules first, then we update them with the rules + # which have a source model + return {rule.field_id.name: rule for rule in model_rules} @api.model def create(self, vals): diff --git a/partner_changeset/models/res_partner_changeset.py b/partner_changeset/models/res_partner_changeset.py index 7484788b9..d0d25f4d1 100644 --- a/partner_changeset/models/res_partner_changeset.py +++ b/partner_changeset/models/res_partner_changeset.py @@ -90,14 +90,11 @@ class ResPartnerChangeset(models.Model): """ Add a changeset on a partner By default, when a partner is modified by a user or by the - system, the changes are applied and a validated changeset is - created. Callers which want to delegate the write of some - fields to the changeset must explicitly ask for it by providing a - key ``__changeset_rules`` in the environment's context. + system, the the changeset will follow the rules configured for + the 'Users' / global rules. A caller should pass the following keys in the context: - * ``__changeset_rules``: activate the rules for the changesets * ``__changeset_rules_source_model``: name of the model which asks for the change * ``__changeset_rules_source_id``: id of the record which asks @@ -122,12 +119,16 @@ class ResPartnerChangeset(models.Model): source_model = self.env.context.get('__changeset_rules_source_model') source_id = self.env.context.get('__changeset_rules_source_id') - if not (source_model and source_id): + if not source_model: # if the changes source is not defined, log the user who # made the change source_model = 'res.users' + if not source_id: source_id = self.env.uid - source = '%s,%s' % (source_model, source_id) + if source_model and source_id: + source = '%s,%s' % (source_model, source_id) + else: + source = False change_model = self.env['res.partner.changeset.change'] write_values = values.copy() @@ -481,10 +482,9 @@ class ResPartnerChangesetChange(models.Model): new_field_name: new_value, 'field_id': rule.field_id.id, } - pop_value = False - if (not self.env.context.get('__changeset_rules') or - rule.action == 'auto'): + if rule.action == 'auto': change['state'] = 'done' + pop_value = False elif rule.action == 'validate': change['state'] = 'draft' pop_value = True # change to apply manually diff --git a/partner_changeset/tests/test_changeset_field_type.py b/partner_changeset/tests/test_changeset_field_type.py index aca94fbaf..a75adf190 100644 --- a/partner_changeset/tests/test_changeset_field_type.py +++ b/partner_changeset/tests/test_changeset_field_type.py @@ -67,7 +67,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_char(self): """ Add a new changeset on a Char field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_char.name: 'New value', }) self.assert_changeset( @@ -80,7 +80,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_text(self): """ Add a new changeset on a Text field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_text.name: 'New comment\non 2 lines', }) self.assert_changeset( @@ -98,7 +98,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): self.field_boolean.name: False, }) - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_boolean.name: True, }) self.assert_changeset( @@ -111,7 +111,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_date(self): """ Add a new changeset on a Date field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_date.name: '2015-09-15', }) self.assert_changeset( @@ -124,7 +124,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_integer(self): """ Add a new changeset on a Integer field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_integer.name: 42, }) self.assert_changeset( @@ -137,7 +137,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_float(self): """ Add a new changeset on a Float field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_float.name: 3.1415, }) self.assert_changeset( @@ -150,7 +150,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_selection(self): """ Add a new changeset on a Selection field """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_selection.name: 'delivery', }) self.assert_changeset( @@ -167,7 +167,7 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): self.field_many2one.name: self.env.ref('base.fr').id, }) - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_many2one.name: self.env.ref('base.ch').id, }) self.assert_changeset( @@ -181,21 +181,21 @@ class TestChangesetFieldType(ChangesetMixin, common.TransactionCase): def test_new_changeset_many2many(self): """ Add a new changeset on a Many2many field is not supported """ with self.assertRaises(NotImplementedError): - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_many2many.name: [self.env.ref('base.ch').id], }) def test_new_changeset_one2many(self): """ Add a new changeset on a One2many field is not supported """ with self.assertRaises(NotImplementedError): - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_one2many.name: [self.env.ref('base.user_root').id], }) def test_new_changeset_binary(self): """ Add a new changeset on a Binary field is not supported """ with self.assertRaises(NotImplementedError): - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ self.field_binary.name: '', }) diff --git a/partner_changeset/tests/test_changeset_flow.py b/partner_changeset/tests/test_changeset_flow.py index ddb88ce7c..950de183f 100644 --- a/partner_changeset/tests/test_changeset_flow.py +++ b/partner_changeset/tests/test_changeset_flow.py @@ -33,9 +33,9 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): suite, so we only use 'char' fields. We have to ensure that the general changeset flows work as expected, that is: - * create a 'done' changeset when a manual/system write is made on partner - * create a changeset according to the changeset rules when the key - '__changeset_rules' is passed in the context + * create a changeset when a manual/system write is made on partner + * create a changeset according to the changeset rules when a source model + is specified * apply a changeset change writes the value on the partner * apply a whole changeset writes all the changes' values on the partner * changes in state 'cancel' or 'done' do not write on the partner @@ -78,10 +78,9 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): def test_new_changeset(self): """ Add a new changeset on a partner - A new changeset is created when we write on a partner with - ``__changeset_rules`` in the context. + A new changeset is created when we write on a partner """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'name': 'Y', 'street': 'street Y', 'street2': 'street2 Y', @@ -100,7 +99,7 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): def test_new_changeset_empty_value(self): """ Create a changeset change that empty a value """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'street': False, }) self.assert_changeset( @@ -109,28 +108,6 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): [(self.field_street, 'street X', False, 'draft')] ) - def test_manual_edition(self): - """ A manual edition of a partner should always be applied - - But should create a 'done' changeset - """ - self.partner.write({ - 'name': 'Y', - 'street': 'street Y', - 'street2': 'street2 Y', - }) - self.assert_changeset( - self.partner, - self.env.user, - [(self.field_name, 'X', 'Y', 'done'), - (self.field_street, 'street X', 'street Y', 'done'), - (self.field_street2, 'street2 X', 'street2 Y', 'done'), - ], - ) - self.assertEqual(self.partner.name, 'Y') - self.assertEqual(self.partner.street, 'street Y') - self.assertEqual(self.partner.street2, 'street2 Y') - def test_apply_change(self): """ Apply a changeset change on a partner """ changes = [ @@ -258,7 +235,7 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): def test_new_changeset_source(self): """ Source is the user who made the change """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'street': False, }) changeset = self.partner.changeset_ids @@ -268,7 +245,6 @@ class TestChangesetFlow(ChangesetMixin, common.TransactionCase): """ Define source from another model """ company = self.env.ref('base.main_company') keys = { - '__changeset_rules': True, '__changeset_rules_source_model': 'res.company', '__changeset_rules_source_id': company.id, } diff --git a/partner_changeset/tests/test_changeset_origin.py b/partner_changeset/tests/test_changeset_origin.py index 1c23f65db..a29329b56 100644 --- a/partner_changeset/tests/test_changeset_origin.py +++ b/partner_changeset/tests/test_changeset_origin.py @@ -55,7 +55,7 @@ class TestChangesetOrigin(ChangesetMixin, common.TransactionCase): According to the state of the change. """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'name': 'Y', }) changeset = self.partner.changeset_ids @@ -63,13 +63,13 @@ class TestChangesetOrigin(ChangesetMixin, common.TransactionCase): self.assertEqual(self.partner.name, 'X') self.assertEqual(change.origin_value_char, 'X') self.assertEqual(change.origin_value_display, 'X') - self.partner.write({'name': 'A'}) + self.partner.with_context(__no_changeset=True).write({'name': 'A'}) self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') change.apply() self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') - self.partner.write({'name': 'B'}) + self.partner.with_context(__no_changeset=True).write({'name': 'B'}) self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') @@ -78,7 +78,7 @@ class TestChangesetOrigin(ChangesetMixin, common.TransactionCase): According to the state of the change. """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'name': 'Y', }) changeset = self.partner.changeset_ids @@ -86,44 +86,44 @@ class TestChangesetOrigin(ChangesetMixin, common.TransactionCase): self.assertEqual(self.partner.name, 'X') self.assertEqual(change.origin_value_char, 'X') self.assertEqual(change.origin_value_display, 'X') - self.partner.write({'name': 'A'}) + self.partner.with_context(__no_changeset=True).write({'name': 'A'}) self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') change.cancel() self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') - self.partner.write({'name': 'B'}) + self.partner.with_context(__no_changeset=True).write({'name': 'B'}) self.assertEqual(change.origin_value_char, 'A') self.assertEqual(change.origin_value_display, 'A') def test_old_field_of_change_with_apply(self): """ Old field is stored when the change is applied """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'name': 'Y', }) changeset = self.partner.changeset_ids change = changeset.change_ids self.assertEqual(self.partner.name, 'X') self.assertFalse(change.old_value_char) - self.partner.write({'name': 'A'}) + self.partner.with_context(__no_changeset=True).write({'name': 'A'}) self.assertFalse(change.old_value_char) change.apply() self.assertEqual(change.old_value_char, 'A') - self.partner.write({'name': 'B'}) + self.partner.with_context(__no_changeset=True).write({'name': 'B'}) self.assertEqual(change.old_value_char, 'A') def test_old_field_of_change_with_cancel(self): """ Old field is stored when the change is canceled """ - self.partner.with_context(__changeset_rules=True).write({ + self.partner.write({ 'name': 'Y', }) changeset = self.partner.changeset_ids change = changeset.change_ids self.assertEqual(self.partner.name, 'X') self.assertFalse(change.old_value_char) - self.partner.write({'name': 'A'}) + self.partner.with_context(__no_changeset=True).write({'name': 'A'}) self.assertFalse(change.old_value_char) change.cancel() self.assertEqual(change.old_value_char, 'A') - self.partner.write({'name': 'B'}) + self.partner.with_context(__no_changeset=True).write({'name': 'B'}) self.assertEqual(change.old_value_char, 'A')