From 39b216bd1b1dacc1ead36ade2ed1b4e00544c4ed Mon Sep 17 00:00:00 2001
From: Ronald Portier <ronald@therp.nl>
Date: Mon, 16 Oct 2017 12:39:37 +0200
Subject: [PATCH] [IMP] Make partner_multi_relation more extendable.

---
 partner_multi_relation/__init__.py            |   2 +-
 partner_multi_relation/__manifest__.py        |   2 +-
 partner_multi_relation/data/demo.xml          |   2 -
 partner_multi_relation/models/__init__.py     |   8 +-
 partner_multi_relation/models/res_partner.py  |   2 +-
 .../models/res_partner_relation.py            |   3 +-
 .../models/res_partner_relation_all.py        | 472 ++++++++++++------
 .../models/res_partner_relation_type.py       |  60 ++-
 .../res_partner_relation_type_selection.py    | 119 +++--
 .../tests/test_partner_relation.py            | 201 ++++----
 .../tests/test_partner_relation_all.py        | 149 +++---
 .../tests/test_partner_relation_common.py     |  52 +-
 .../views/res_partner_relation_all.xml        |   4 +-
 .../views/res_partner_relation_type.xml       |   6 +-
 14 files changed, 659 insertions(+), 423 deletions(-)

diff --git a/partner_multi_relation/__init__.py b/partner_multi_relation/__init__.py
index 91d6405f3..c3e6b003f 100644
--- a/partner_multi_relation/__init__.py
+++ b/partner_multi_relation/__init__.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from . import models
 from . import tests
diff --git a/partner_multi_relation/__manifest__.py b/partner_multi_relation/__manifest__.py
index a8f4c1739..1ed334d82 100644
--- a/partner_multi_relation/__manifest__.py
+++ b/partner_multi_relation/__manifest__.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 {
     "name": "Partner relations",
diff --git a/partner_multi_relation/data/demo.xml b/partner_multi_relation/data/demo.xml
index 5289a211d..47318e281 100644
--- a/partner_multi_relation/data/demo.xml
+++ b/partner_multi_relation/data/demo.xml
@@ -1,6 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <odoo>
-    <data>
         <!-- Added partner categories and partners to this file, because it
              turned out to be a bad idea to rely on demo data in base module,
              that can change from release to release. Only dependency on
@@ -114,5 +113,4 @@
             <field name="right_partner_id" ref="res_partner_pmr_best" />
             <field name="type_id" ref="rel_type_competitor" />
         </record>
-    </data>
 </odoo>
diff --git a/partner_multi_relation/models/__init__.py b/partner_multi_relation/models/__init__.py
index 8aef17344..5890ea7e0 100644
--- a/partner_multi_relation/models/__init__.py
+++ b/partner_multi_relation/models/__init__.py
@@ -1,8 +1,8 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
-from . import res_partner
-from . import res_partner_relation
 from . import res_partner_relation_type
-from . import res_partner_relation_all
 from . import res_partner_relation_type_selection
+from . import res_partner_relation
+from . import res_partner_relation_all
+from . import res_partner
diff --git a/partner_multi_relation/models/res_partner.py b/partner_multi_relation/models/res_partner.py
index f62d872e4..bfaabdea9 100644
--- a/partner_multi_relation/models/res_partner.py
+++ b/partner_multi_relation/models/res_partner.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 """Support connections between partners."""
 import numbers
diff --git a/partner_multi_relation/models/res_partner_relation.py b/partner_multi_relation/models/res_partner_relation.py
index 0176fe9b2..e19b9aaf6 100644
--- a/partner_multi_relation/models/res_partner_relation.py
+++ b/partner_multi_relation/models/res_partner_relation.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+# pylint: disable=api-one-deprecated
 """Store relations (connections) between partners."""
 from openerp import _, api, fields, models
 from openerp.exceptions import ValidationError
diff --git a/partner_multi_relation/models/res_partner_relation_all.py b/partner_multi_relation/models/res_partner_relation_all.py
index 1b462a8e8..725dbdee5 100644
--- a/partner_multi_relation/models/res_partner_relation_all.py
+++ b/partner_multi_relation/models/res_partner_relation_all.py
@@ -1,18 +1,100 @@
 # -*- coding: utf-8 -*-
-# © 2014-2016 Therp BV <http://therp.nl>
+# Copyright 2014-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+# pylint: disable=method-required-super
 """Abstract model to show each relation from two sides."""
+import collections
+import logging
+
 from psycopg2.extensions import AsIs
 
 from openerp import _, api, fields, models
+from openerp.exceptions import ValidationError
 from openerp.tools import drop_view_if_exists
 
 
-PADDING = 10
-_RECORD_TYPES = [
-    ('a', 'Left partner to right partner'),
-    ('b', 'Right partner to left partner'),
-]
+_logger = logging.getLogger(__name__)
+
+
+_last_key_offset = -1
+_specification_register = collections.OrderedDict()
+
+
+def register_select_specification(base_name, is_inverse, select_sql):
+    """Register SELECT clause for rows to be included in view.
+
+    Each SELECT clause must contain a first column in the form:
+        '<base_keyfield> * %%(padding)s + %(key_offset),'
+    The %%(padding)s will be used as a parameter for the
+    cursor.execute function, the %(key_offset) will be replaced
+    by dictionary replacement.
+    The columns for each SELECT clause must be ordered as follows:
+    id - specified as defined above:
+    res_model: model._name of the underlying table,
+    res_id: base key in the underlying table,
+    this_partner_id: must refer to a partner
+    other_partner_id: must refer to a related partner
+    type_id: refers to res.partner.relation.type,
+    date_start: start date of relation if relevant or NULL,
+    date_end: end date of relation if relevant or NULL,
+    %(is_inverse)s as is_inverse: used to determine type_selection_id,
+    """
+    global _last_key_offset
+    key_name = base_name + (is_inverse and '_inverse' or '')
+    assert key_name not in _specification_register
+    assert '%%(padding)s' in select_sql
+    assert '%(key_offset)s' in select_sql
+    assert '%(is_inverse)s' in select_sql
+    _last_key_offset += 1
+    _specification_register[key_name] = dict(
+        base_name=base_name,
+        is_inverse=is_inverse,
+        key_offset=_last_key_offset,
+        select_sql=select_sql % {
+            'key_offset': _last_key_offset,
+            'is_inverse': is_inverse})
+
+
+def get_select_specification(base_name, is_inverse):
+    key_name = base_name + (is_inverse and '_inverse' or '')
+    return _specification_register[key_name]
+
+
+# Register relations
+register_select_specification(
+    base_name='relation',
+    is_inverse=False,
+    select_sql="""\
+SELECT
+    (rel.id * %%(padding)s) + %(key_offset)s AS id,
+    'res.partner.relation' AS res_model,
+    rel.id AS res_id,
+    rel.left_partner_id AS this_partner_id,
+    rel.right_partner_id AS other_partner_id,
+    rel.type_id,
+    rel.date_start,
+    rel.date_end,
+    %(is_inverse)s as is_inverse
+FROM res_partner_relation rel
+    """)
+
+# Register inverse relations
+register_select_specification(
+    base_name='relation',
+    is_inverse=True,
+    select_sql="""\
+SELECT
+    (rel.id * %%(padding)s) + %(key_offset)s AS id,
+    'res.partner.relation',
+    rel.id,
+    rel.right_partner_id,
+    rel.left_partner_id,
+    rel.type_id,
+    rel.date_start,
+    rel.date_end,
+    %(is_inverse)s as is_inverse
+FROM res_partner_relation rel
+    """)
 
 
 class ResPartnerRelationAll(models.AbstractModel):
@@ -21,108 +103,116 @@ class ResPartnerRelationAll(models.AbstractModel):
     _log_access = False
     _name = 'res.partner.relation.all'
     _description = 'All (non-inverse + inverse) relations between partners'
-    _order = (
-        'this_partner_id, type_selection_id,'
-        'date_end desc, date_start desc'
-    )
-
-    _additional_view_fields = []
-    """append to this list if you added fields to res_partner_relation that
-    you need in this model and related fields are not adequate (ie for sorting)
-    You must use the same name as in res_partner_relation.
-    Don't overwrite this list in your declaration but append in _auto_init:
-
-    @api.model_cr_context
-    def _auto_init(self):
-        self._additional_view_fields.append('my_field')
-        return super(ResPartnerRelationAll, self)._auto_init()
-
-    my_field = fields...
-    """
+    _order = \
+        'this_partner_id, type_selection_id, date_end desc, date_start desc'
 
+    res_model = fields.Char(
+        string='Resource Model',
+        readonly=True,
+        required=True,
+        help="The database object this relation is based on.")
+    res_id = fields.Integer(
+        string='Resource ID',
+        readonly=True,
+        required=True,
+        help="The id of the object in the model this relation is based on.")
     this_partner_id = fields.Many2one(
         comodel_name='res.partner',
         string='One Partner',
-        required=True,
-    )
+        required=True)
     other_partner_id = fields.Many2one(
         comodel_name='res.partner',
         string='Other Partner',
-        required=True,
-    )
-    type_selection_id = fields.Many2one(
-        comodel_name='res.partner.relation.type.selection',
-        string='Relation Type',
-        required=True,
-    )
-    relation_id = fields.Many2one(
-        comodel_name='res.partner.relation',
-        string='Relation',
+        required=True)
+    type_id = fields.Many2one(
+        comodel_name='res.partner.relation.type',
+        string='Underlying Relation Type',
         readonly=True,
-    )
-    record_type = fields.Selection(
-        selection=_RECORD_TYPES,
-        string='Record Type',
-        readonly=True,
-    )
+        required=True)
     date_start = fields.Date('Starting date')
     date_end = fields.Date('Ending date')
+    is_inverse = fields.Boolean(
+        string="Is reverse type?",
+        readonly=True,
+        help="Inverse relations are from right to left partner.")
+    type_selection_id = fields.Many2one(
+        comodel_name='res.partner.relation.type.selection',
+        string='Relation Type',
+        required=True)
     active = fields.Boolean(
         string='Active',
-        help="Records with date_end in the past are inactive",
-    )
+        readonly=True,
+        help="Records with date_end in the past are inactive")
     any_partner_id = fields.Many2many(
         comodel_name='res.partner',
         string='Partner',
         compute=lambda self: None,
-        search='_search_any_partner_id'
-    )
+        search='_search_any_partner_id')
+
+    def _get_active_selects(self):
+        """Return selects actually to be used.
+
+        Selects are registered from all modules PRESENT. But should only be
+        used to build view if module actually INSTALLED.
+        """
+        return ['relation', 'relation_inverse']
+
+    def _get_statement(self):
+        """Allow other modules to add to statement."""
+        active_selects = self._get_active_selects()
+        union_select = ' UNION '.join(
+            [_specification_register[key]['select_sql']
+             for key in active_selects])
+        return """\
+CREATE OR REPLACE VIEW %%(table)s AS
+     WITH base_selection AS (%(union_select)s)
+ SELECT
+     bas.*,
+     CASE
+         WHEN NOT bas.is_inverse OR typ.is_symmetric
+         THEN bas.type_id * 2
+         ELSE (bas.type_id * 2) + 1
+     END as type_selection_id,
+     (bas.date_end IS NULL OR bas.date_end >= current_date) AS active
+     %%(additional_view_fields)s
+ FROM base_selection bas
+ JOIN res_partner_relation_type typ ON (bas.type_id = typ.id)
+ %%(additional_tables)s
+        """ % {'union_select': union_select}
+
+    def _get_padding(self):
+        """Utility function to define padding in one place."""
+        return 100
+
+    def _get_additional_view_fields(self):
+        """Allow inherit models to add fields to view.
+
+        If fields are added, the resulting string must have each field
+        prepended by a comma, like so:
+            return ', typ.allow_self, typ.left_partner_category'
+        """
+        return ''
+
+    def _get_additional_tables(self):
+        """Allow inherit models to add tables (JOIN's) to view.
+
+        Example:
+            return 'JOIN type_extention ext ON (bas.type_id = ext.id)'
+        """
+        return ''
 
     @api.model_cr_context
     def _auto_init(self):
         cr = self._cr
         drop_view_if_exists(cr, self._table)
-        additional_view_fields = ','.join(self._additional_view_fields)
-        additional_view_fields = (',' + additional_view_fields)\
-            if additional_view_fields else ''
         cr.execute(
-            """\
-CREATE OR REPLACE VIEW %(table)s AS
-    SELECT
-        rel.id * %(padding)s AS id,
-        rel.id AS relation_id,
-        cast('a' AS CHAR(1)) AS record_type,
-        rel.left_partner_id AS this_partner_id,
-        rel.right_partner_id AS other_partner_id,
-        rel.date_start,
-        rel.date_end,
-        (rel.date_end IS NULL OR rel.date_end >= current_date) AS active,
-        rel.type_id * %(padding)s AS type_selection_id
-        %(additional_view_fields)s
-    FROM res_partner_relation rel
-    UNION SELECT
-        rel.id * %(padding)s + 1,
-        rel.id,
-        CAST('b' AS CHAR(1)),
-        rel.right_partner_id,
-        rel.left_partner_id,
-        rel.date_start,
-        rel.date_end,
-        rel.date_end IS NULL OR rel.date_end >= current_date,
-        CASE
-            WHEN typ.is_symmetric THEN rel.type_id * %(padding)s
-            ELSE rel.type_id * %(padding)s + 1
-        END
-        %(additional_view_fields)s
-    FROM res_partner_relation rel
-    JOIN res_partner_relation_type typ ON (rel.type_id = typ.id)
-            """,
-            {
-                'table': AsIs(self._table),
-                'padding': PADDING,
-                'additional_view_fields': AsIs(additional_view_fields),
-            }
-        )
+            self._get_statement(),
+            {'table': AsIs(self._table),
+             'padding': self._get_padding(),
+             'additional_view_fields':
+                AsIs(self._get_additional_view_fields()),
+             'additional_tables':
+                AsIs(self._get_additional_tables())})
         return super(ResPartnerRelationAll, self)._auto_init()
 
     @api.model
@@ -132,8 +222,7 @@ CREATE OR REPLACE VIEW %(table)s AS
         return [
             '|',
             ('this_partner_id', operator, value),
-            ('other_partner_id', operator, value),
-        ]
+            ('other_partner_id', operator, value)]
 
     @api.multi
     def name_get(self):
@@ -142,9 +231,7 @@ CREATE OR REPLACE VIEW %(table)s AS
                 this.this_partner_id.name,
                 this.type_selection_id.display_name,
                 this.other_partner_id.name,
-            )
-            for this in self
-        }
+            ) for this in self}
 
     @api.onchange('type_selection_id')
     def onchange_type_selection_id(self):
@@ -166,13 +253,11 @@ CREATE OR REPLACE VIEW %(table)s AS
                 if partner:
                     warning['message'] = (
                         _('%s partner incompatible with relation type.') %
-                        side.title()
-                    )
+                        side.title())
                 else:
                     warning['message'] = (
                         _('No %s partner available for relation type.') %
-                        side
-                    )
+                        side)
             return warning
 
         this_partner_domain = []
@@ -180,45 +265,50 @@ CREATE OR REPLACE VIEW %(table)s AS
         if self.type_selection_id.contact_type_this:
             this_partner_domain.append((
                 'is_company', '=',
-                self.type_selection_id.contact_type_this == 'c'
-            ))
+                self.type_selection_id.contact_type_this == 'c'))
         if self.type_selection_id.partner_category_this:
             this_partner_domain.append((
                 'category_id', 'in',
-                self.type_selection_id.partner_category_this.ids
-            ))
+                self.type_selection_id.partner_category_this.ids))
         if self.type_selection_id.contact_type_other:
             other_partner_domain.append((
                 'is_company', '=',
-                self.type_selection_id.contact_type_other == 'c'
-            ))
+                self.type_selection_id.contact_type_other == 'c'))
         if self.type_selection_id.partner_category_other:
             other_partner_domain.append((
                 'category_id', 'in',
-                self.type_selection_id.partner_category_other.ids
-            ))
+                self.type_selection_id.partner_category_other.ids))
         result = {'domain': {
             'this_partner_id': this_partner_domain,
-            'other_partner_id': other_partner_domain,
-        }}
+            'other_partner_id': other_partner_domain}}
         # Check wether domain results in no choice or wrong choice of partners:
         warning = {}
+        partner_model = self.env['res.partner']
         if this_partner_domain:
+            this_partner = False
+            if bool(self.this_partner_id.id):
+                this_partner = self.this_partner_id
+            else:
+                this_partner_id = \
+                    'default_this_partner_id' in self.env.context and \
+                    self.env.context['default_this_partner_id'] or \
+                    'active_id' in self.env.context and \
+                    self.env.context['active_id'] or \
+                    False
+                if this_partner_id:
+                    this_partner = partner_model.browse(this_partner_id)
             warning = check_partner_domain(
-                self.this_partner_id, this_partner_domain, _('this')
-            )
+                this_partner, this_partner_domain, _('this'))
         if not warning and other_partner_domain:
             warning = check_partner_domain(
-                self.other_partner_id, other_partner_domain, _('other')
-            )
+                self.other_partner_id, other_partner_domain, _('other'))
         if warning:
             result['warning'] = warning
         return result
 
     @api.onchange(
         'this_partner_id',
-        'other_partner_id',
-    )
+        'other_partner_id')
     def onchange_partner_id(self):
         """Set domain on type_selection_id based on partner(s) selected."""
 
@@ -233,15 +323,13 @@ CREATE OR REPLACE VIEW %(table)s AS
                 return warning
             test_domain = (
                 [('id', '=', self.type_selection_id.id)] +
-                type_selection_domain
-            )
+                type_selection_domain)
             type_model = self.env['res.partner.relation.type.selection']
             types_found = type_model.search(test_domain, limit=1)
             if not types_found:
                 warning['title'] = _('Error!')
                 warning['message'] = _(
-                    'Relation type incompatible with selected partner(s).'
-                )
+                    'Relation type incompatible with selected partner(s).')
             return warning
 
         type_selection_domain = []
@@ -254,8 +342,7 @@ CREATE OR REPLACE VIEW %(table)s AS
                 '|',
                 ('partner_category_this', '=', False),
                 ('partner_category_this', 'in',
-                 self.this_partner_id.category_id.ids),
-            ]
+                 self.this_partner_id.category_id.ids)]
         if self.other_partner_id:
             type_selection_domain += [
                 '|',
@@ -265,11 +352,9 @@ CREATE OR REPLACE VIEW %(table)s AS
                 '|',
                 ('partner_category_other', '=', False),
                 ('partner_category_other', 'in',
-                 self.other_partner_id.category_id.ids),
-            ]
+                 self.other_partner_id.category_id.ids)]
         result = {'domain': {
-            'type_selection_id': type_selection_domain,
-        }}
+            'type_selection_id': type_selection_domain}}
         # Check wether domain results in no choice or wrong choice for
         # type_selection_id:
         warning = check_type_selection_domain(type_selection_domain)
@@ -278,68 +363,123 @@ CREATE OR REPLACE VIEW %(table)s AS
         return result
 
     @api.model
-    def _correct_vals(self, vals):
+    def _correct_vals(self, vals, type_selection):
         """Fill left and right partner from this and other partner."""
         vals = vals.copy()
-        if 'this_partner_id' in vals:
-            vals['left_partner_id'] = vals['this_partner_id']
-            del vals['this_partner_id']
-        if 'other_partner_id' in vals:
-            vals['right_partner_id'] = vals['other_partner_id']
-            del vals['other_partner_id']
-        if 'type_selection_id' not in vals:
-            return vals
-        selection = self.type_selection_id.browse(vals['type_selection_id'])
-        type_id = selection.type_id.id
-        is_inverse = selection.is_inverse
-        vals['type_id'] = type_id
-        del vals['type_selection_id']
-        # Need to switch right and left partner if we are in reverse id:
-        if 'left_partner_id' in vals or 'right_partner_id' in vals:
-            if is_inverse:
-                left_partner_id = False
-                right_partner_id = False
-                if 'left_partner_id' in vals:
-                    right_partner_id = vals['left_partner_id']
-                    del vals['left_partner_id']
-                if 'right_partner_id' in vals:
-                    left_partner_id = vals['right_partner_id']
-                    del vals['right_partner_id']
-                if left_partner_id:
-                    vals['left_partner_id'] = left_partner_id
-                if right_partner_id:
-                    vals['right_partner_id'] = right_partner_id
+        if 'type_selection_id' in vals:
+            vals['type_id'] = type_selection.type_id.id
+        if type_selection.is_inverse:
+            if 'this_partner_id' in vals:
+                vals['right_partner_id'] = vals['this_partner_id']
+            if 'other_partner_id' in vals:
+                vals['left_partner_id'] = vals['other_partner_id']
+        else:
+            if 'this_partner_id' in vals:
+                vals['left_partner_id'] = vals['this_partner_id']
+            if 'other_partner_id' in vals:
+                vals['right_partner_id'] = vals['other_partner_id']
+        # Delete values not in underlying table:
+        for key in (
+                'this_partner_id',
+                'type_selection_id',
+                'other_partner_id'):
+            if key in vals:
+                del vals[key]
         return vals
 
+    @api.multi
+    def get_base_resource(self):
+        """Get base resource from res_model and res_id."""
+        self.ensure_one()
+        base_model = self.env[self.res_model]
+        return base_model.browse([self.res_id])
+
+    @api.multi
+    def write_resource(self, base_resource, vals):
+        """write handled by base resource."""
+        self.ensure_one()
+        # write for models other then res.partner.relation SHOULD
+        # be handled in inherited models:
+        relation_model = self.env['res.partner.relation']
+        assert self.res_model == relation_model._name
+        base_resource.write(vals)
+
+    @api.model
+    def _get_type_selection_from_vals(self, vals):
+        """Get type_selection_id straight from vals or compute from type_id.
+        """
+        type_selection_id = vals.get('type_selection_id', False)
+        if not type_selection_id:
+            type_id = vals.get('type_id', False)
+            if type_id:
+                is_inverse = vals.get('is_inverse')
+                type_selection_id = type_id * 2 + (is_inverse and 1 or 0)
+        return type_selection_id and self.type_selection_id.browse(
+            type_selection_id) or False
+
     @api.multi
     def write(self, vals):
-        """divert non-problematic writes to underlying table"""
-        vals = self._correct_vals(vals)
+        """For model 'res.partner.relation' call write on underlying model.
+        """
+        new_type_selection = self._get_type_selection_from_vals(vals)
         for rec in self:
-            rec.relation_id.write(vals)
+            type_selection = new_type_selection or rec.type_selection_id
+            vals = rec._correct_vals(vals, type_selection)
+            base_resource = rec.get_base_resource()
+            rec.write_resource(base_resource, vals)
+        # Invalidate cache to make res.partner.relation.all reflect changes
+        # in underlying res.partner.relation:
+        self.env.invalidate_all()
         return True
 
+    @api.model
+    def _compute_base_name(self, type_selection):
+        """This will be overridden for each inherit model."""
+        return 'relation'
+
+    @api.model
+    def _compute_id(self, base_resource, type_selection):
+        """Compute id. Allow for enhancements in inherit model."""
+        base_name = self._compute_base_name(type_selection)
+        key_offset = get_select_specification(
+            base_name, type_selection.is_inverse)['key_offset']
+        return base_resource.id * self._get_padding() + key_offset
+
+    @api.model
+    def create_resource(self, vals, type_selection):
+        relation_model = self.env['res.partner.relation']
+        return relation_model.create(vals)
+
     @api.model
     def create(self, vals):
         """Divert non-problematic creates to underlying table.
 
         Create a res.partner.relation but return the converted id.
         """
-        is_inverse = False
-        if 'type_selection_id' in vals:
-            selection = self.type_selection_id.browse(
-                vals['type_selection_id']
-            )
-            is_inverse = selection.is_inverse
-        vals = self._correct_vals(vals)
-        res = self.relation_id.create(vals)
-        return_id = res.id * PADDING + (is_inverse and 1 or 0)
-        return self.browse(return_id)
+        type_selection = self._get_type_selection_from_vals(vals)
+        if not type_selection:  # Should not happen
+            raise ValidationError(
+                _('No relation type specified in vals: %s.') % vals)
+        vals = self._correct_vals(vals, type_selection)
+        base_resource = self.create_resource(vals, type_selection)
+        res_id = self._compute_id(base_resource, type_selection)
+        return self.browse(res_id)
+
+    @api.multi
+    def unlink_resource(self, base_resource):
+        """Delegate unlink to underlying model."""
+        self.ensure_one()
+        # unlink for models other then res.partner.relation SHOULD
+        # be handled in inherited models:
+        relation_model = self.env['res.partner.relation']
+        assert self.res_model == relation_model._name
+        base_resource.unlink()
 
     @api.multi
     def unlink(self):
-        """divert non-problematic creates to underlying table"""
-        # pylint: disable=arguments-differ
+        """For model 'res.partner.relation' call unlink on underlying model.
+        """
         for rec in self:
-            rec.relation_id.unlink()
+            base_resource = rec.get_base_resource()
+            rec.unlink_resource(base_resource)
         return True
diff --git a/partner_multi_relation/models/res_partner_relation_type.py b/partner_multi_relation/models/res_partner_relation_type.py
index 5cfee5c6c..f10968ceb 100644
--- a/partner_multi_relation/models/res_partner_relation_type.py
+++ b/partner_multi_relation/models/res_partner_relation_type.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# © 2013-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 """Define the type of relations that can exist between partners."""
 from openerp import _, api, fields, models
@@ -84,16 +84,6 @@ class ResPartnerRelationType(models.Model):
             ('p', _('Person')),
         ]
 
-    @api.onchange('is_symmetric')
-    def onchange_is_symmetric(self):
-        """Set right side to left side if symmetric."""
-        if self.is_symmetric:
-            self.update({
-                'name_inverse': self.name,
-                'contact_type_right': self.contact_type_left,
-                'partner_category_right': self.partner_category_left,
-            })
-
     @api.multi
     def check_existing(self, vals):
         """Check wether records exist that do not fit new criteria."""
@@ -168,8 +158,54 @@ class ResPartnerRelationType(models.Model):
                                 relation.date_end > cutoff_date):
                             relation.write({'date_end': cutoff_date})
 
+    @api.multi
+    def _update_right_vals(self, vals):
+        """Make sure that on symmetric relations, right vals follow left vals.
+
+        @attention: All fields ending in `_right` will have their values
+                    replaced by the values of the fields whose names end
+                    in `_left`.
+        """
+        vals['name_inverse'] = vals.get('name', self.name)
+        # For all left keys in model, take value for right either from
+        # left key in vals, or if not present, from right key in self:
+        left_keys = [key for key in self._fields if key.endswith('_left')]
+        for left_key in left_keys:
+            right_key = left_key.replace('_left', '_right')
+            vals[right_key] = vals.get(left_key, self[left_key])
+            if hasattr(vals[right_key], 'id'):
+                vals[right_key] = vals[right_key].id
+
+    @api.model
+    def create(self, vals):
+        if vals.get('is_symmetric'):
+            self._update_right_vals(vals)
+        return super(ResPartnerRelationType, self).create(vals)
+
     @api.multi
     def write(self, vals):
         """Handle existing relations if conditions change."""
         self.check_existing(vals)
-        return super(ResPartnerRelationType, self).write(vals)
+        for rec in self:
+            rec_vals = vals.copy()
+            if rec_vals.get('is_symmetric', rec.is_symmetric):
+                self._update_right_vals(rec_vals)
+            super(ResPartnerRelationType, rec).write(rec_vals)
+        return True
+
+    @api.multi
+    def unlink(self):
+        """Allow delete of relation type, even when connections exist.
+
+        Relations can be deleted if relation type allows it.
+        """
+        relation_model = self.env['res.partner.relation']
+        for rec in self:
+            if rec.handle_invalid_onchange == 'delete':
+                # Automatically delete relations, so existing relations
+                # do not prevent unlink of relation type:
+                relations = relation_model.search([
+                    ('type_id', '=', rec.id),
+                ])
+                relations.unlink()
+        return super(ResPartnerRelationType, self).unlink()
diff --git a/partner_multi_relation/models/res_partner_relation_type_selection.py b/partner_multi_relation/models/res_partner_relation_type_selection.py
index 794d7d854..bb01c38d3 100644
--- a/partner_multi_relation/models/res_partner_relation_type_selection.py
+++ b/partner_multi_relation/models/res_partner_relation_type_selection.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# © 2014-2016 Therp BV <http://therp.nl>
+# Copyright 2013-2017 Therp BV <http://therp.nl>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 """
 For the model defined here _auto is set to False to prevent creating a
@@ -18,11 +18,6 @@ from psycopg2.extensions import AsIs
 from openerp import api, fields, models
 from openerp.tools import drop_view_if_exists
 
-from .res_partner_relation_type import ResPartnerRelationType
-
-
-PADDING = 10
-
 
 class ResPartnerRelationTypeSelection(models.Model):
     """Virtual relation types"""
@@ -33,13 +28,20 @@ class ResPartnerRelationTypeSelection(models.Model):
     _log_access = False
     _order = 'name asc'
 
+    @api.model
+    def get_partner_types(self):
+        """Partner types are defined by model res.partner.relation.type."""
+        # pylint: disable=no-self-use
+        rprt_model = self.env['res.partner.relation.type']
+        return rprt_model.get_partner_types()
+
     type_id = fields.Many2one(
         comodel_name='res.partner.relation.type',
         string='Type',
     )
     name = fields.Char('Name')
     contact_type_this = fields.Selection(
-        selection=ResPartnerRelationType.get_partner_types.im_func,
+        selection='get_partner_types',
         string='Current record\'s partner type',
     )
     is_inverse = fields.Boolean(
@@ -47,7 +49,7 @@ class ResPartnerRelationTypeSelection(models.Model):
         help="Inverse relations are from right to left partner.",
     )
     contact_type_other = fields.Selection(
-        selection=ResPartnerRelationType.get_partner_types.im_func,
+        selection='get_partner_types',
         string='Other record\'s partner type',
     )
     partner_category_this = fields.Many2one(
@@ -65,43 +67,68 @@ class ResPartnerRelationTypeSelection(models.Model):
         string='Symmetric',
     )
 
+    def _get_additional_view_fields(self):
+        """Allow inherit models to add fields to view.
+
+        If fields are added, the resulting string must have each field
+        prepended by a comma, like so:
+            return ', typ.allow_self, typ.left_partner_category'
+        """
+        return ''
+
+    def _get_additional_tables(self):
+        """Allow inherit models to add tables (JOIN's) to view.
+
+        Example:
+            return 'JOIN type_extention ext ON (bas.type_id = ext.id)'
+        """
+        return ''
+
     @api.model_cr_context
     def _auto_init(self):
         cr = self._cr
         drop_view_if_exists(cr, self._table)
         cr.execute(
-            """CREATE OR REPLACE VIEW %(table)s AS
-            SELECT
-                id * %(padding)s AS id,
-                id AS type_id,
-                name AS name,
-                False AS is_inverse,
-                contact_type_left AS contact_type_this,
-                contact_type_right AS contact_type_other,
-                partner_category_left AS partner_category_this,
-                partner_category_right AS partner_category_other,
-                allow_self,
-                is_symmetric
-            FROM %(underlying_table)s
-            UNION SELECT
-                id * %(padding)s + 1,
-                id,
-                name_inverse,
-                True,
-                contact_type_right,
-                contact_type_left,
-                partner_category_right,
-                partner_category_left,
-                allow_self,
-                is_symmetric
-             FROM %(underlying_table)s
-             WHERE not is_symmetric
+            """\
+CREATE OR REPLACE VIEW %(table)s AS
+     WITH selection_type AS (
+ SELECT
+     id * 2 AS id,
+     id AS type_id,
+     name AS name,
+     False AS is_inverse,
+     contact_type_left AS contact_type_this,
+     contact_type_right AS contact_type_other,
+     partner_category_left AS partner_category_this,
+     partner_category_right AS partner_category_other
+ FROM %(underlying_table)s
+ UNION SELECT
+     (id * 2) + 1,
+     id,
+     name_inverse,
+     True,
+     contact_type_right,
+     contact_type_left,
+     partner_category_right,
+     partner_category_left
+     FROM %(underlying_table)s
+     WHERE not is_symmetric
+ )
+ SELECT
+     bas.*,
+     typ.allow_self,
+     typ.is_symmetric
+     %(additional_view_fields)s
+ FROM selection_type bas
+ JOIN res_partner_relation_type typ ON (bas.type_id = typ.id)
+     %(additional_tables)s
             """,
-            {
-                'table': AsIs(self._table),
-                'padding': PADDING,
-                'underlying_table': AsIs('res_partner_relation_type'),
-            })
+            {'table': AsIs(self._table),
+             'underlying_table': AsIs('res_partner_relation_type'),
+             'additional_view_fields':
+                AsIs(self._get_additional_view_fields()),
+             'additional_tables':
+                 AsIs(self._get_additional_tables())})
         return super(ResPartnerRelationTypeSelection, self)._auto_init()
 
     @api.multi
@@ -111,18 +138,14 @@ class ResPartnerRelationTypeSelection(models.Model):
             (this.id,
              this.is_inverse and this.type_id.name_inverse or
              this.type_id.display_name)
-            for this in self
-        ]
+            for this in self]
 
     @api.model
     def name_search(self, name='', args=None, operator='ilike', limit=100):
         """Search for name or inverse name in underlying model."""
         # pylint: disable=no-value-for-parameter
         return self.search(
-            [
-                '|',
-                ('type_id.name', operator, name),
-                ('type_id.name_inverse', operator, name),
-            ] + (args or []),
-            limit=limit
-        ).name_get()
+            ['|',
+             ('type_id.name', operator, name),
+             ('type_id.name_inverse', operator, name)] + (args or []),
+            limit=limit).name_get()
diff --git a/partner_multi_relation/tests/test_partner_relation.py b/partner_multi_relation/tests/test_partner_relation.py
index b3ba3bd7b..8768c95d5 100644
--- a/partner_multi_relation/tests/test_partner_relation.py
+++ b/partner_multi_relation/tests/test_partner_relation.py
@@ -1,8 +1,9 @@
 # -*- coding: utf-8 -*-
-# Copyright 2016 Therp BV
+# Copyright 2016-2017 Therp BV
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from datetime import date
 from dateutil.relativedelta import relativedelta
+from psycopg2 import IntegrityError
 
 from openerp import fields
 from openerp.exceptions import ValidationError
@@ -12,16 +13,16 @@ from .test_partner_relation_common import TestPartnerRelationCommon
 
 class TestPartnerRelation(TestPartnerRelationCommon):
 
+    post_install = True
+
     def test_selection_name_search(self):
         """Test wether we can find type selection on reverse name."""
         selection_types = self.selection_model.name_search(
-            name=self.selection_person2company.name
-        )
+            name=self.selection_person2company.name)
         self.assertTrue(selection_types)
         self.assertTrue(
             (self.selection_person2company.id,
-             self.selection_person2company.name) in selection_types
-        )
+             self.selection_person2company.name) in selection_types)
 
     def test_self_allowed(self):
         """Test creation of relation to same partner when type allows."""
@@ -30,14 +31,12 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'name_inverse': 'allow_inverse',
             'contact_type_left': 'p',
             'contact_type_right': 'p',
-            'allow_self': True
-        })
+            'allow_self': True})
         self.assertTrue(type_allow)
         reflexive_relation = self.relation_model.create({
             'type_id': type_allow.id,
             'left_partner_id': self.partner_01_person.id,
-            'right_partner_id': self.partner_01_person.id,
-        })
+            'right_partner_id': self.partner_01_person.id})
         self.assertTrue(reflexive_relation)
 
     def test_self_disallowed(self):
@@ -51,15 +50,13 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'name_inverse': 'disallow_inverse',
             'contact_type_left': 'p',
             'contact_type_right': 'p',
-            'allow_self': False
-        })
+            'allow_self': False})
         self.assertTrue(type_disallow)
         with self.assertRaises(ValidationError):
             self.relation_model.create({
                 'type_id': type_disallow.id,
                 'left_partner_id': self.partner_01_person.id,
-                'right_partner_id': self.partner_01_person.id,
-            })
+                'right_partner_id': self.partner_01_person.id})
 
     def test_self_default(self):
         """Test default not to allow relation with same partner.
@@ -72,15 +69,13 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'name': 'default',
             'name_inverse': 'default_inverse',
             'contact_type_left': 'p',
-            'contact_type_right': 'p',
-        })
+            'contact_type_right': 'p'})
         self.assertTrue(type_default)
         with self.assertRaises(ValidationError):
             self.relation_model.create({
                 'type_id': type_default.id,
                 'left_partner_id': self.partner_01_person.id,
-                'right_partner_id': self.partner_01_person.id,
-            })
+                'right_partner_id': self.partner_01_person.id})
 
     def test_self_mixed(self):
         """Test creation of relation with wrong types.
@@ -92,8 +87,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             self.relation_model.create({
                 'type_id': self.type_company2person.id,
                 'left_partner_id': self.partner_01_person.id,
-                'right_partner_id': self.partner_02_company.id,
-            })
+                'right_partner_id': self.partner_02_company.id})
 
     def test_symmetric(self):
         """Test creating symmetric relation."""
@@ -103,56 +97,41 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'name_inverse': 'the other side of not symmetric',
             'is_symmetric': False,
             'contact_type_left': False,
-            'contact_type_right': 'p',
-        })
+            'contact_type_right': 'p'})
         # not yet symmetric relation should result in two records in
         # selection:
         selection_symmetric = self.selection_model.search([
-            ('type_id', '=', type_symmetric.id),
-        ])
+            ('type_id', '=', type_symmetric.id)])
         self.assertEqual(len(selection_symmetric), 2)
         # Now change to symmetric and test name and inverse name:
         with self.env.do_in_draft():
-            type_symmetric.write(
-                vals={
-                    'name': 'sym',
-                    'is_symmetric': True,
-                }
-            )
-        with self.env.do_in_onchange():
-            type_symmetric.onchange_is_symmetric()
+            type_symmetric.write({
+                'name': 'sym',
+                'is_symmetric': True})
         self.assertEqual(type_symmetric.is_symmetric, True)
         self.assertEqual(
             type_symmetric.name_inverse,
-            type_symmetric.name
-        )
+            type_symmetric.name)
         self.assertEqual(
             type_symmetric.contact_type_right,
-            type_symmetric.contact_type_left
-        )
+            type_symmetric.contact_type_left)
         # now update the database:
-        type_symmetric.write(
-            vals={
-                'name': type_symmetric.name,
-                'is_symmetric': type_symmetric.is_symmetric,
-                'name_inverse': type_symmetric.name_inverse,
-                'contact_type_right': type_symmetric.contact_type_right,
-            }
-        )
+        type_symmetric.write({
+            'name': type_symmetric.name,
+            'is_symmetric': type_symmetric.is_symmetric,
+            'name_inverse': type_symmetric.name_inverse,
+            'contact_type_right': type_symmetric.contact_type_right})
         # symmetric relation should result in only one record in
         # selection:
         selection_symmetric = self.selection_model.search([
-            ('type_id', '=', type_symmetric.id),
-        ])
+            ('type_id', '=', type_symmetric.id)])
         self.assertEqual(len(selection_symmetric), 1)
         relation = self.relation_all_model.create({
             'type_selection_id': selection_symmetric.id,
             'this_partner_id': self.partner_02_company.id,
-            'other_partner_id': self.partner_01_person.id,
-        })
+            'other_partner_id': self.partner_01_person.id})
         partners = self.partner_model.search([
-            ('search_relation_type_id', '=', relation.type_selection_id.id)
-        ])
+            ('search_relation_type_id', '=', relation.type_selection_id.id)])
         self.assertTrue(self.partner_01_person in partners)
         self.assertTrue(self.partner_02_company in partners)
 
@@ -163,118 +142,154 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             self.relation_model.create({
                 'type_id': self.type_ngo2volunteer.id,
                 'left_partner_id': self.partner_02_company.id,
-                'right_partner_id': self.partner_04_volunteer.id,
-            })
+                'right_partner_id': self.partner_04_volunteer.id})
         # Check on right side:
         with self.assertRaises(ValidationError):
             self.relation_model.create({
                 'type_id': self.type_ngo2volunteer.id,
                 'left_partner_id': self.partner_03_ngo.id,
-                'right_partner_id': self.partner_01_person.id,
-            })
+                'right_partner_id': self.partner_01_person.id})
 
     def test_relation_type_change(self):
         """Test change in relation type conditions."""
         # First create a relation type having no particular conditions.
         (type_school2student,
          school2student,
-         school2student_inverse) = (
+         school2student_inverse) = \
             self._create_relation_type_selection({
                 'name': 'school has student',
-                'name_inverse': 'studies at school',
-            })
-        )
+                'name_inverse': 'studies at school'})
         # Second create relations based on those conditions.
         partner_school = self.partner_model.create({
             'name': 'Test School',
             'is_company': True,
-            'ref': 'TS',
-        })
+            'ref': 'TS'})
         partner_bart = self.partner_model.create({
             'name': 'Bart Simpson',
             'is_company': False,
-            'ref': 'BS',
-        })
+            'ref': 'BS'})
         partner_lisa = self.partner_model.create({
             'name': 'Lisa Simpson',
             'is_company': False,
-            'ref': 'LS',
-        })
+            'ref': 'LS'})
         relation_school2bart = self.relation_all_model.create({
             'this_partner_id': partner_school.id,
             'type_selection_id': school2student.id,
-            'other_partner_id': partner_bart.id,
-        })
+            'other_partner_id': partner_bart.id})
         self.assertTrue(relation_school2bart)
         relation_school2lisa = self.relation_all_model.create({
             'this_partner_id': partner_school.id,
             'type_selection_id': school2student.id,
-            'other_partner_id': partner_lisa.id,
-        })
+            'other_partner_id': partner_lisa.id})
         self.assertTrue(relation_school2lisa)
         relation_bart2lisa = self.relation_all_model.create({
             'this_partner_id': partner_bart.id,
             'type_selection_id': school2student.id,
-            'other_partner_id': partner_lisa.id,
-        })
+            'other_partner_id': partner_lisa.id})
         self.assertTrue(relation_bart2lisa)
         # Third creata a category and make it a condition for the
         #     relation type.
         # - Test restriction
         # - Test ignore
-        category_student = self.category_model.create({
-            'name': 'Student',
-        })
+        category_student = self.category_model.create({'name': 'Student'})
         with self.assertRaises(ValidationError):
             type_school2student.write({
-                'partner_category_right': category_student.id,
-            })
+                'partner_category_right': category_student.id})
         self.assertFalse(type_school2student.partner_category_right.id)
         type_school2student.write({
             'handle_invalid_onchange': 'ignore',
-            'partner_category_right': category_student.id,
-        })
+            'partner_category_right': category_student.id})
         self.assertEqual(
             type_school2student.partner_category_right.id,
-            category_student.id
-        )
+            category_student.id)
         # Fourth make company type a condition for left partner
         # - Test ending
         # - Test deletion
         partner_bart.write({
-            'category_id': [(4, category_student.id)],
-        })
+            'category_id': [(4, category_student.id)]})
         partner_lisa.write({
-            'category_id': [(4, category_student.id)],
-        })
+            'category_id': [(4, category_student.id)]})
         # Future student to be deleted by end action:
         partner_homer = self.partner_model.create({
             'name': 'Homer Simpson',
             'is_company': False,
             'ref': 'HS',
-            'category_id': [(4, category_student.id)],
-        })
+            'category_id': [(4, category_student.id)]})
         relation_lisa2homer = self.relation_all_model.create({
             'this_partner_id': partner_lisa.id,
             'type_selection_id': school2student.id,
             'other_partner_id': partner_homer.id,
             'date_start': fields.Date.to_string(
-                date.today() + relativedelta(months=+6)
-            ),
-        })
+                date.today() + relativedelta(months=+6))})
         self.assertTrue(relation_lisa2homer)
         type_school2student.write({
             'handle_invalid_onchange': 'end',
-            'contact_type_left': 'c',
-        })
+            'contact_type_left': 'c'})
         self.assertEqual(
             relation_bart2lisa.date_end,
-            fields.Date.today()
-        )
+            fields.Date.today())
         self.assertFalse(relation_lisa2homer.exists())
         type_school2student.write({
             'handle_invalid_onchange': 'delete',
             'contact_type_left': 'c',
-            'contact_type_right': 'p',
-        })
+            'contact_type_right': 'p'})
         self.assertFalse(relation_bart2lisa.exists())
+
+    def test_relation_type_unlink_dberror(self):
+        """Test deleting relation type when not possible.
+
+        This test will catch a DB Integrity error. Because of that the
+        cursor will be invalidated, and further tests using the objects
+        will not be possible.
+        """
+        # First create a relation type having restrict particular conditions.
+        type_model = self.env['res.partner.relation.type']
+        relation_model = self.env['res.partner.relation']
+        partner_model = self.env['res.partner']
+        type_school2student = type_model.create({
+            'name': 'school has student',
+            'name_inverse': 'studies at school',
+            'handle_invalid_onchange': 'restrict'})
+        # Second create relation based on those conditions.
+        partner_school = partner_model.create({
+            'name': 'Test School',
+            'is_company': True,
+            'ref': 'TS'})
+        partner_bart = partner_model.create({
+            'name': 'Bart Simpson',
+            'is_company': False,
+            'ref': 'BS'})
+        relation_model.create({
+            'left_partner_id': partner_school.id,
+            'type_id': type_school2student.id,
+            'right_partner_id': partner_bart.id})
+        # Unlink should for the moment lead to error because of restrict:
+        with self.assertRaises(IntegrityError):
+            type_school2student.unlink()
+
+    def test_relation_type_unlink(self):
+        """Test delete of relation type, including deleting relations."""
+        # First create a relation type having restrict particular conditions.
+        type_model = self.env['res.partner.relation.type']
+        relation_model = self.env['res.partner.relation']
+        partner_model = self.env['res.partner']
+        type_school2student = type_model.create({
+            'name': 'school has student',
+            'name_inverse': 'studies at school',
+            'handle_invalid_onchange': 'delete'})
+        # Second create relation based on those conditions.
+        partner_school = partner_model.create({
+            'name': 'Test School',
+            'is_company': True,
+            'ref': 'TS'})
+        partner_bart = partner_model.create({
+            'name': 'Bart Simpson',
+            'is_company': False,
+            'ref': 'BS'})
+        relation_school2bart = relation_model.create({
+            'left_partner_id': partner_school.id,
+            'type_id': type_school2student.id,
+            'right_partner_id': partner_bart.id})
+        # Delete type. Relations with type should also cease to exist:
+        type_school2student.unlink()
+        self.assertFalse(relation_school2bart.exists())
diff --git a/partner_multi_relation/tests/test_partner_relation_all.py b/partner_multi_relation/tests/test_partner_relation_all.py
index e2c48f46a..8da567e61 100644
--- a/partner_multi_relation/tests/test_partner_relation_all.py
+++ b/partner_multi_relation/tests/test_partner_relation_all.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Copyright 2016 Therp BV
+# Copyright 2016-2017 Therp BV
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from openerp.exceptions import ValidationError
 
@@ -13,8 +13,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
 
         # Create a new relation type which will not have valid relations:
         category_nobody = self.category_model.create({
-            'name': 'Nobody',
-        })
+            'name': 'Nobody'})
         (self.type_nobody,
          self.selection_nobody,
          self.selection_nobody_inverse) = (
@@ -24,30 +23,38 @@ class TestPartnerRelation(TestPartnerRelationCommon):
                 'contact_type_left': 'c',
                 'contact_type_right': 'p',
                 'partner_category_left': category_nobody.id,
-                'partner_category_right': category_nobody.id,
-            })
-        )
+                'partner_category_right': category_nobody.id}))
 
     def _get_empty_relation(self):
         """Get empty relation record for onchange tests."""
         # Need English, because we will compare text
         return self.relation_all_model.with_context(lang='en_US').new({})
 
+    def test_get_partner_types(self):
+        """Partner types should contain at least 'c' and 'p'."""
+        partner_types = self.selection_model.get_partner_types()
+        type_codes = [ptype[0] for ptype in partner_types]
+        self.assertTrue('c' in type_codes)
+        self.assertTrue('p' in type_codes)
+
     def test_create_with_active_id(self):
         """Test creation with this_partner_id from active_id."""
         # Check wether we can create connection from company to person,
         # taking the particular company from the active records:
         relation = self.relation_all_model.with_context(
             active_id=self.partner_02_company.id,
-            active_ids=self.partner_02_company.ids,
-        ).create({
-            'other_partner_id': self.partner_01_person.id,
-            'type_selection_id': self.selection_company2person.id,
-        })
+            active_ids=self.partner_02_company.ids).create({
+                'other_partner_id': self.partner_01_person.id,
+                'type_selection_id': self.selection_company2person.id})
         self.assertTrue(relation)
         self.assertEqual(relation.this_partner_id, self.partner_02_company)
         # Partner should have one relation now:
         self.assertEqual(self.partner_01_person.relation_count, 1)
+        # Test create without type_selection_id:
+        with self.assertRaises(ValidationError):
+            self.relation_all_model.create({
+                'this_partner_id': self.partner_02_company.id,
+                'other_partner_id': self.partner_01_person.id})
 
     def test_display_name(self):
         """Test display name"""
@@ -56,16 +63,12 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             relation.display_name, '%s %s %s' % (
                 relation.this_partner_id.name,
                 relation.type_selection_id.name,
-                relation.other_partner_id.name,
-            )
-        )
+                relation.other_partner_id.name))
 
     def test__regular_write(self):
         """Test write with valid data."""
         relation = self._create_company2person_relation()
-        relation.write({
-            'date_start': '2014-09-01',
-        })
+        relation.write({'date_start': '2014-09-01'})
         relation.invalidate_cache(ids=relation.ids)
         self.assertEqual(relation.date_start, '2014-09-01')
 
@@ -75,8 +78,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         with self.assertRaises(ValidationError):
             relation.write({
                 'date_start': '2016-09-01',
-                'date_end': '2016-08-01',
-            })
+                'date_end': '2016-08-01'})
 
     def test_validate_overlapping_01(self):
         """Test create overlapping with no start / end dates."""
@@ -86,8 +88,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             self.relation_all_model.create({
                 'this_partner_id': relation.this_partner_id.id,
                 'type_selection_id': relation.type_selection_id.id,
-                'other_partner_id': relation.other_partner_id.id,
-            })
+                'other_partner_id': relation.other_partner_id.id})
 
     def test_validate_overlapping_02(self):
         """Test create overlapping with start / end dates."""
@@ -96,8 +97,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'type_selection_id': self.selection_company2person.id,
             'other_partner_id': self.partner_01_person.id,
             'date_start': '2015-09-01',
-            'date_end': '2016-08-31',
-        })
+            'date_end': '2016-08-31'})
         # New relation with overlapping start / end should give error
         with self.assertRaises(ValidationError):
             self.relation_all_model.create({
@@ -105,8 +105,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
                 'type_selection_id': relation.type_selection_id.id,
                 'other_partner_id': relation.other_partner_id.id,
                 'date_start': '2016-08-01',
-                'date_end': '2017-07-30',
-            })
+                'date_end': '2017-07-30'})
 
     def test_validate_overlapping_03(self):
         """Test create not overlapping."""
@@ -115,15 +114,13 @@ class TestPartnerRelation(TestPartnerRelationCommon):
             'type_selection_id': self.selection_company2person.id,
             'other_partner_id': self.partner_01_person.id,
             'date_start': '2015-09-01',
-            'date_end': '2016-08-31',
-        })
+            'date_end': '2016-08-31'})
         relation_another_record = self.relation_all_model.create({
             'this_partner_id': relation.this_partner_id.id,
             'type_selection_id': relation.type_selection_id.id,
             'other_partner_id': relation.other_partner_id.id,
             'date_start': '2016-09-01',
-            'date_end': '2017-08-31',
-        })
+            'date_end': '2017-08-31'})
         self.assertTrue(relation_another_record)
 
     def test_inverse_record(self):
@@ -131,35 +128,45 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         relation = self._create_company2person_relation()
         inverse_relation = self.relation_all_model.search([
             ('this_partner_id', '=', relation.other_partner_id.id),
-            ('other_partner_id', '=', relation.this_partner_id.id),
-        ])
+            ('other_partner_id', '=', relation.this_partner_id.id)])
         self.assertEqual(len(inverse_relation), 1)
         self.assertEqual(
             inverse_relation.type_selection_id.name,
-            self.selection_person2company.name
-        )
+            self.selection_person2company.name)
 
     def test_inverse_creation(self):
         """Test creation of record through inverse selection."""
         relation = self.relation_all_model.create({
             'this_partner_id': self.partner_01_person.id,
             'type_selection_id': self.selection_person2company.id,
-            'other_partner_id': self.partner_02_company.id,
-        })
+            'other_partner_id': self.partner_02_company.id})
         # Check wether display name is what we should expect:
         self.assertEqual(
             relation.display_name, '%s %s %s' % (
                 self.partner_01_person.name,
                 self.selection_person2company.name,
-                self.partner_02_company.name,
-            )
-        )
+                self.partner_02_company.name))
+
+    def test_inverse_creation_type_id(self):
+        """Test creation of record through inverse selection with type_id."""
+        relation = self.relation_all_model.create({
+            'this_partner_id': self.partner_01_person.id,
+            'type_id': self.selection_person2company.type_id.id,
+            'is_inverse': True,
+            'other_partner_id': self.partner_02_company.id})
+        # Check wether display name is what we should expect:
+        self.assertEqual(
+            relation.display_name, '%s %s %s' % (
+                self.partner_01_person.name,
+                self.selection_person2company.name,
+                self.partner_02_company.name))
 
     def test_unlink(self):
         """Unlinking derived relation should unlink base relation."""
         # Check wether underlying record is removed when record is removed:
         relation = self._create_company2person_relation()
-        base_relation = relation.relation_id
+        base_model = self.env[relation.res_model]
+        base_relation = base_model.browse([relation.res_id])
         relation.unlink()
         self.assertFalse(base_relation.exists())
 
@@ -178,23 +185,20 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         relation = self._create_company2person_relation()
         domain = relation.onchange_type_selection_id()['domain']
         self.assertTrue(
-            ('is_company', '=', False) in domain['other_partner_id']
-        )
-        # 3. Test with relation needing categories.
-        relation_ngo_volunteer = self.relation_all_model.create({
-            'this_partner_id': self.partner_03_ngo.id,
-            'type_selection_id': self.selection_ngo2volunteer.id,
-            'other_partner_id': self.partner_04_volunteer.id,
-        })
+            ('is_company', '=', False) in domain['other_partner_id'])
+        # 3. Test with relation needing categories,
+        #    take active partner from active_id:
+        relation_ngo_volunteer = self.relation_all_model.with_context(
+            active_id=self.partner_03_ngo.id).create({
+                'type_selection_id': self.selection_ngo2volunteer.id,
+                'other_partner_id': self.partner_04_volunteer.id})
         domain = relation_ngo_volunteer.onchange_type_selection_id()['domain']
         self.assertTrue(
             ('category_id', 'in', [self.category_01_ngo.id]) in
-            domain['this_partner_id']
-        )
+            domain['this_partner_id'])
         self.assertTrue(
             ('category_id', 'in', [self.category_02_volunteer.id]) in
-            domain['other_partner_id']
-        )
+            domain['other_partner_id'])
         # 4. Test with invalid or impossible combinations
         relation_nobody = self._get_empty_relation()
         with self.env.do_in_draft():
@@ -208,9 +212,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         self.assertTrue('message' in warning)
         self.assertTrue('incompatible' in warning['message'])
         # Allow left partner and check message for other partner:
-        self.type_nobody.write({
-            'partner_category_left': False,
-        })
+        self.type_nobody.write({'partner_category_left': False})
         self.selection_nobody.invalidate_cache(ids=self.selection_nobody.ids)
         warning = relation_nobody.onchange_type_selection_id()['warning']
         self.assertTrue('message' in warning)
@@ -229,8 +231,7 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         relation = self._create_company2person_relation()
         domain = relation.onchange_partner_id()['domain']
         self.assertTrue(
-            ('contact_type_this', '=', 'c') in domain['type_selection_id']
-        )
+            ('contact_type_this', '=', 'c') in domain['type_selection_id'])
         # 3. Test with invalid or impossible combinations
         relation_nobody = self._get_empty_relation()
         with self.env.do_in_draft():
@@ -239,3 +240,39 @@ class TestPartnerRelation(TestPartnerRelationCommon):
         warning = relation_nobody.onchange_partner_id()['warning']
         self.assertTrue('message' in warning)
         self.assertTrue('incompatible' in warning['message'])
+
+    def test_write(self):
+        """Test write. Special attention for changing type."""
+        relation_company2person = self._create_company2person_relation()
+        company_partner = relation_company2person.this_partner_id
+        # First get another worker:
+        partner_extra_person = self.partner_model.create({
+            'name': 'A new worker',
+            'is_company': False,
+            'ref': 'NW01'})
+        relation_company2person.write({
+            'other_partner_id': partner_extra_person.id})
+        self.assertEqual(
+            relation_company2person.other_partner_id.name,
+            partner_extra_person.name)
+        # We will also change to a type going from person to company:
+        (type_worker2company,
+         selection_worker2company,
+         selection_company2worker) = self._create_relation_type_selection({
+             'name': 'works for',
+             'name_inverse': 'has worker',
+             'contact_type_left': 'p',
+             'contact_type_right': 'c'})
+        relation_company2person.write({
+            'this_partner_id': partner_extra_person.id,
+            'type_selection_id': selection_worker2company.id,
+            'other_partner_id': company_partner.id})
+        self.assertEqual(
+            relation_company2person.this_partner_id.id,
+            partner_extra_person.id)
+        self.assertEqual(
+            relation_company2person.type_selection_id.id,
+            selection_worker2company.id)
+        self.assertEqual(
+            relation_company2person.other_partner_id.id,
+            company_partner.id)
diff --git a/partner_multi_relation/tests/test_partner_relation_common.py b/partner_multi_relation/tests/test_partner_relation_common.py
index 94249c6c7..fde3d5385 100644
--- a/partner_multi_relation/tests/test_partner_relation_common.py
+++ b/partner_multi_relation/tests/test_partner_relation_common.py
@@ -18,75 +18,60 @@ class TestPartnerRelationCommon(common.TransactionCase):
         self.partner_01_person = self.partner_model.create({
             'name': 'Test User 1',
             'is_company': False,
-            'ref': 'PR01',
-        })
+            'ref': 'PR01'})
         self.partner_02_company = self.partner_model.create({
             'name': 'Test Company',
             'is_company': True,
-            'ref': 'PR02',
-        })
+            'ref': 'PR02'})
         # Create partners with specific categories:
-        self.category_01_ngo = self.category_model.create({
-            'name': 'NGO',
-        })
+        self.category_01_ngo = self.category_model.create({'name': 'NGO'})
         self.partner_03_ngo = self.partner_model.create({
             'name': 'Test NGO',
             'is_company': True,
             'ref': 'PR03',
-            'category_id': [(4, self.category_01_ngo.id)],
-        })
+            'category_id': [(4, self.category_01_ngo.id)]})
         self.category_02_volunteer = self.category_model.create({
-            'name': 'Volunteer',
-        })
+            'name': 'Volunteer'})
         self.partner_04_volunteer = self.partner_model.create({
             'name': 'Test Volunteer',
             'is_company': False,
             'ref': 'PR04',
-            'category_id': [(4, self.category_02_volunteer.id)],
-        })
+            'category_id': [(4, self.category_02_volunteer.id)]})
         # Create a new relation type withouth categories:
         (self.type_company2person,
          self.selection_company2person,
-         self.selection_person2company) = (
+         self.selection_person2company) = \
             self._create_relation_type_selection({
                 'name': 'mixed',
                 'name_inverse': 'mixed_inverse',
                 'contact_type_left': 'c',
-                'contact_type_right': 'p',
-            })
-        )
+                'contact_type_right': 'p'})
         # Create a new relation type with categories:
         (self.type_ngo2volunteer,
          self.selection_ngo2volunteer,
-         self.selection_volunteer2ngo) = (
+         self.selection_volunteer2ngo) = \
             self._create_relation_type_selection({
                 'name': 'NGO has volunteer',
                 'name_inverse': 'volunteer works for NGO',
                 'contact_type_left': 'c',
                 'contact_type_right': 'p',
                 'partner_category_left': self.category_01_ngo.id,
-                'partner_category_right': self.category_02_volunteer.id,
-            })
-        )
+                'partner_category_right': self.category_02_volunteer.id})
 
     def _create_relation_type_selection(self, vals):
         """Create relation type and return this with selection types."""
         assert 'name' in vals, (
             "Name missing in vals to create relation type. Vals: %s."
-            % vals
-        )
+            % vals)
         assert 'name' in vals, (
             "Name_inverse missing in vals to create relation type. Vals: %s."
-            % vals
-        )
+            % vals)
         new_type = self.type_model.create(vals)
         self.assertTrue(
             new_type,
-            msg="No relation type created with vals %s." % vals
-        )
+            msg="No relation type created with vals %s." % vals)
         selection_types = self.selection_model.search([
-            ('type_id', '=', new_type.id),
-        ])
+            ('type_id', '=', new_type.id)])
         for st in selection_types:
             if st.is_inverse:
                 inverse_type_selection = st
@@ -95,13 +80,11 @@ class TestPartnerRelationCommon(common.TransactionCase):
         self.assertTrue(
             inverse_type_selection,
             msg="Failed to find inverse type selection based on"
-                " relation type created with vals %s." % vals
-        )
+                " relation type created with vals %s." % vals)
         self.assertTrue(
             type_selection,
             msg="Failed to find type selection based on"
-                " relation type created with vals %s." % vals
-        )
+                " relation type created with vals %s." % vals)
         return (new_type, type_selection, inverse_type_selection)
 
     def _create_company2person_relation(self):
@@ -109,5 +92,4 @@ class TestPartnerRelationCommon(common.TransactionCase):
         return self.relation_all_model.create({
             'type_selection_id': self.selection_company2person.id,
             'this_partner_id': self.partner_02_company.id,
-            'other_partner_id': self.partner_01_person.id,
-        })
+            'other_partner_id': self.partner_01_person.id})
diff --git a/partner_multi_relation/views/res_partner_relation_all.xml b/partner_multi_relation/views/res_partner_relation_all.xml
index 0841835fd..76d3a0ea0 100644
--- a/partner_multi_relation/views/res_partner_relation_all.xml
+++ b/partner_multi_relation/views/res_partner_relation_all.xml
@@ -41,11 +41,11 @@
                     <field name="type_selection_id"/>
                     <filter
                         string="Left to right"
-                        domain="[('record_type', '=', 'a')]"
+                        domain="[('is_inverse', '=', False)]"
                         />
                     <filter
                         string="Right to left"
-                        domain="[('record_type', '=', 'b')]"
+                        domain="[('is_inverse', '=', True)]"
                         />
                     <filter
                         string="Include past records"
diff --git a/partner_multi_relation/views/res_partner_relation_type.xml b/partner_multi_relation/views/res_partner_relation_type.xml
index 2854c6f0e..fb3cbd605 100644
--- a/partner_multi_relation/views/res_partner_relation_type.xml
+++ b/partner_multi_relation/views/res_partner_relation_type.xml
@@ -39,7 +39,11 @@
                                 <field name="partner_category_right" />
                             </group>
                         </group>
-                        <group name="properties" string="Properties">
+                        <group
+                            name="properties"
+                            string="Properties"
+                            colspan= "6" col="4"
+                            >
                             <field name="allow_self" />
                             <field name="is_symmetric" />
                             <field name="handle_invalid_onchange" />