Merge lp:~acsone-openerp/openerp-connector/multi-company-job into lp:~openerp-connector-core-editors/openerp-connector/7.0

[Laurent Mignon (Acsone)]

Here is the proposed changes to support multi companies by the connector.

Summary of changes:
-------------------
* add a new field company_id on the queue_job model (optional)
* modify the method OpenERPJobStorage.enqueue to fill the company_id with the default company of the user or the one provided in the context.
* add a record rule on queue.job to restrict the visibility by company
* modify the method QueueJob.subscribe_users to only subscribe users members of the company_id

Motivations and details:
------------------------
Motivations and details of the changes were discussed in https://lists.launchpad.net/openerp-connector-community/msg00253.html,

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hi,

This is a really good work. Thanks

I'm wondering, when 'company_id' is not in the context, the job will be assigned to the default company (by a rule or the user's one), so it will never be empty, could it be a problem? Also, it seems to me that the column could be 'not null'.

review: Needs Information

[Laurent Mignon (Acsone) (lmi)]

Guewen,
>
> This is a really good work. Thanks

Thank you for the review.

> I'm wondering, when 'company_id' is not in the context, the job will be
> assigned to the default company (by a rule or the user's one), so it will
> never be empty, could it be a problem?

This is a point that I wondered for a long time. I think that in some cases, we need to have the possibility to keep the company_id empty by default. In this kind of use cases the developer can subclass the ConnectorSession class to always put an empty company_id in the context.

> Also, it seems to me that the column could be 'not null'.

I do not think so. In my use of the framework, I've some global jobs not linked to a company_id and I think it's safer to let the possibility to not restrict a job to a company_id. Since the ConnectorSession provides a clean way to locally change the context, we have a clean way to put an empty company_id in the context before delaying a job.

 with self.session.change_context({'company_id': None}):
    my_job.delay(...)

Regards,

lmi

[Laurent Mignon (Acsone) (lmi)]

I wonder if it could be interesting to have a select=True on the company_id to have an SQL index on this column.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

On Thu, May 1, 2014 at 9:57 AM, Laurent Mignon (Acsone)
<email address hidden> wrote:
> I wonder if it could be interesting to have a select=True on the company_id to have an SQL index on this column.
> --
> https://code.launchpad.net/~acsone-openerp/openerp-connector/multi-company-job/+merge/214895
> You are reviewing the proposed merge of lp:~acsone-openerp/openerp-connector/multi-company-job into lp:openerp-connector.

Most of the OpenERP implementation will have 1 to a few companies, so
I don't think an index would be efficient with only a few distinct
values.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> Guewen,
> >
> > This is a really good work. Thanks
>
> Thank you for the review.
>
> > I'm wondering, when 'company_id' is not in the context, the job will be
> > assigned to the default company (by a rule or the user's one), so it will
> > never be empty, could it be a problem?
>
> This is a point that I wondered for a long time. I think that in some cases,
> we need to have the possibility to keep the company_id empty by default. In
> this kind of use cases the developer can subclass the ConnectorSession class
> to always put an empty company_id in the context.
>
> > Also, it seems to me that the column could be 'not null'.
>
> I do not think so. In my use of the framework, I've some global jobs not
> linked to a company_id and I think it's safer to let the possibility to not
> restrict a job to a company_id. Since the ConnectorSession provides a clean
> way to locally change the context, we have a clean way to put an empty
> company_id in the context before delaying a job.
>
> with self.session.change_context({'company_id': None}):
> my_job.delay(...)
>
> Regards,
>
> lmi

Well, I agree. That's ok for me.
Thanks again for your work.

review: Approve (code review, launched unit test)

=== modified file 'connector/queue/job.py'
--- connector/queue/job.py	2014-02-04 14:39:23 +0000
+++ connector/queue/job.py	2014-04-09 08:17:14 +0000
@@ -112,6 +112,14 @@
         job = Job(func=func, model_name=model_name, args=args, kwargs=kwargs,
                   priority=priority, eta=eta, max_retries=max_retries, description=description)
         job.user_id = self.session.uid
+        if 'company_id' in self.session.context:
+            company_id = self.session.context['company_id']
+        else:
+            company_id = self.session.pool.get('res.company')._company_default_get(self.session.cr, job.user_id,
+                                    object='queue.job',
+                                    field='company_id',
+                                    context=self.session.context)
+        job.company_id = company_id
         self.store(job)
         return job.uuid
 
@@ -167,6 +175,7 @@
                 'max_retries': job.max_retries,
                 'exc_info': job.exc_info,
                 'user_id': job.user_id or self.session.uid,
+                'company_id': job.company_id,
                 'result': unicode(job.result) if job.result else False,
                 'date_enqueued': False,
                 'date_started': False,
@@ -266,6 +275,8 @@
         job.max_retries = stored.max_retries
         if stored.worker_id:
             job.worker_uuid = stored.worker_id.uuid
+        if stored.company_id:
+            job.company_id = stored.company_id.id
         return job
 
 
@@ -443,6 +454,7 @@
         self.exc_info = None
 
         self.user_id = None
+        self.company_id = None
         self._eta = None
         self.eta = eta
         self.canceled = False

=== modified file 'connector/queue/model.py'
--- connector/queue/model.py	2013-11-19 10:14:40 +0000
+++ connector/queue/model.py	2014-04-09 08:17:14 +0000
@@ -51,6 +51,7 @@
                                      ondelete='set null', readonly=True),
         'uuid': fields.char('UUID', readonly=True, select=True, required=True),
         'user_id': fields.many2one('res.users', string='User ID', required=True),
+        'company_id' : fields.many2one('res.company', 'Company'),
         'name': fields.char('Description', readonly=True),
         'func_string': fields.char('Task', readonly=True),
         'func': fields.text('Pickled Function', readonly=True, required=True),
@@ -132,8 +133,12 @@
         if not group_ref:
             return
         group_id = group_ref[1]
+        company_ids = [val['company_id'][0] for val in  self.read(cr, uid, ids, ['company_id'], context=context) if val['company_id']]
+        domain = [('groups_id', '=', group_id)]
+        if company_ids:
+            domain.append(('company_ids', 'child_of', company_ids))
         user_ids = self.pool.get('res.users').search(
-                cr, uid, [('groups_id', '=', group_id)], context=context)
+                cr, uid, domain, context=context)
         self.message_subscribe_users(cr, uid, ids,
                                      user_ids=user_ids,
                                      context=context)

=== modified file 'connector/queue/model_view.xml'
--- connector/queue/model_view.xml	2013-03-22 15:00:58 +0000
+++ connector/queue/model_view.xml	2014-04-09 08:17:14 +0000
@@ -80,6 +80,7 @@
                             <field name="func_string"/>
                             <field name="priority"/>
                             <field name="eta"/>
+                            <field name="company_id" groups="base.group_multi_company"/>
                             <field name="user_id"/>
                             <field name="date_created"/>
                             <field name="date_enqueued"/>
@@ -123,6 +124,7 @@
                     <field name="eta"/>
                     <field name="date_created"/>
                     <field name="date_done"/>
+                    <field name="company_id" groups="base.group_multi_company"/>
                 </tree>
             </field>
         </record>
@@ -135,6 +137,7 @@
                     <field name="uuid"/>
                     <field name="name"/>
                     <field name="func_string"/>
+                    <field name="company_id" groups="base.group_multi_company" widget="selection"/>
                     <filter name="pending" string="Pending"
                         domain="[('state', '=', 'pending')]"/>
                     <filter name="enqueued" string="Enqueued"

=== modified file 'connector/security/connector_security.xml'
--- connector/security/connector_security.xml	2013-03-20 19:43:49 +0000
+++ connector/security/connector_security.xml	2014-04-09 08:17:14 +0000
@@ -14,4 +14,16 @@
     </record>
 
     </data>
+    
+    <data noupdate="1">
+
+    <record id="queue_job_comp_rule" model="ir.rule">
+      <field name="name" >Queue job multi-company</field>
+      <field name="model_id" ref="model_queue_job"/>
+      <field name="global" eval="True"/>
+      <field name="domain_force">['|',('company_id','=',False),('company_id','child_of',[user.company_id.id])]</field>
+    </record>
+
+  </data>
+ 
 </openerp>

=== modified file 'connector/tests/test_job.py'
--- connector/tests/test_job.py	2014-01-13 16:07:35 +0000
+++ connector/tests/test_job.py	2014-04-09 08:17:14 +0000
@@ -5,6 +5,7 @@
 from datetime import datetime, timedelta
 
 import openerp
+from openerp import SUPERUSER_ID
 import openerp.tests.common as common
 from openerp.addons.connector.queue.job import (
         Job, OpenERPJobStorage, job,
@@ -133,6 +134,7 @@
                   eta=eta,
                   description="My description")
         job.user_id = 1
+        job.company_id = self.ref("base.main_company")
         storage = OpenERPJobStorage(self.session)
         storage.store(job)
         job_read = storage.load(job.uuid)
@@ -149,13 +151,14 @@
         self.assertEqual(job.exc_info, job_read.exc_info)
         self.assertEqual(job.result, job_read.result)
         self.assertEqual(job.user_id, job_read.user_id)
+        self.assertEqual(job.company_id, job_read.company_id)
         delta = timedelta(seconds=1)  # DB does not keep milliseconds
         self.assertAlmostEqual(job.date_created, job_read.date_created,
                                delta=delta)
         self.assertAlmostEqual(job.date_started, job_read.date_started,
                                delta=delta)
         self.assertAlmostEqual(job.date_enqueued, job_read.date_enqueued,
-                               delta=delta)
+                               delta=delta) 
         self.assertAlmostEqual(job.date_done, job_read.date_done,
                                delta=delta)
         self.assertAlmostEqual(job.eta, job_read.eta,
@@ -179,3 +182,121 @@
         task_a.delay(self.session, 'res.users', 'o', 'k', c='!')
         stored = self.queue_job.search(self.cr, self.uid, [])
         self.assertEqual(len(stored), 1)
+
+
+class test_job_storage_multi_company(common.TransactionCase):
+    """ Test storage of jobs """
+
+    def setUp(self):
+        super(test_job_storage_multi_company, self).setUp()
+        self.pool = openerp.modules.registry.RegistryManager.get(common.DB)
+        self.session = ConnectorSession(self.cr, self.uid, context={})
+        self.queue_job = self.registry('queue.job')
+        self.other_partner_id_a = self.registry('res.partner').create(self.cr, self.uid, {
+                            "name": "My Company a",
+                            "is_company": True,
+                            "email": "[e-mail]",
+                            })
+        self.other_company_id_a = self.registry('res.company').create(self.cr, self.uid, {
+                            "name": "My Company a",
+                            "partner_id": self.other_partner_id_a,
+                            "rml_header1": "My Company Tagline",
+                            "currency_id": self.ref("base.EUR")
+                            })
+        self.other_user_id_a = self.registry('res.users').create(self.cr, self.uid, {
+                            "partner_id": self.other_partner_id_a,
+                            "company_id": self.other_company_id_a,
+                            "company_ids": [(4, self.other_company_id_a)],
+                            "login": "my_login a",
+                            "name": "my user",
+                            "groups_id": [
+                                                  (4, self.ref("connector.group_connector_manager"))]
+                            })
+        self.other_partner_id_b = self.registry('res.partner').create(self.cr, self.uid, {
+                            "name": "My Company b",
+                            "is_company": True,
+                            "email": "[e-mail]",
+                            })
+        self.other_company_id_b = self.registry('res.company').create(self.cr, self.uid, {
+                            "name": "My Company b",
+                            "partner_id": self.other_partner_id_b,
+                            "rml_header1": "My Company Tagline",
+                            "currency_id": self.ref("base.EUR")
+                            })
+        self.other_user_id_b = self.registry('res.users').create(self.cr, self.uid, {
+                            "partner_id": self.other_partner_id_b,
+                            "company_id": self.other_company_id_b,
+                            "company_ids": [(4, self.other_company_id_b)],
+                            "login": "my_login_b",
+                            "name": "my user 1",
+                            "groups_id": [
+                                                  (4, self.ref("connector.group_connector_manager"))]
+                            })
+
+    def _create_job(self):
+        self.cr.execute('delete from queue_job')
+        job(task_a)
+        task_a.delay(self.session, 'res.users')
+        stored = self.queue_job.search(self.cr, self.uid, [])
+        self.assertEqual(len(stored), 1)
+        return self.queue_job.browse(self.cr, self.uid, stored[0])
+
+    def test_job_default_company_id(self):
+        """the default company is the one from the current user_id"""
+        stored_brw = self._create_job()
+        self.assertEqual(
+                         stored_brw.company_id.id,
+                         self.ref("base.main_company"),
+                         'Incorrect default company_id')
+        with self.session.change_user(self.other_user_id_b):
+            stored_brw = self._create_job()
+            self.assertEqual(
+                             stored_brw.company_id.id,
+                             self.other_company_id_b,
+                             'Incorrect default company_id')
+
+    def test_job_no_company_id(self):
+        """ if we put an empty company_id in the context
+         jobs are created without company_id"""
+        with self.session.change_context({'company_id': None}):
+            stored_brw = self._create_job()
+            self.assertFalse(
+                             stored_brw.company_id,
+                             ' Company_id should be empty')
+
+    def test_job_specific_company_id(self):
+        """If a company_id specified in the context
+        it's used by default for the job creation"""
+        with self.session.change_context({'company_id': self.other_company_id_a}):
+            stored_brw = self._create_job()
+            self.assertEqual(
+                             stored_brw.company_id.id,
+                             self.other_company_id_a,
+                             'Incorrect company_id')
+
+    def test_job_subscription(self):
+        # if the job is created without company_id, all members of
+        # connector.group_connector_manager must be followers
+        with self.session.change_context({'company_id': None}):
+            stored_brw = self._create_job()
+        self.queue_job. _subscribe_users(self.cr, self.uid, [stored_brw.id])
+        stored_brw.refresh()
+        user_ids = self.registry('res.users').search(
+                self.cr, self.uid, [('groups_id', '=', self.ref('connector.group_connector_manager'))])
+        self.assertEqual(len(stored_brw.message_follower_ids), len(user_ids))
+        expected_partners = [u.partner_id for u in self.registry("res.users").browse(self.cr, self.uid, user_ids)]
+        self.assertSetEqual(set(stored_brw.message_follower_ids), set(expected_partners))
+        followers_id = [f.id for f in stored_brw.message_follower_ids]
+        self.assertIn(self.other_partner_id_a, followers_id)
+        self.assertIn(self.other_partner_id_b, followers_id)
+        # jobs created for a specific company_id are followed only by company's members
+        with self.session.change_context({'company_id': self.other_company_id_a}):
+            stored_brw = self._create_job()
+        self.queue_job. _subscribe_users(self.cr, self.other_user_id_a, [stored_brw.id])
+        stored_brw.refresh()
+        self.assertEqual(len(stored_brw.message_follower_ids), 2)  # 2 because admin + self.other_partner_id_a
+        expected_partners = [u.partner_id for u in self.registry("res.users").browse(self.cr, self.uid, [SUPERUSER_ID, self.other_user_id_a])]
+        self.assertSetEqual(set(stored_brw.message_follower_ids), set(expected_partners))
+        followers_id = [f.id for f in stored_brw.message_follower_ids]
+        self.assertIn(self.other_partner_id_a, followers_id)
+        self.assertNotIn(self.other_partner_id_b, followers_id)